Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Etsuro Fujita]

On 2015/09/04 19:50, Etsuro Fujita wrote:

I'm attaching an updated version of the patch.  The patch is based on
the SS_finalize_plan patch that has been recently committed.  I'd be
happy if this helps people discuss more about how to fix this issue.


In the updated version, I modified finalize_plan so that initPlans 
attached to a ForeignScan node doing a remote join are considered for 
the computed params for a local join plan for EvalPlanQual testing.  But 
I noticed no need for that.  The reason is, no initPlans will be 
attached to the ForeignScan node due to that the ForeignScan node is 
unable to be the topmost plan node for the query level in case of 
EvalPlanQual testing.  So, I removed that code.  Patch attached.  (That 
no longer depends on the SS_finalize_plan patch.)


Best regards,
Etsuro Fujita
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***
*** 525,530  fileGetForeignPaths(PlannerInfo *root,
--- 525,531 
  	 total_cost,
  	 NIL,		/* no pathkeys */
  	 NULL,		/* no outer rel either */
+ 	 NULL,		/* no alternative path */
  	 coptions));
  
  	/*
***
*** 563,569  fileGetForeignPlan(PlannerInfo *root,
  			scan_relid,
  			NIL,	/* no expressions to evaluate */
  			best_path->fdw_private,
! 			NIL /* no custom tlist */ );
  }
  
  /*
--- 564,571 
  			scan_relid,
  			NIL,	/* no expressions to evaluate */
  			best_path->fdw_private,
! 			NIL,	/* no custom tlist */
! 			NIL /* no remote quals */ );
  }
  
  /*
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***
*** 560,565  postgresGetForeignPaths(PlannerInfo *root,
--- 560,566 
     fpinfo->total_cost,
     NIL, /* no pathkeys */
     NULL,		/* no outer rel either */
+    NULL,		/* no alternative path */
     NIL);		/* no fdw_private list */
  	add_path(baserel, (Path *) path);
  
***
*** 727,732  postgresGetForeignPaths(PlannerInfo *root,
--- 728,734 
  	   total_cost,
  	   NIL,		/* no pathkeys */
  	   param_info->ppi_req_outer,
+ 	   NULL,	/* no alternative path */
  	   NIL);	/* no fdw_private list */
  		add_path(baserel, (Path *) path);
  	}
***
*** 748,753  postgresGetForeignPlan(PlannerInfo *root,
--- 750,756 
  	Index		scan_relid = baserel->relid;
  	List	   *fdw_private;
  	List	   *remote_conds = NIL;
+ 	List	   *remote_exprs = NIL;
  	List	   *local_exprs = NIL;
  	List	   *params_list = NIL;
  	List	   *retrieved_attrs;
***
*** 769,776  postgresGetForeignPlan(PlannerInfo *root,
  	 *
  	 * This code must match "extract_actual_clauses(scan_clauses, false)"
  	 * except for the additional decision about remote versus local execution.
! 	 * Note however that we only strip the RestrictInfo nodes from the
! 	 * local_exprs list, since appendWhereClause expects a list of
  	 * RestrictInfos.
  	 */
  	foreach(lc, scan_clauses)
--- 772,779 
  	 *
  	 * This code must match "extract_actual_clauses(scan_clauses, false)"
  	 * except for the additional decision about remote versus local execution.
! 	 * Note however that we don't strip the RestrictInfo nodes from the
! 	 * remote_conds list, since appendWhereClause expects a list of
  	 * RestrictInfos.
  	 */
  	foreach(lc, scan_clauses)
***
*** 784,794  postgresGetForeignPlan(PlannerInfo *root,
--- 787,803 
  			continue;
  
  		if (list_member_ptr(fpinfo->remote_conds, rinfo))
+ 		{
  			remote_conds = lappend(remote_conds, rinfo);
+ 			remote_exprs = lappend(remote_exprs, rinfo->clause);
+ 		}
  		else if (list_member_ptr(fpinfo->local_conds, rinfo))
  			local_exprs = lappend(local_exprs, rinfo->clause);
  		else if (is_foreign_expr(root, baserel, rinfo->clause))
+ 		{
  			remote_conds = lappend(remote_conds, rinfo);
+ 			remote_exprs = lappend(remote_exprs, rinfo->clause);
+ 		}
  		else
  			local_exprs = lappend(local_exprs, rinfo->clause);
  	}
***
*** 874,880  postgresGetForeignPlan(PlannerInfo *root,
  			scan_relid,
  			params_list,
  			fdw_private,
! 			NIL /* no custom tlist */ );
  }
  
  /*
--- 883,890 
  			scan_relid,
  			params_list,
  			fdw_private,
! 			NIL,	/* no custom tlist */
! 			remote_exprs);
  }
  
  /*
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***
*** 333,339  GetForeignJoinPaths (PlannerInfo *root,
   remote join cannot be found from the system catalogs, the FDW must
   fill fdw_scan_tlist with an appropriate list
   of TargetEntry nodes, representing the set of columns
!  it will supply at runtime in the tuples it returns.
  
  
  
--- 333,343 
   remote join cannot be found from the system catalogs, the FDW must
   fill fdw_scan_tlist with an appropriate list
   

Re: [HACKERS] checkpointer continuous flushing

[Fabien COELHO]

Hello Amit,


It'd not really simplify things, but it'd keep it local.


How about using the value of guc (checkpoint_flush_to_disk) and 
AmCheckpointerProcess to identify whether to do async flush in 
FileWrite?


ISTM that what you suggest would just replace the added function arguments 
with global variables to communicate and keep the necessary data for 
managing the asynchronous flushing, which is called per tablespace

(1) on file changes (2) when the checkpointer is going to sleep.

Although it can be done obviously, I prefer to have functions arguments 
rather than global variables, on principle.


Also, because of (2) and of the dependency on the number of tablespaces 
being flushed, the flushing stuff cannot be fully hidden from the 
checkpointer anyway.


Also I think that probably the bgwriter should do something similar, so 
function parameters would be useful to drive flushing from it, rather than 
adding yet another set of global variables, or share the same variables 
for somehow different purposes.


So having these added parameters look reasonable to me.

--
Fabien.


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Foreign join pushdown vs EvalPlanQual

[Kyotaro HORIGUCHI]

Hello, sorry in advance for possible brought up of past
discussions or pointless discussion.

> I'm attaching an updated version of the patch.  The patch is based on
> the SS_finalize_plan patch that has been recently committed.  I'd be
> happy if this helps people discuss more about how to fix this issue.

The two patches make a good contrast to clarify the problem for
me, maybe.

> > code in ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
> > ExecReScanForeignScan.  I think that would resolve the name problem
> > also.

I found two points in this discussion.

1. Where (or When) to initialize a foreign/custom scan node for
   recheck.

   Having a new list to hold substitute plans in planner global
   (and PlannedStmt) is added, EvalPlanQualStart() looks to be
   the best place to initialize them.

   Of couse it could not be a solution unless the new member and
   related code are not acceptable or rather unreasonable. The
   possible timing left for the case would be ExecInitNode() (as
   v2.0) or FDW routines (as v1.0).

2. How the core informs fdw/custom scan handlers wheter it is
   during recheck.

   In v1.0 patch, nodeForignscan.c routines detect the situation
   using es_epqTuple and Scan.scanrelid which the core as is
   gives, and v2.0 alternatively replaces scan node implicitly
   (and maybe irregularly) itself on initialization. The latter
   don't looks to me tidy.

   I think refining v1.0 would be more desirable, and resolving
   the redundancy would be simply a matter of notation.

   If I understand there correctly, Exec*ForeignScan() other than
   ExecInitForeignScan() can determine the behavior simply
   looking outerPlanState(scanstate).  (If we continue to use the
   member lefttree for the purpose..). Is it right? and does it
   eliminate the redundancy?

ExecEndForeignScan()
{
  if ((outerplan = outerPlanState(node)) != NULL)
 ExecEndNode(outerPlan);
...



regards,

At Fri, 04 Sep 2015 19:50:46 +0900, Etsuro Fujita  
wrote in <55e97786.30...@lab.ntt.co.jp>
> On 2015/09/03 19:25, Etsuro Fujita wrote:
> > On 2015/09/03 14:22, Etsuro Fujita wrote:
> >> On 2015/09/03 9:41, Robert Haas wrote:
> >>> That having been said, I don't entirely like Fujita-san's patch
> >>> either.  Much of the new code is called immediately adjacent to an FDW
> >>> callback which could pretty trivially do the same thing itself, if
> >>> needed.
...
> > I gave it another thought; the following changes to ExecInitNode would
> > make the patch much simpler, ie, we would no longer need to call the
> > new
> > code in ExecInitForeignScan, ExecForeignScan, ExecEndForeignScan, and
> > ExecReScanForeignScan.  I think that would resolve the name problem
> > also.
> 
> I'm attaching an updated version of the patch.  The patch is based on
> the SS_finalize_plan patch that has been recently committed.  I'd be
> happy if this helps people discuss more about how to fix this issue.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] Refactoring of LWLock tranches

[Ildus Kurbangaliev]

On Sun, 6 Sep 2015 23:18:02 +0200
"and...@anarazel.de"  wrote:

> On 2015-09-06 22:57:04 +0300, Ildus Kurbangaliev wrote:
> > Ok, I've kept only one tranche for individual LWLocks
> 
> But you've added the lock names as a statically sized array to all
> tranches? Why not just a pointer to an array that's either NULL ('not
> individualy named locks') or appropriately sized?

A tranche contains only the tranche name. Yes, it's statically sized,
because we need to know an exact space in shared memory for it. This 
name is enough to describe all LWLocks in that tranche (except main
tranche), because they have one type (for example BufferMgrLocks). 
In main tranche this field contains 'main' (like now) and 
an additional array is used for determining a name for LWLock. If you
suggest keep a pointer to this array in main tranche (and NULL for
others) then I have no objections to do that.

So generally the code is similar to code that we have in Postgres now
except the tranches located in shared memory and created by backends.

> 
> > > I don't really like the tranche model as in the patch right now.
> > > I'd rather have in a way that we have one tranch for all the
> > > individual lwlocks, where the tranche points to an array of names
> > > alongside the tranche's name. And then for the others we just
> > > supply the tranche name, but leave the name array empty, whereas
> > > a name can be generated.
> > 
> > Maybe I don't understand something here, but why add extra field to
> > all tranches if we need only one array (especially the array for
> > individual LWLocks).
> 
> It's cheap to add an optional pointer field to an individual
> tranche. It'll be far less cheap to extend the max number of
> tranches. But it's also just ugly to to use a tranche per lock -
> they're intended to describe 'runs' of locks.

The current patch exactly does that - contains 'runs' of locks.


Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
 The Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] On-demand running query plans using auto_explain and signals

[Shulgin, Oleksandr]

On Fri, Sep 4, 2015 at 6:11 AM, Pavel Stehule 
wrote:

> Sorry, but I still don't see how the slots help this issue - could you
>> please elaborate?
>>
> with slot (or some similiar) there is not global locked resource. If I'll
> have a time at weekend I'll try to write some prototype.
>

But you will still lock on the slots list to find an unused one.  How is
that substantially different from what I'm doing?

> >> Other smaller issues:
>> >>
>> >> * probably sending line by line is useless - shm_mq_send can pass
>> bigger data when nowait = false
>>
>> I'm not sending it like that because of the message size - I just find it
>> more convenient. If you think it can be problematic, its easy to do this as
>> before, by splitting lines on the receiving side.
>>
> Yes, shm queue sending data immediately - so slicing on sender generates
> more interprocess communication
>

Well, we are talking about hundreds to thousands bytes per plan in total.
And if my reading of shm_mq implementation is correct, if the message fits
into the shared memory buffer, the receiver gets the direct pointer to the
shared memory, no extra allocation/copy to process-local memory.  So this
can be actually a win.

> >> * pg_usleep(1000L); - it is related to single point resource
>>
>> But not a highly concurrent one.
>>
> I believe so it is not becessary - waiting (sleeping) can be deeper in
> reading from queue - the code will be cleaner
>

The only way I expect this line to be reached is when a concurrent
pg_cmdstatus() call is in progress: the receiving backend has set the
target_pid and has created the queue, released the lock and now waits to
read something from shm_mq.  So the backend that's trying to also use this
communication channel can obtain the lwlock, checks if the channel is not
used at the time, fails and then it needs to check again, but that's going
to put a load on the CPU, so there's a small sleep.

The real problem could be if the process that was signaled to connect to
the message queue never handles the interrupt, and we keep waiting forever
in shm_mq_receive().  We could add a timeout parameter or just let the user
cancel the call: send a cancellation request, use pg_cancel_backend() or
set statement_timeout before running this.

--
Alex

Re: [HACKERS] Waits monitoring

[Ildus Kurbangaliev]

On Mon, 7 Sep 2015 08:58:15 +0530
Amit Kapila  wrote:

> On Sun, Sep 6, 2015 at 5:58 PM, Andres Freund 
> wrote:
> >
> > On 2015-09-04 23:44:21 +0100, Simon Riggs wrote:
> > > I see the need for both current wait information and for
> > > cumulative historical detail.
> > >
> > > I'm willing to wait before reviewing this, but not for more than
> > > 1 more
> CF.
> > >
> > > Andres, please decide whether we should punt to next CF now,
> > > based upon other developments. Thanks
> >
> > I think we can do some of the work concurrently - the whole lwlock
> > infrastructure piece is rather independent and what currently most
> > of the arguments are about. I agree that the actual interface will
> > need to be coordinated.
> >
> > Ildus, could you please review Amit & Robert's patch?
> >
> 
> Are you talking about patch where I have fixed few issues in Robert's
> patch [1] or the original patch [3] written by me.
> 
> IIUC, this is somewhat different than what Ildus is doing in his
> latest patch [2].
> 
> Sorry, but I think there is some confusion about that patch [1]
> development. Let me try to summarize what I think has happened and
> why I feel there is some confusion.  Initially Robert has proposed
> the idea of having a column in pg_stat_activity for wait_event on
> hackers and then I wrote an initial patch so that we can discuss the
> same in a more meaningful way and wanted to extend that patch based
> on consensus and what any other patch like Waits monitoring would
> need from it.  In-between Ildus has proposed
> Waits monitoring patch and also started hacking the other
> pg_stat_activity patch and that was the starting point of confusion.
> Now I think that the current
> situation is there's one patch [1] of Robert (with some fixes by
> myself) for standardising
> LWLock stuff, so that we can build pg_stat_activity patch on top of
> it and then
> a patch [2] from Ildus for doing something similar but I think it
> hasn't used Robert's
> patch.
> 
> I think the intention of having multiple people develop same patch is
> to get the work done faster, but I think here it is causing confusion
> and I feel that
> is one reason the patch is getting dragged as well.  Let me know your
> thoughts
> about what is the best way to proceed?
> 
> [1] -
> http://www.postgresql.org/message-id/caa4ek1kdec1tm5ya9gkv85vtn4qqsrxzkjru-tu70g_tl1x...@mail.gmail.com
> [2] -
> http://www.postgresql.org/message-id/3f71da37-a17b-4961-9908-016e6323e...@postgrespro.ru
> [3] -
> http://www.postgresql.org/message-id/caa4ek1kt2e6xhvigisr5o1ac9nfo0j2wtb8n0ggd1_jklde...@mail.gmail.com
> 
> P.S. - This mail is not to point anything wrong with any particular
> individual,
> rather about the development of a particular patch getting haphazard
> because of some confusion.  I am not sure that this is the right
> thread to write about
> it, but as it has been asked here to review the patch in other related
> thread,
> so I have mentioned my thoughts on the same.

About [1] and [2]. They are slightly conflicting, but if [1] is
going to be committed I can easily use it in [2]. And it will simplify
my patch, so I have no objections here. And the same time [3] can be
significantly simplified and improved on top of [1] and [2].

For example this code from [3]:

static void
LWLockReportStat(LWLock *lock)
{
int lockid;
const char  *tranche_name;

tranche_name = T_NAME(lock);

if (strcmp(tranche_name, "main") == 0)
{
lockid = T_ID(lock);

if (lockid < NUM_INDIVIDUAL_LWLOCKS)
pgstat_report_wait_event(LWLockTypeToWaitEvent(lockid));
else if (lockid >= BUFFER_MAPPING_LWLOCK_OFFSET &&
 lockid <  LOCK_MANAGER_LWLOCK_OFFSET)
pgstat_report_wait_event(WaitEvent_BufferMappingLock);
else if (lockid >= LOCK_MANAGER_LWLOCK_OFFSET &&
 lockid <
PREDICATELOCK_MANAGER_LWLOCK_OFFSET)
pgstat_report_wait_event(WaitEvent_LockManagerLock);
else if (lockid >=
PREDICATELOCK_MANAGER_LWLOCK_OFFSET&& lockid <
NUM_FIXED_LWLOCKS)
pgstat_report_wait_event(WaitEvent_PredicateManagerLock);
else pgstat_report_wait_event(WaitEvent_OtherLWLock); }
else if (strcmp(tranche_name, "WALInsertLocks") == 0)
pgstat_report_wait_event(WaitEvent_WALInsertLocks);
else if (strcmp(tranche_name, "ReplicationOrigins") == 0)
pgstat_report_wait_event(WaitEvent_ReplicationOrigins);
else
pgstat_report_wait_event(WaitEvent_OtherTrancheLWLock);
}

can be changed to something like:

#define LWLOCK_WAIT_ID(lock) \
 (lock->tranche == 0? T_ID(lock) : lock->tranche +
 NUM_INDIVIDUAL_LWLOCKS)

static void
LWLockReportStat(LWLock *lock)
{
int 

Re: [HACKERS] about fsync in CLOG buffer write

[张广舟(明虚)]

Andres,

Thanks for the reply!

I will try the ForwardFsyncRequest-like approach.

在 15-9-2 下午8:32， "Andres Freund"  写入:

>On 2015-09-10 19:39:59 +0800, 张广舟(明虚) wrote:
>> We found there is a fsync call when CLOG buffer
>> is written out in SlruPhysicalWritePage(). It is often called when a
>>backend
>> needs to check transaction status with SimpleLruReadPage().
>
>That's when there's not enough buffers available some other, and your
>case dirty, needs to be written out.
>
>You could try increasing the max (32) in CLOGShmemBuffers() further.
>
>>  ctl->do_fsync is true for CLOG.  Question is, could we just disable
>>fsync
>> for CLOG buffer write out here? Is it safe to do so? I understand a
>> checkpoint will calls SimpleLruFlush to flush all CLOG buffer at once,
>>and
>> the fsync call here (for buffer write out) is not necessary.
>
>No, that'd not be safe. The reason we fsync in SlruPhysicalWritePage()
>is that the flush during checkpoint won't necessarily touch those files
>at all (as they've been replaced with other buffers in memory).
>
>This could be optimized though - it should be possible to delay the
>fsync for slru files that have been evicted from memory till
>checkpoint. Using something like ForwardFsyncRequest() except that it
>obviously has to be usable for other files than normal relfilenodes.
>
>Greetings,
>
>Andres Freund




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Fwd: [Snowball-discuss] New website

[Magnus Hagander]

I assume this is to replace the old site completely? If so, I think we
should just backpatch it to all supported branches -- it's basically a
bugfix as I understand it. The old site is eventually going out of business
or at least not being properly updated, so we'll eventually end up with
broken links.

Is there any reason not to do that?

//Magnus

On Mon, Sep 7, 2015 at 1:30 PM, Oleg Bartunov  wrote:

> Snowball stemmer has moved to the new site http://snowballstem.org, I
> made a simple patch for master . I'm not sure if it's ok for 9.5.
>
>
> -- Forwarded message --
> From: Olly Betts 
> Date: Mon, Sep 7, 2015 at 1:46 AM
> Subject: [Snowball-discuss] New website
> To: snowball-disc...@lists.tartarus.org
>
>
> On Wed, Mar 25, 2015 at 01:22:40AM +, Richard Boulton wrote:
> >  - The website at http://snowball.tartarus.org/ will be preserved in
> > essentially its current state, for the foreseeable future. Snowball has
> > been referenced in many academic papers and other places, so we feel it
> > important to preserve the website in essentially its current state.
>
> I have set up a new website for snowball:
>
> http://snowballstem.org/
>
> The content is forked from the old website.
>
> Currently there are some dead internal links I need to fix (to things
> like the source code of the stemmers), and the demo page isn't
> operational.  Please report any other problems.
>
> I'd encourage people to update external links to "the snowball project"
> to point to the new site.
>
> Cheers,
> Olly
>
> ___
> Snowball-discuss mailing list
> snowball-disc...@lists.tartarus.org
> http://lists.tartarus.org/mailman/listinfo/snowball-discuss
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

[HACKERS] Creating unique or "internal-use-only" column names (ColumnRef)

[Peter Moser]

Good afternoon,
is it possible to create unique column names or to give column names 
inside the parser phase that do not interfer with the outside world, 
i.e. with column names from tables or aliases given by the user.


Some short background:
I created my own from-clause-item, that gets rewritten into a sub-query. 
I do this because I want to re-use existing code as much as possible. 
The rewritten sub-query gets transformed with "transformRangeSubselect"...
Within this sub-query I need 3 columns that shouldn't interfer with 
columns from the input. We refer to them from a JOIN-ON clause and an 
ORDER-BY clause.


Example code:

ColumnRef   *ref;
ref = makeNode(ColumnRef);
ref->fields = list_make1(makeString("some_unique_name"));
ref->location = -1; /* Unknown location */

...

sb1 = makeNode(SortBy);
sb1->node = ref;

...

ssResult = makeNode(SelectStmt);
ssResult->withClause = NULL;
ssResult->fromClause = list_make1(joinExpr);
ssResult->targetList = list_make1(rtAStarWithR);  /* input = r.* */
ssResult->sortClause = list_make2(sb1, sb2);

Is there a possibility for such column names?

Thanks for your help,
Peter



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [COMMITTERS] pgsql: Move DTK_ISODOW DTK_DOW and DTK_DOY to be type UNITS rather than

[Greg Stark]

On Sun, Sep 6, 2015 at 4:14 AM, Greg Stark  wrote:
>> Also, what about ecpg's copy of the code?
>
> That I hadn't thought of. Will look at it but it's after 4am here now
> so I'll get to it tomorrow.

So the only mention of these constants I can find in ECPG is the
structures in dt_common.c. I don't see a any code using it and I'm not
clear what the purpose of having this structure is or the significance
of changing it. Is this sufficient? Is it even necessary?


diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c
b/src/interfaces/ecpg/pgtypeslib/dt_common.c
index 7fe2982..01cdfa9 100644
*** a/src/interfaces/ecpg/pgtypeslib/dt_common.c
--- b/src/interfaces/ecpg/pgtypeslib/dt_common.c
***
*** 123,130  static datetkn datetktbl[] = {
  {"dec", MONTH, 12},
  {"december", MONTH, 12},
  {"dnt", TZ, 3600}, /* Dansk Normal Tid */
! {"dow", RESERV, DTK_DOW}, /* day of week */
! {"doy", RESERV, DTK_DOY}, /* day of year */
  {"dst", DTZMOD, SECS_PER_HOUR},
  #if 0
  {"dusst", DTZ, 21600}, /* Dushanbe Summer Time */
--- 123,130 
  {"dec", MONTH, 12},
  {"december", MONTH, 12},
  {"dnt", TZ, 3600}, /* Dansk Normal Tid */
! {"dow", UNITS, DTK_DOW}, /* day of week */
! {"doy", UNITS, DTK_DOY}, /* day of year */
  {"dst", DTZMOD, SECS_PER_HOUR},
  #if 0
  {"dusst", DTZ, 21600}, /* Dushanbe Summer Time */
***
*** 206,212  static datetkn datetktbl[] = {
  {"irkst", DTZ, 32400}, /* Irkutsk Summer Time */
  {"irkt", TZ, 28800}, /* Irkutsk Time */
  {"irt", TZ, 12600}, /* Iran Time */
! {"isodow", RESERV, DTK_ISODOW}, /* ISO day of week, Sunday == 7 */
  #if 0
  isst
  #endif
--- 206,212 
  {"irkst", DTZ, 32400}, /* Irkutsk Summer Time */
  {"irkt", TZ, 28800}, /* Irkutsk Time */
  {"irt", TZ, 12600}, /* Iran Time */
! {"isodow", UNITS, DTK_ISODOW}, /* ISO day of week, Sunday == 7 */
  #if 0
  isst
  #endif


-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Creating unique or "internal-use-only" column names (ColumnRef)

[Tom Lane]

Andrew Dunstan  writes:
> On 09/07/2015 09:28 AM, Alvaro Herrera wrote:
>> This seems pretty much the same as a junk attribute, if I understand you
>> correctly.  I suggest given a look at how those work.

> Is that actually documented anywhere much?

I don't think there's much besides a code comment here and there.
Grepping for functions that touch the "resjunk" field of TargetListEntries
should give you the lay of the land.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Use pg_rewind when target timeline was switched

[Alexander Korotkov]

On Thu, Aug 20, 2015 at 9:57 AM, Michael Paquier 
wrote:

> On Wed, Jul 22, 2015 at 4:28 PM, Alexander Korotkov <
> a.korot...@postgrespro.ru> wrote:
>
>> On Wed, Jul 22, 2015 at 8:48 AM, Michael Paquier <
>> michael.paqu...@gmail.com> wrote
>>
>>> On Mon, Jul 20, 2015 at 9:18 PM, Alexander Korotkov
>>>  wrote:
>>> > attached patch allows pg_rewind to work when target timeline was
>>> switched.
>>> > Actually, this patch fixes TODO from pg_rewind comments.
>>> >
>>> >   /*
>>> >* Trace the history backwards, until we hit the target timeline.
>>> >*
>>> >* TODO: This assumes that there are no timeline switches on the
>>> target
>>> >* cluster after the fork.
>>> >*/
>>> >
>>> > This patch allows pg_rewind to handle data directory synchronization
>>> is much
>>> > more general way.
>>> For instance, user can return promoted standby to old master.
>>
>>
> +   /*
> +* Since incomplete segments are copied into next
> timelines, find the
> +* lastest timeline holding required segment.
> +*/
> +   while (private->tliIndex < targetNentries - 1 &&
> +  targetHistory[private->tliIndex].end <
> targetSegEnd)
> +   {
> +   private->tliIndex++;
> +   tli_index++;
> +   }
> It seems to me that the patch is able to handle timeline switches onwards,
> but not backwards and this is what would be required to return a promoted
> standby, that got switched to let's say timeline 2 when promoted, to an old
> master, that is still on timeline 1. This code actually fails when scanning
> for the last checkpoint before WAL forked as it will be on the timeline 1
> of the old master. Imagine for example that the WAL has forked at
> 0/30X which is saved in segment 00020003 (say 2/0/3) on
> the promoted standby, and that the last checkpoint record is on 0/20X,
> which is part of 00010002 (1/0/2). I think that we should
> scan 2/0/3 (not the partial segment 1/0/3), and then 1/0/2 when looking for
> the last checkpoint record. Hence the startup index TLI should be set to
> the highest timeline and should be decremented depending on what is in the
> history file.
> The code above looks correct to me when scanning the WAL history onwards
> though, which is what is done when extracting the page map, but not
> backwards when we try to find the last common checkpoint record. This code
> actually fails trying to open 2/0/2 that does not exist in the promoted
> standby's pg_xlog in my test case.
>
> Attached is a small script I have used to reproduce the failure.
>

Right, thanks! It should be fixed in the attached version of patch.

I think that the documentation needs a brush up as well to outline the fact
> that pg_rewind would be able to put back as well a standby in a cluster,
> after for example an operator mistake when promoting a node that should not
> be.
>

Yes. But from current pg_rewind documentation I can't figure out that it
can't do so. However, we can add an explicit statement which claims that it
can.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


pg-rewind-target-switch-3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Freeze avoidance of very large table.

[Masahiko Sawada]

On Sat, Sep 5, 2015 at 7:35 AM, Simon Riggs  wrote:
> On 3 September 2015 at 18:23, Masahiko Sawada  wrote:
>
>>
>> The previous patch lacks some files for regression test.
>> Attached fixed v12 patch.
>
>
> This looks OK. You saw that I was proposing to solve this problem a
> different way ("Summary of plans to avoid the annoyance of Freezing"),
> suggesting that we wait for a few CFs to see if a patch emerges for that -
> then fall back to this patch if it doesn't? So I am moving this patch to
> next CF.
>
> I apologise for the personal annoyance caused by this; I hope whatever
> solution we find we can work together on it.
>

I had missed that thread actually, but have understood status of
around freeze avoidance topic.
It's no problem to me that we address Heikki's solution at first and
next is other plan(maybe frozen map).
But this frozen map patch is still under the reviewing and might have
serious problem, that is still need to be reviewed.
So I think we should continue to review this patch at least, while
reviewing Heikki's solution, and then we can select solution for
frozen map.
Otherwise, if frozen map has serious problem or other big problem is
occurred, the reviewing of patch will be not enough, and then it will
leads bad result, I think.

Regards,

--
Masahiko Sawada


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] LLVM Address Sanitizer (ASAN) and valgrind support

[Greg Stark]

I feel like I remember hearing about this before but I can't find any
mention of it in my mail archives. It seems pretty simple to add
support for LLVM's Address Sanitizer (asan) by using the hooks we
already have for valgrind.

In fact I think this would actually be sufficient. I'm not sure what
the MEMPOOL valgrind stuff is though. I don't think it's relevant to
address sanitizer which only tracks references to free'd or
unallocated pointers.

I don't even see any need offhand for a configure flag or autoconf
test. We could have a configure flag just to be consistent with
valgrind but it seems pointless. If you're compiling with asan I don't
see any reason to not use it. I'm building this to see if it works
now.

Incidentally there's another sanitizer called msan that looks even
more promising. It's more like valgrind in that it tracks undefined
memory. It's not working for me though and I haven't spent much time
trying to figure out why yet.


diff --git a/src/include/utils/memdebug.h b/src/include/utils/memdebug.h
index 608facc..7696986 100644
--- a/src/include/utils/memdebug.h
+++ b/src/include/utils/memdebug.h
@@ -19,6 +19,27 @@

 #ifdef USE_VALGRIND
 #include 
+
+#elif __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+
+#include 
+
+#define VALGRIND_MAKE_MEM_DEFINED(addr, size) \
+ ASAN_UNPOISON_MEMORY_REGION(addr, size)
+
+#define VALGRIND_MAKE_MEM_NOACCESS(addr, size) \
+ ASAN_POISON_MEMORY_REGION(addr, size)
+
+#define VALGRIND_MAKE_MEM_UNDEFINED(addr, size) \
+ ASAN_UNPOISON_MEMORY_REGION(addr, size)
+
+#define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0)
+#define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0)
+#define VALGRIND_DESTROY_MEMPOOL(context) do {} while (0)
+#define VALGRIND_MEMPOOL_ALLOC(context, addr, size) do {} while (0)
+#define VALGRIND_MEMPOOL_FREE(context, addr) do {} while (0)
+#define VALGRIND_MEMPOOL_CHANGE(context, optr, nptr, size) do {} while (0)
+
 #else
 #define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0)
 #define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0)


-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Access method extendability

[Petr Jelinek]

On 2015-09-07 17:41, Teodor Sigaev wrote:

Some notices:

1) create-am.3.patch.gz
   As I understand, you didn't add schema name to access method. Why?
Suppose, if we implement SQL-like interface for AM screation/dropping
then we should provide a schema qualification for them


Why would access methods need to be schema qualified?

--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: Implement failover on libpq connect level.

[Daniel Verite]

Victor Wagner wrote:

> It would just take a bit more time for client and a bit more load for
> server - to make sure that this connection is read-write by
> issuing
> 
>show transaction_read_only 
> 
> statement before considering connection useful.

If the purpose of the feature is to wait for a failover to complete,
shouldn't it check for pg_is_in_recovery() rather than 
 transaction_read_only ?

That's because a database or user can be made read-only-on-connect 
on an otherwise read-write instance by issuing
  ALTER DATABASE dbname SET default_transaction_read_only TO on;
The same for a user with ALTER USER.

In that case,  transaction_read_only  would be OFF after connecting,
both on the master and on a slave, independantly of any failover
in progress or finished or not having occurred at all.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Access method extendability

[Teodor Sigaev]

Some notices:

1) create-am.3.patch.gz
  As I understand, you didn't add schema name to access method. Why? Suppose, 
if we implement SQL-like interface for AM screation/dropping then we should 
provide a schema qualification for them


2) create-am.3.patch.gz get_object_address_am()
+   switch (list_length(objname))
...
+   case 2:
+   catalogname = strVal(linitial(objname));
+   amname = strVal(lthird(objname));
^^ seems, it should be lsecond()
3) create-am.3.patch.gz
 Suppose, RM_GENERIC_ID is part of generic-xlog.3.patch.gz

4) Makefile(s)
As I can see, object files are lexicographically ordered

5) gindesc.c -> genericdesc.c in file header

6) generic-xlog.3.patch.gz
I don't like an idea to split START_CRIT_SECTION and END_CRIT_SECTION to 
GenericXLogStart and GenericXLogFinish. User's code could throw a error or 
allocate memory between them and error will become a panic.



--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] Microvacuum for gist.

[Anastasia Lubennikova]

04.09.2015 15:11, Teodor Sigaev:

Some notices

1 gistvacuumpage():
OffsetNumber deletable[MaxOffsetNumber];
  Seems, MaxOffsetNumber is too much, MaxIndexTuplesPerPage is enough


Fixed.


2 Loop in gistkillitems() for searching heap pointer. It seems to me that
it could be a performance problem. To fix it, it's needed to remember 
index tuple's offset number somewhere near 
GISTScanOpaqueData->killedItems. And
gistkillitems() will loop over offsets and compare heap pointer from 
killedItems and index tuple, if they doesn't match then just skip this 
index tuple.
Thanks for suggestion. I've rewritten this function. Now killedItems[] 
contains only OffsetNumbers of tuples which we are going to delete. 
PageLSN check is enough to ensure that nothing has changed on the page. 
Heap pointer recheck is unnecessary. (It's really important for btree, 
where tuple could be inserted in the middle of page. But we can't have 
such situation for GiST index page).

It works 50% faster than before.


3 Connected with previous, could you show some performance tests?


Perfomance test is attached.
Test is following - portion of tuples is deleted and after that selected 
several times.


Without microvacuum. All 'select' queries are executed at about same time
Time: 360,468 ms
Time: 243,197 ms
Time: 238,310 ms
Time: 238,018 ms

With microvacuum. First 'select' invokes gistkillitems(). It's executed 
a bit slower than before.
But following queries are executed significantly faster than without 
microvacuum.

Time: 368,780 ms
Time: 69,769 ms
Time: 9,545 ms
Time: 12,427 ms

Please, review the patch again. I could have missed something.

P.S. Do I need to write any documentation update?

--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

*** a/src/backend/access/gist/gist.c
--- b/src/backend/access/gist/gist.c
***
*** 36,41  static bool gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
--- 36,42 
   bool unlockbuf, bool unlockleftchild);
  static void gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack,
  GISTSTATE *giststate, List *splitinfo, bool releasebuf);
+ static void gistvacuumpage(Relation rel, Page page, Buffer buffer);
  
  
  #define ROTATEDIST(d) do { \
***
*** 209,214  gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate,
--- 210,226 
  	 * because the tuple vector passed to gistSplit won't include this tuple.
  	 */
  	is_split = gistnospace(page, itup, ntup, oldoffnum, freespace);
+ 
+ 	/*
+ 	 * If leaf page is full, try at first to delete dead tuples. And then
+ 	 * check again.
+ 	 */
+ 	if (is_split && GistPageIsLeaf(page) && GistPageHasGarbage(page))
+ 	{
+ 		gistvacuumpage(rel, page, buffer);
+ 		is_split = gistnospace(page, itup, ntup, oldoffnum, freespace);
+ 	}
+ 
  	if (is_split)
  	{
  		/* no space for insertion */
***
*** 1440,1442  freeGISTstate(GISTSTATE *giststate)
--- 1452,1523 
  	/* It's sufficient to delete the scanCxt */
  	MemoryContextDelete(giststate->scanCxt);
  }
+ 
+ /*
+  * gistvacuumpage() -- try to remove LP_DEAD items from the given page.
+  */
+ static void
+ gistvacuumpage(Relation rel, Page page, Buffer buffer)
+ {
+ 	OffsetNumber deletable[MaxOffsetNumber];
+ 	int			ndeletable = 0;
+ 	OffsetNumber offnum,
+ minoff,
+ maxoff;
+ 
+ 	/*
+ 	 * Scan over all items to see which ones need to be deleted according to
+ 	 * LP_DEAD flags.
+ 	 */
+ 	maxoff = PageGetMaxOffsetNumber(page);
+ 	for (offnum = FirstOffsetNumber;
+ 		 offnum <= maxoff;
+ 		 offnum = OffsetNumberNext(offnum))
+ 	{
+ 		ItemId		itemId = PageGetItemId(page, offnum);
+ 
+ 		if (ItemIdIsDead(itemId))
+ 			deletable[ndeletable++] = offnum;
+ 	}
+ 
+ 	if (ndeletable > 0)
+ 	{
+ 		START_CRIT_SECTION();
+ 
+ 		PageIndexMultiDelete(page, deletable, ndeletable);
+ 
+ 		/*
+ 		 * Mark the page as not containing any LP_DEAD items.  This is not
+ 		 * certainly true (there might be some that have recently been marked,
+ 		 * but weren't included in our target-item list), but it will almost
+ 		 * always be true and it doesn't seem worth an additional page scan to
+ 		 * check it. Remember that F_HAS_GARBAGE is only a hint anyway.
+ 		 */
+ 		GistClearPageHasGarbage(page);
+ 
+ 		MarkBufferDirty(buffer);
+ 
+ 		/* XLOG stuff */
+ 		if (RelationNeedsWAL(rel))
+ 		{
+ 			XLogRecPtr	recptr;
+ 
+ 			recptr = gistXLogUpdate(rel->rd_node, buffer,
+ 	deletable, ndeletable,
+ 	NULL, 0, InvalidBuffer);
+ 
+ 			PageSetLSN(page, recptr);
+ 		}
+ 		else
+ 			PageSetLSN(page, gistGetFakeLSN(rel));
+ 
+ 		END_CRIT_SECTION();
+ 	}
+ 
+ 	/*
+ 	 * Note: if we didn't find any LP_DEAD items, then the page's
+ 	 * F_HAS_GARBAGE hint bit is falsely set.  We do not bother expending a
+ 	 * separate write to clear it, however.  We will clear it when we split
+ 	 * the page.
+ 	 */
+ }
*** a/src/backend/access/gist/gistget.c
--- 

Re: [HACKERS] Creating unique or "internal-use-only" column names (ColumnRef)

[Andrew Dunstan]

On 09/07/2015 09:28 AM, Alvaro Herrera wrote:

Peter Moser wrote:

Good afternoon,
is it possible to create unique column names or to give column names inside
the parser phase that do not interfer with the outside world, i.e. with
column names from tables or aliases given by the user.

Some short background:
I created my own from-clause-item, that gets rewritten into a sub-query. I
do this because I want to re-use existing code as much as possible. The
rewritten sub-query gets transformed with "transformRangeSubselect"...
Within this sub-query I need 3 columns that shouldn't interfer with columns
from the input. We refer to them from a JOIN-ON clause and an ORDER-BY
clause.

This seems pretty much the same as a junk attribute, if I understand you
correctly.  I suggest given a look at how those work.





Is that actually documented anywhere much? I had to use one recently for 
the Redis FDW and looked in vain for some docco - not saying it's not 
there, just that I didn't find it.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Creating unique or "internal-use-only" column names (ColumnRef)

[Alvaro Herrera]

Peter Moser wrote:
> Good afternoon,
> is it possible to create unique column names or to give column names inside
> the parser phase that do not interfer with the outside world, i.e. with
> column names from tables or aliases given by the user.
> 
> Some short background:
> I created my own from-clause-item, that gets rewritten into a sub-query. I
> do this because I want to re-use existing code as much as possible. The
> rewritten sub-query gets transformed with "transformRangeSubselect"...
> Within this sub-query I need 3 columns that shouldn't interfer with columns
> from the input. We refer to them from a JOIN-ON clause and an ORDER-BY
> clause.

This seems pretty much the same as a junk attribute, if I understand you
correctly.  I suggest given a look at how those work.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Access method extendability

[Alexander Korotkov]

On Tue, Sep 1, 2015 at 6:50 PM, Teodor Sigaev  wrote:

> In general pattern of generic WAL usage is following.
>>
>> 1) Start using generic WAL: specify relation
>>
>
> M-m, what about extensions which wants to use WAL but WAL record doesn't
> connected to any relation? For example, transaction manager or kind of FDW.
>
>
>> GenericXLogStart(index);
>>
>> 2) Register buffers
>>
>> GenericXLogRegister(0, buffer1, false);
>> GenericXLogRegister(1, buffer2, true);
>>
>> first argument is a slot number, second is the buffer, third is flag
>> indicating
>> new buffer
>>
>
> Why do we need a slot number? to replace already registered buffer?


For further simplification slot number could be omitted. In the attached
patches, generic xlog replay applies changes in the same order
GenericXLogRegister was called.
Patches was rebased against latest version of am interface rework patch.
http://www.postgresql.org/message-id/CAPpHfduGY=KZSBPZN5+USTXev-9M2PAUp3Yi=syfdo2n244...@mail.gmail.com

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


create-am.3.patch.gz
Description: GNU Zip compressed data


generic-xlog.3.patch.gz
Description: GNU Zip compressed data


bloom-contrib.3.patch.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Move DTK_ISODOW DTK_DOW and DTK_DOY to be type UNITS rather than

[Tom Lane]

Greg Stark  writes:
>>> Also, what about ecpg's copy of the code?

> So the only mention of these constants I can find in ECPG is the
> structures in dt_common.c. I don't see a any code using it and I'm not
> clear what the purpose of having this structure is or the significance
> of changing it. Is this sufficient? Is it even necessary?

Hm.  It does look like these entries might be dead code so far as ecpg
is concerned.  I'd still make the changes you illustrate, just to avoid
having the ecpg code diverge further than necessary from the backend.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] synchronous_commit = apply

[Thomas Munro]

[Combining replies to emails from different authors into one message]

On Wed, Sep 2, 2015 at 2:21 PM, Jaime Casanova <
jaime.casan...@2ndquadrant.com> wrote:

> On 1 September 2015 at 20:25, Thomas Munro 
> wrote:

> As a quick weekend learning exercise/hack I recently went looking into how
> > we could support $SUBJECT.  I discovered we already report the apply
> > progress back to the master, and the synchronous waiting facility seemed
> to
> > be all ready to support this.  In fact it seemed a little too easy so
> > something tells me it must be wrong!  But anyway, please see the attached
> > toy POC patch which does that.
>
> i haven't seen the patch, but probably is as easy as you see it...
> IIRC, Simon proposed a patch for this a few years ago and this was
> actually contempleted from the beggining in the design of SR.
>

Ah,  thanks, that certainly explains that.  The source code practically had
big arrows pointing to the place to type.  I don't want to step on anyone's
toes, so if Simon or anyone else is actively working on this, please let me
know, I'll happily cease and desist.


On Thu, Sep 3, 2015 at 12:35 AM, Robert Haas  wrote:

> On Tue, Sep 1, 2015 at 9:25 PM, Thomas Munro
>  wrote:
> > The next problem is that the master can be waiting quite a long time for
> a
> > reply from the remote walreceiver containing the desired apply LSN: in
> the
> > best case it learns of apply progress from replies to subsequent
> unrelated
> > records (which might be very soon on a busy system but still involves
> > waiting for the next transaction's WAL flush), and in the worst case it
> > needs to wait for wal_receiver_status_interval (10 seconds by default),
> > which makes for a long COMMIT delay.  I was thinking that the solution to
> > that may be to teach StartupLOG to signal the walreceiver after it
> updates
> > XLogCtl->lastReplayedEndRecPtr, which should cause walrcv_receive to be
> > interrupted and return early, and then walreceiver could send a reply if
> it
> > sees that lastReplayedEndRecPtr has moved.  Maybe that would generate an
> > unacceptably high frequency of signals, and maybe there is a better form
> of
> > IPC for this.
>
> Yeah, that could be a problem, as could reply volume. If you've got a
> bunch of heap inserts of narrow rows into some table, you don't really
> want to send a reply after each one.  That would be a lot of replies,
> and nobody can really care about them anyway, at least not for
> synchronous_commit purposes.  But what if you only sent a signal when
> the just-replayed record was a COMMIT record?  I suppose that could
> still be a lot of replies on something like a full-tilt pgbench
> workload, but even in that case it would help a lot.
>

Here's a version that does that.  It's still ugly POC code for now -- the
flow control in walreceiver.c probably needs a bit of refactoring so it
doesn't have to do the same work in two different places, and it needs some
thought about how it balances time spent write wal and sending replies.
But ... it seems to work for simple tests.

I have also attached a test program.  Here are some numbers I measured with
master and standby running on my laptop using that program:

synchronous_commit  loops  Time   TPS
off 1  0.841s 11890
local   1  1.869s  5350
remote_write1  3.123s  3202
on  1  3.085s  3241
apply   1  3.361s  2975

If you run it with "--check" you can see that the changes are not always
immediately visible in anything below "apply" and are always visible in
"apply".  (I can't explain why "on" consistently beats "remote_write" on my
machine by a small margin...  Maybe something to do with being an assert
build.)


On Thu, Sep 3, 2015 at 12:02 AM, Fujii Masao  wrote:
>
> One idea is to change the standby so that it manages the locations
> that the backends in "apply" mode are waiting for in the master,
> and to make the startup process wake the walreceiver up whenever
> the replay location reaches either of those locations. In this idea,
> walreceiver sends back the "apply" location to the master only when
> needed.
>

Hmm.  So maybe commit records could have a flag saying 'someone is waiting
for this to commit to apply', and the startup process's apply loop would
only bother to signal the walreceiver if it sees that flag.  I will try
that.

-- 
Thomas Munro
http://www.enterprisedb.com


synchronous-commit-apply-v2.patch
Description: Binary data
#include 

#include 
#include 
#include 
#include 
#include 

int
main(int argc, char *argv[])
{
	PGconn *master;
	PGconn *standby;
	PGresult *result;
	int i;
	int loops = 1;
	char buffer[1024];
	const char *level = "off";
	bool check_applied = false;

	for (i = 1; i != argc; ++i)
	{
		bool more = (i < argc - 1);

		if (strcmp(argv[i], "--check") == 0)
			check_applied = true;
		else if 

Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers

[Alvaro Herrera]

Andres Freund wrote:

> The buffer replacement algorithm for clog is rather stupid - I do wonder
> where the cutoff is that it hurts.
> 
> Could you perhaps try to create a testcase where xids are accessed that
> are so far apart on average that they're unlikely to be in memory? And
> then test that across a number of client counts?
> 
> There's two reasons that I'd like to see that: First I'd like to avoid
> regression, second I'd like to avoid having to bump the maximum number
> of buffers by small buffers after every hardware generation...

I wonder if it would make sense to explore an idea that has been floated
for years now -- to have pg_clog pages be allocated as part of shared
buffers rather than have their own separate pool.  That way, no separate
hardcoded allocation limit is needed.  It's probably pretty tricky to
implement, though :-(

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Making tab-complete.c easier to maintain

[Alvaro Herrera]

Thomas Munro wrote:

> Thanks, good point.  Here's a version that uses NULL via a macro ANY.
> Aside from a few corrections it also now distinguishes between
> TAIL_MATCHESn (common) and MATCHESn (rarely used for now), for example:

This looks pretty neat -- 100x neater than what we have, at any rate.  I
would use your new MATCHESn() macros a bit more -- for instance the
completion for "ALTER but not ALTER after ALTER TABLE" could be
rephrased as simply MATCHES1("ALTER"), i.e. have it match at start of
command only.  Maybe that's just a matter of going over the new code
after the initial run, so that we can have a first patch that's mostly
mechanical and a second pass in which more semantically relevant changes
are applied.  Seems easier to review ...

I would use "ANY" as a keyword here.  Sounds way too generic to me.
Maybe "CompleteAny" or something like that.

Stylistically, I find there's too much uppercasing here.  Maybe rename the
macros like this instead:

> +   else if (TailMatches4("ALL", "IN", "TABLESPACE", ANY))
> +   CompleteWithList2("SET TABLESPACE", "OWNED BY");

Not totally sure about this part TBH.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Improving test coverage of extensions with pg_dump

[Michael Paquier]

On Thu, Sep 3, 2015 at 10:35 AM, Michael Paquier
 wrote:
> On Thu, Sep 3, 2015 at 12:38 AM, Andres Freund wrote:
>> Isn't a full test with a separate initdb, create extension etc. a really
>> heavyhanded way to test this? I mean that's a test where the setup takes
>> up to 10s, whereas the actual runtime is in the millisecond range?
>>
>> Adding tests in this manner doesn't seem scalable to me.
>
> How to include such kind of tests is in the heart of the discussion
> since this patch has showed up. I think we are now close to 5
> different opinions with 4 different hackers on the matter, the Riggs'
> theorem applies nicely.
>
>> I think we should rather add *one* test that does dump/restore over the
>> normal regression test database. Something similar to the pg_upgrade
>> tests. And then work at adding more content to the regression test
>> database - potentially sourcing from src/test/modules.
>
> If you are worrying about run time, pg_upgrade already exactly does
> that. What would be the point to double the amount of time to do the
> same thing in two different places?

So, to summarize the state of this patch whose status is now "Waiting
on Author", we have the following possibilities:
1) Keep the test as-is, as an independent test of src/test/modules.
2) Integrate it in the test suite of src/test/regress and let
pg_upgrade make the work with dump/restore.
3) Create a new facility as src/test/modules/extensions that does a
run of the regression test suite with a set of extensions, and then a
dump/restore. For now the only extension added in the installation is
tables_fk. Future ones would be added here as well, though it is not
clear to me what they are and if we'd have some (there may be... Or
not).
4) Include it as a test of pg_dump in src/bin/pg_dump/t, with the
extension located in for example t/extensions and extend prove_check
to install as well any extensions in t/extensions.

I would still go for 1), the infrastructures included by the other
proposals may become useful depending on the types of tests that are
added in the future, but it is still unclear what those tests are, and
they may even need something else than what listed here (see that as
an example http://www.postgresql.org/message-id/54f62c3f.8070...@gmx.net)
to run properly.
Regards,
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers

[Andres Freund]

Hi,

On 2015-09-07 10:34:10 -0300, Alvaro Herrera wrote:
> I wonder if it would make sense to explore an idea that has been floated
> for years now -- to have pg_clog pages be allocated as part of shared
> buffers rather than have their own separate pool.  That way, no separate
> hardcoded allocation limit is needed.  It's probably pretty tricky to
> implement, though :-(

I still think that'd be a good plan, especially as it'd also let us use
a lot of related infrastructure. I doubt we could just use the standard
cache replacement mechanism though - it's not particularly efficient
either... I also have my doubts that a hash table lookup at every clog
lookup is going to be ok performancewise.

The biggest problem will probably be that the buffer manager is pretty
directly tied to relations and breaking up that bond won't be all that
easy. My guess is that the best bet here is that the easiest way to at
least explore this is to define pg_clog/... as their own tablespaces
(akin to pg_global) and treat the files therein as plain relations.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Separating Buffer LWlocks

[Andres Freund]

On 2015-09-06 15:28:40 +0200, Andres Freund wrote:
> Hm. I found that the buffer content lwlocks can actually also be a
> significant source of contention - I'm not sure reducing padding for
> those is going to be particularly nice. I think we should rather move
> the *content* lock inline into the buffer descriptor. The io lock
> doesn't matter and can be as small as possible.

POC patch along those lines attached. This way the content locks have
full 64byte alignment *without* any additional memory usage because
buffer descriptors are already padded to 64bytes.  I'd to reorder
BufferDesc contents a bit and reduce the width of usagecount to 8bit
(which is fine given that 5 is our highest value) to make enough room.

I've experimented reducing the padding of the IO locks to nothing since
they're not that often contended on the CPU level. But even on my laptop
that lead to a noticeable regression for a readonly pgbench workload
where the dataset fit into the OS page cache but not into s_b.

> Additionally I think we should increase the lwlock padding to 64byte
> (i.e. the by far most command cacheline size). In the past I've seen
> that to be rather beneficial.

You'd already done that...


Benchmarking this on my 4 core/8 threads laptop I see a very slight
performance increase - which is about what we expect since this really
only should affect multi-socket machines.

Greetings,

Andres Freund
>From 55bc6e368206be230a690394ab541105f22a9827 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 7 Sep 2015 19:57:40 +0200
Subject: [PATCH] lwlock-andres-v3

---
 src/backend/storage/buffer/buf_init.c | 52 +++
 src/backend/storage/buffer/bufmgr.c   | 52 +--
 src/backend/storage/lmgr/lwlock.c | 12 +---
 src/include/storage/buf_internals.h   | 15 ++
 src/include/storage/lwlock.h  | 17 
 5 files changed, 102 insertions(+), 46 deletions(-)

diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c
index 3ae2848..14a2707 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -21,6 +21,16 @@
 BufferDescPadded *BufferDescriptors;
 char	   *BufferBlocks;
 
+/*
+ * This points to the buffer LWLocks in shared memory.  Backends inherit
+ * the pointer by fork from the postmaster (except in the EXEC_BACKEND case,
+ * where we have special measures to pass it down).
+ */
+const int BufferIOLWLockTrancheId = 1;
+LWLockTranche BufferIOLWLockTranche;
+LWLockMinimallyPadded *BufferIOLWLockArray = NULL;
+const int BufferContentLWLockTrancheId = 2;
+LWLockTranche BufferContentLWLockTranche;
 
 /*
  * Data Structures:
@@ -65,7 +75,8 @@ void
 InitBufferPool(void)
 {
 	bool		foundBufs,
-foundDescs;
+foundDescs,
+foundIOLocks;
 
 	/* Align descriptors to a cacheline boundary. */
 	BufferDescriptors = (BufferDescPadded *) CACHELINEALIGN(
@@ -77,10 +88,25 @@ InitBufferPool(void)
 		ShmemInitStruct("Buffer Blocks",
 		NBuffers * (Size) BLCKSZ, );
 
-	if (foundDescs || foundBufs)
+	BufferIOLWLockArray = (LWLockMinimallyPadded *)
+		ShmemInitStruct("Buffer IO Locks",
+		NBuffers * (Size) sizeof(LWLockMinimallyPadded), );
+
+	BufferIOLWLockTranche.name = "buffer-io";
+	BufferIOLWLockTranche.array_base = BufferIOLWLockArray;
+	BufferIOLWLockTranche.array_stride = sizeof(LWLockMinimallyPadded);
+	LWLockRegisterTranche(1, );
+
+	BufferContentLWLockTranche.name = "buffer-content";
+	BufferContentLWLockTranche.array_base =
+		((char *) BufferDescriptors) + offsetof(BufferDesc, content_lock);
+	BufferContentLWLockTranche.array_stride = sizeof(BufferDescPadded);
+	LWLockRegisterTranche(2, );
+
+	if (foundDescs || foundBufs || foundIOLocks)
 	{
 		/* both should be present or neither */
-		Assert(foundDescs && foundBufs);
+		Assert(foundDescs && foundBufs && foundIOLocks);
 		/* note: this path is only taken in EXEC_BACKEND case */
 	}
 	else
@@ -110,12 +136,21 @@ InitBufferPool(void)
 			 */
 			buf->freeNext = i + 1;
 
-			buf->io_in_progress_lock = LWLockAssign();
-			buf->content_lock = LWLockAssign();
+			LWLockInitialize(BufferDescriptorGetContentLock(buf),
+			 BufferContentLWLockTrancheId);
 		}
 
 		/* Correct last entry of linked list */
 		GetBufferDescriptor(NBuffers - 1)->freeNext = FREENEXT_END_OF_LIST;
+
+		/* Initialize all buffer IO LWLocks  */
+		for (i = 0; i < NBuffers; i++)
+		{
+			BufferDesc *buf = GetBufferDescriptor(i);
+
+			LWLockInitialize(BufferDescriptorGetIOLock(buf),
+			 BufferIOLWLockTrancheId);
+		}
 	}
 
 	/* Init other shared buffer-management stuff */
@@ -144,5 +179,12 @@ BufferShmemSize(void)
 	/* size of stuff controlled by freelist.c */
 	size = add_size(size, StrategyShmemSize());
 
+	/*
+	 * Space for the io in progress LWLock array, allocated in a separate
+	 * tranche, with a different stride. This allows us to pad out the main
+	 * array to reduce contention, without wasting space 

Re: [HACKERS] WIP: Rework access method interface

[Petr Jelinek]

On 2015-09-04 16:26, Alexander Korotkov wrote:


Attached patch is implementing this. It doesn't pretend to be fully
correct implementation, but it should be enough for proof the concept.
In this patch access method exposes another function: amvalidate. It
takes data structure representing opclass and throws error if it finds
it invalid.
This method is used on new opclass definition (alter operator family
etc. are not yet implemented but planned). Also, there is SQL function
validate_opclass(oid) which is used in regression tests.
Any thoughts?



This is starting to look good.

However I don't like the naming differences between validate_opclass and 
amvalidate. If you expect that the current amvalidate will only be used 
for opclass validation then it should be renamed accordingly.


Also GetIndexAmRoutine should check the return type of the amhandler.

--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: contrib/sslinfo: add ssl_extension_info SRF

[Michael Paquier]

On Tue, Sep 8, 2015 at 9:24 AM, Alvaro Herrera  wrote:
> contrib/sslinfo: add ssl_extension_info SRF
>
> This new function provides information about SSL extensions present in
> the X509 certificate used for the current connection.
>
> Extension version updated to version 1.1.

Nitpicky note:
s/Do when there is no more left/Done when there is no more left/
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pageinspect patch, for showing tuple data

[Michael Paquier]

On Tue, Sep 8, 2015 at 11:53 AM, Michael Paquier wrote:
> Honestly, heap_page_item_attrs and heap_page_item_detoast_attrs are way too
> similar to what heap_page_items does, leading to a code maze that is going
> to make future extensions more difficult, which is what lead to the
> refactoring your patch does.
> Hence, instead of all those complications, why not simply introducing two
> functions that take as input the tuple data and the OID of the relation,
> returning those bytea arrays? It seems to be that this would be a more handy
> interface, and as this is for educational purposes I guess that the addition
> of the overhead induced by the extra function call would be acceptable.

Actually not two functions, but just one, with an extra flag to be
able to enforce detoasting on the field values where this can be done.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] On-demand running query plans using auto_explain and signals

[Pavel Stehule]

2015-09-07 11:55 GMT+02:00 Shulgin, Oleksandr 
:

> On Fri, Sep 4, 2015 at 6:11 AM, Pavel Stehule 
> wrote:
>
>> Sorry, but I still don't see how the slots help this issue - could you
>>> please elaborate?
>>>
>> with slot (or some similiar) there is not global locked resource. If I'll
>> have a time at weekend I'll try to write some prototype.
>>
>
> But you will still lock on the slots list to find an unused one.  How is
> that substantially different from what I'm doing?
>

It is not necessary - you can use similar technique to what it does PGPROC.
I am sending "lock free" demo.

I don't afraid about locks - short locks, when the range and time are
limited. But there are lot of bugs, and fixes with the name "do
interruptible some", and it is reason, why I prefer typical design for work
with shared memory.


> >> Other smaller issues:
>>> >>
>>> >> * probably sending line by line is useless - shm_mq_send can pass
>>> bigger data when nowait = false
>>>
>>> I'm not sending it like that because of the message size - I just find
>>> it more convenient. If you think it can be problematic, its easy to do this
>>> as before, by splitting lines on the receiving side.
>>>
>> Yes, shm queue sending data immediately - so slicing on sender generates
>> more interprocess communication
>>
>
> Well, we are talking about hundreds to thousands bytes per plan in total.
> And if my reading of shm_mq implementation is correct, if the message fits
> into the shared memory buffer, the receiver gets the direct pointer to the
> shared memory, no extra allocation/copy to process-local memory.  So this
> can be actually a win.
>

you have to calculate with signals and interprocess communication. the cost
of memory allocation is not all.

> >> * pg_usleep(1000L); - it is related to single point resource
>>>
>>> But not a highly concurrent one.
>>>
>> I believe so it is not becessary - waiting (sleeping) can be deeper in
>> reading from queue - the code will be cleaner
>>
>
> The only way I expect this line to be reached is when a concurrent
> pg_cmdstatus() call is in progress: the receiving backend has set the
> target_pid and has created the queue, released the lock and now waits to
> read something from shm_mq.  So the backend that's trying to also use this
> communication channel can obtain the lwlock, checks if the channel is not
> used at the time, fails and then it needs to check again, but that's going
> to put a load on the CPU, so there's a small sleep.
>
> The real problem could be if the process that was signaled to connect to
> the message queue never handles the interrupt, and we keep waiting forever
> in shm_mq_receive().  We could add a timeout parameter or just let the user
> cancel the call: send a cancellation request, use pg_cancel_backend() or
> set statement_timeout before running this.
>

This is valid question - for begin we can use a statement_timeout and we
don't need to design some special (if you don't hold some important lock).

My example (the code has prototype quality) is little bit longer, but it
work without global lock - the requester doesn't block any other

Pavel


>
> --
> Alex
>
>
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
new file mode 100644
index 32ac58f..f5db55a
*** a/src/backend/storage/ipc/ipci.c
--- b/src/backend/storage/ipc/ipci.c
***
*** 43,48 
--- 43,49 
  #include "storage/procsignal.h"
  #include "storage/sinvaladt.h"
  #include "storage/spin.h"
+ #include "utils/signal_demo.h"
  
  
  shmem_startup_hook_type shmem_startup_hook = NULL;
*** CreateSharedMemoryAndSemaphores(bool mak
*** 139,144 
--- 140,146 
  		size = add_size(size, BTreeShmemSize());
  		size = add_size(size, SyncScanShmemSize());
  		size = add_size(size, AsyncShmemSize());
+ 		size = add_size(size, ProcPipeShmemSize());
  #ifdef EXEC_BACKEND
  		size = add_size(size, ShmemBackendArraySize());
  #endif
*** CreateSharedMemoryAndSemaphores(bool mak
*** 243,248 
--- 245,251 
  	ReplicationOriginShmemInit();
  	WalSndShmemInit();
  	WalRcvShmemInit();
+ 	ProcPipeShmemInit();
  
  	/*
  	 * Set up other modules that need some shared memory space
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
new file mode 100644
index 0abde43..c28b44f
*** a/src/backend/storage/ipc/procsignal.c
--- b/src/backend/storage/ipc/procsignal.c
***
*** 27,32 
--- 27,33 
  #include "storage/sinval.h"
  #include "tcop/tcopprot.h"
  
+ #include "utils/signal_demo.h"
  
  /*
   * The SIGUSR1 signal is multiplexed to support signalling multiple event
*** bool		set_latch_on_sigusr1;
*** 69,75 
  
  static ProcSignalSlot *ProcSignalSlots = NULL;
  static volatile ProcSignalSlot *MyProcSignalSlot = NULL;
! 
  static bool CheckProcSignal(ProcSignalReason reason);
  static void CleanupProcSignalState(int status, Datum arg);
  

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Alvaro Herrera]

Andrew Dunstan wrote:

> I already gave a use case that you dismissed in favour of a vague solution
> that we don't actually have. You seem to be the only person objecting to
> this proposal.

I think that use case would be better served by a completely different
interface -- some way to query the server, "does this installation
support feature X?"  What you proposed, using a regexp to look for
--enable-xml in the pg_config --configure output, doesn't look all that
nice to me.

For instance, we already have sql_features.txt, which is exposed as a
table in the docs.  It's not quite the same thing (because what you want
is not the same as conformance to the SQL standard), but I think a
system view that has a list of features and a boolean flag for each one
is more practical (though admittedly it's more work to implement.)
Another alternative which I think is simpler is a read-only GUC, which
we already have for a number of compile-time properties.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Proposal: Implement failover on libpq connect level.

[Victor Wagner]

В Mon, 07 Sep 2015 17:32:48 +0200
"Daniel Verite"  пишет:

>   Victor Wagner wrote:
> 
> > It would just take a bit more time for client and a bit more load
> > for server - to make sure that this connection is read-write by
> > issuing
> > 
> >show transaction_read_only 
> > 
> > statement before considering connection useful.
> 
> If the purpose of the feature is to wait for a failover to complete,
> shouldn't it check for pg_is_in_recovery() rather than 
>  transaction_read_only ?
> 

Purpose of this feature is to distinguish between master and standby
servers. 

This allows failover system to work with standby servers accepting
client connections, and even to create system where read-only clients 
can be loadbalanced among several hot backup servers, and read-write
clients work with master, but do not need reconfiguration when
failover happens.

pg_is_in_recovery() is really better. But it seems that chapter 25 of
documentation should be improved and this function mentioned in the
section 25.5.1 (Hot Standby / User Overview)



-- 
   Victor Wagner 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] One question about security label command

[Joe Conway]

On 08/30/2015 11:17 AM, Joe Conway wrote:
>>> 3.) Rework patch for 9.2 (Kohei)

>>> 4.) Finish standing up the RHEL/CentOS 7.x buildfarm member to
>>> test sepgsql on 9.2 and up. The animal (rhinoceros) is running
>>> already, but still needs some custom scripting. (Joe, Andrew)

>>> 5.) Additionally stand up a RHEL/CentOS 6.x buildfarm member to
>>> test sepgsql on 9.1 (no changes) (Joe).

> Additionally I have #5 (the 6.x buildfarm member) created/working and
> I sent in my registration form. It has not been accepted yet.

Many thanks to Andrew -- #4 and 5 now complete. Rhinoceros is now
successfully testing sepgsql for 9.2 -> HEAD on CentOS 7.1, and a new
animal, agama, is testing 9.1 on CentOS 6.7.

Remember, expect 9.2 to fail until/unless we get a reworked patch for it.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Josh Berkus]

On 09/06/2015 12:34 PM, Joe Conway wrote:
> To the extent that we want specific pg_controldata output in non-text
> form, we should identify which items those are and provide individual
> functions for them.

Well, I think it's pretty simple, let's take it down:

# function pg_control_control_data() returning INT, TSTZ
pg_control version number:942
pg_control last modified: Thu 20 Aug 2015 10:05:33 AM PDT

#function pg_catversion() returning BIGINT
Catalog version number:   201409291

# have function for this, no?
Database system identifier:   6102142380557650900

# not relevant, if we can connect, it's running
Database cluster state:   shut down

# Do we have functions for all of the below?
# if not I suggest virtual table pg_checkpoint_status
# returning the below 17 columns
# that would be useful even if we have some of them
Latest checkpoint location:   0/1A1BF178
Prior checkpoint location:0/1A1BF0D8
Latest checkpoint's REDO location:0/1A1BF178
Latest checkpoint's REDO WAL file:0001001A
Latest checkpoint's TimeLineID:   1
Latest checkpoint's PrevTimeLineID:   1
Latest checkpoint's full_page_writes: on
Latest checkpoint's NextXID:  0/2038
Latest checkpoint's NextOID:  19684
Latest checkpoint's NextMultiXactId:  1
Latest checkpoint's NextMultiOffset:  0
Latest checkpoint's oldestXID:711
Latest checkpoint's oldestXID's DB:   1
Latest checkpoint's oldestActiveXID:  0
Latest checkpoint's oldestMultiXid:   1
Latest checkpoint's oldestMulti's DB: 1
Time of latest checkpoint:Thu 20 Aug 2015 10:05:33 AM PDT

# Not in any way useful
Fake LSN counter for unlogged rels:   0/1

# add another system view,
# pg_recovery_state, holding the
# below 5 columns
Minimum recovery ending location: 0/0
Min recovery ending loc's timeline: 0
Backup start location:0/0
Backup end location:  0/0
End-of-backup record required:no

# duplicates system settings, not needed
Current wal_level setting:logical
Current wal_log_hints setting:off
Current max_connections setting:  100
Current max_worker_processes setting: 8
Current max_prepared_xacts setting:   0
Current max_locks_per_xact setting:   64
Maximum data alignment:   8
Database block size:  8192

# do we have the below anywhere else?
# this is somewhat duplicative of config info
Blocks per segment of large relation: 131072
WAL block size:   8192
Bytes per WAL segment:16777216
Maximum length of identifiers:64
Maximum columns in an index:  32
Maximum size of a TOAST chunk:1996
Size of a large-object chunk: 2048
Date/time type storage:   64-bit integers
Float4 argument passing:  by value
Float8 argument passing:  by value

# return INT function pg_data_page_checksum_version()
Data page checksum version:   0


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] creating extension including dependencies

[Michael Paquier]

On Tue, Sep 8, 2015 at 10:44 AM, Michael Paquier
 wrote:
> On Tue, Sep 8, 2015 at 6:14 AM, Petr Jelinek wrote:
>> Attached patch uses just boolean in cascade DefElem and splits the
>> CreateExtension into two functions, the cascade code now calls the
>> CreateExtensionInternal. One thing though - I am passing the DefElems
>> directly to the cascaded CreateExtensionStmt options, I think it's not
>> problem but want to give it extra visibility.
>>
>> Also the schema check was moved.
>
> OK, passing the list of extensions through the new routine is indeed a
> cleaner approach. One point of detail is that instead of doing this
> part:
> +   /* Handle the CASCADE option. */
> +   if (d_cascade)
> +   cascade = defGetBoolean(d_cascade);
> +   else
> +   cascade = false;
> You may as well just initialize cascade to false at the beginning of
> the routine and update it only if d_cascade is defined.
>
> Attached are as well changes for the documentation that I spotted in
> earlier reviews but were not included in the last version sent by Petr
> yesterday. Feel free to discard them if you think they are not
> adapted, the patch attached applies on top of Petr's patch.

And /log/ is missing in src/test/modules/extensions/.gitignore.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] New functions

[Alvaro Herrera]

Michael Paquier wrote:
> On Sun, Aug 23, 2015 at 10:21 PM, Michael Paquier wrote:
> > By the way, perhaps it would be worth doing similar things for the
> > other calls of BIO_free and BIO_new, no? I have attached a second
> > patch.
> 
> And... This second patch had a stupid mistake making for example
> ssl_client_dn() fail, so here they are again.

What stupid mistake was this?  I checked your 0002 files and they looked
identical.  I just pushed it all the way back to 9.0, after changing the
elogs to ereports.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_hba_lookup function to get all matching pg_hba.conf entries

[Haribabu Kommi]

On Mon, Sep 7, 2015 at 4:34 AM, Pavel Stehule  wrote:
> Hi
>
>
>>
>> postgres=# select pg_hba_lookup('postgres','all');
>>  pg_hba_lookup
>> ---
>>  (84,local,"[""all""]","[""all""]",,,trust,{})
>>  (86,host,"[""all""]","[""all""]",127.0.0.1,,trust,{})
>>  (88,host,"[""all""]","[""all""]",::1,,trust,{})
>>
>> Here I attached a proof of concept patch for the same.
>>
>> Any suggestions/comments on this proposed approach?
>>
>
> If I understand well to your proposal, the major benefit is in impossibility
> to enter pg_hba keywords - so you don't need to analyse if parameter is
> keyword or not? It has sense, although It can be hard to do image about
> pg_hba conf from these partial views.

>From the function output, it is little bit difficult to map the
pg_hba.conf file.
Because of problems in processing keywords in where clause of a view, I changed
from view to function.

Is there any possibility with rule or something, that the where clause
details can be passed
as function arguments to get the data?

> There can be other way - theoretically we can have a function pg_hba_debug
> with similar API like pg_hba_conf. The result will be a content of
> pg_hba.conf with information about result of any rule.


The output of pg_hba_debug function looks like, the entry of
pg_hba.conf and the result
match for the given input data.

Regards,
Hari Babu
Fujitsu Australia


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] creating extension including dependencies

[Michael Paquier]

On Tue, Sep 8, 2015 at 6:14 AM, Petr Jelinek wrote:
> Attached patch uses just boolean in cascade DefElem and splits the
> CreateExtension into two functions, the cascade code now calls the
> CreateExtensionInternal. One thing though - I am passing the DefElems
> directly to the cascaded CreateExtensionStmt options, I think it's not
> problem but want to give it extra visibility.
>
> Also the schema check was moved.

OK, passing the list of extensions through the new routine is indeed a
cleaner approach. One point of detail is that instead of doing this
part:
+   /* Handle the CASCADE option. */
+   if (d_cascade)
+   cascade = defGetBoolean(d_cascade);
+   else
+   cascade = false;
You may as well just initialize cascade to false at the beginning of
the routine and update it only if d_cascade is defined.

Attached are as well changes for the documentation that I spotted in
earlier reviews but were not included in the last version sent by Petr
yesterday. Feel free to discard them if you think they are not
adapted, the patch attached applies on top of Petr's patch.
Regards,
-- 
Michael


20150908_extension_cascade_docs.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Should TIDs be typbyval = FLOAT8PASSBYVAL to speed up CREATE INDEX CONCURRENTLY?

[Peter Geoghegan]

On Mon, Sep 7, 2015 at 9:03 PM, Tom Lane  wrote:
> I'm not sure that it would be just "a little work" --- I suspect that
> the idea that pass-by-val types are 1, 2, 4, or 8 bytes is embedded in
> a fair number of places, including alignment macros in which any added
> complexity would have a large distributed cost.

I didn't immediately consider that.

>> This matters because a major cost during CREATE INDEX CONCURRENTLY is
>> a TID-based datum sort (this is probably most of the cost over and
>> above a conventional CREATE INDEX).
>
> Might be better to hack a special case right there (ie, embed TIDs into
> int8s and sort the int8s) rather than try to change the type's SQL
> declaration.

That sounds at least as effective as what I originally sketched. It
would also be better to have a simple comparator, rather than the more
complex TID comparator. Baking the details into the int8
representation, rather than having the comparator tease it apart will
be faster. Ordinarily, abbreviation does that kind of thing, but there
are upsides to not doing that when tie-breaker comparisons are not
actually required, and this really only needs to happen in one place.

I hope someone picks this up soon.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Alvaro Herrera]

Jeff Davis wrote:
> On Mon, 2015-09-07 at 18:28 -0300, Alvaro Herrera wrote:

> > I think the extra ugliness is warranted, since it's not THAT much
> > additional ugliness, and not doing it could be considered a regression;
> > apparently strftime can be slower even than snprintf, so doing it twice
> > per log message might be excessive overhead.
> 
> Patch attached. Please take a quick look.

Looks good.


-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Make timestamptz_out less slow.

[David Rowley]

On 3 September 2015 at 22:17, Andres Freund  wrote:

> On 2015-09-03 16:28:40 +1200, David Rowley wrote:
> > > Wouldn't it be better to just normalize fsec to an integer in that
> case?
> > > Afaics that's the only remaining reason for the alternative path?
> > >
> > > A special case would need to exist somewhere, unless we just modified
> > timestamp2tm() to multiply fsec by 1 million when HAVE_INT64_TIMESTAMP is
> > not defined, but that changes the function signature.
>
> Sure, but that's a one line #ifdef?
>

I had a look at this but I found that the precision is 10 with double
timestamps per:

#define MAX_TIME_PRECISION 10
#define TIME_PREC_INV 100.0

So if I do something like:

int fseconds = fsec * TIME_PREC_INV;

In AppendSeconds(), it overflows fseconds.

I could of course make fseconds an int64, but then I'll need to include a
64bit version of pg_int2str(). That does not seem like an improvement.

I'm also starting to not like the name pg_int2str(), It may cause confusion
with 2 bytes, instead of convert "2".

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] Should TIDs be typbyval = FLOAT8PASSBYVAL to speed up CREATE INDEX CONCURRENTLY?

[Tom Lane]

Peter Geoghegan  writes:
> I noticed that the TID type is cataloged as typbyval = f, despite the
> fact that it is 6 bytes, and so could be made typbyval = t on 64-bit
> platforms (i.e. typbyval = FLOAT8PASSBYVAL) with a little work.

I'm not sure that it would be just "a little work" --- I suspect that
the idea that pass-by-val types are 1, 2, 4, or 8 bytes is embedded in
a fair number of places, including alignment macros in which any added
complexity would have a large distributed cost.

> This matters because a major cost during CREATE INDEX CONCURRENTLY is
> a TID-based datum sort (this is probably most of the cost over and
> above a conventional CREATE INDEX).

Might be better to hack a special case right there (ie, embed TIDs into
int8s and sort the int8s) rather than try to change the type's SQL
declaration.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Rework access method interface

[Alexander Korotkov]

On Mon, Sep 7, 2015 at 10:02 PM, Petr Jelinek  wrote:

> On 2015-09-07 20:56, Alexander Korotkov wrote:
>
>> On Mon, Sep 7, 2015 at 9:17 PM, Petr Jelinek >
>> However I don't like the naming differences between validate_opclass
>> and amvalidate. If you expect that the current amvalidate will only
>> be used for opclass validation then it should be renamed accordingly.
>>
>>
>> I'm not yet sure if we need separate validation of opfamilies.
>>
>>
> Well either the amvalidate or the validate_opclass should be renamed IMHO,
> depending on which way the checking goes (one interface for everything with
> generic name or multiple interfaces for multiple validations).


Yes, I agree with you about naming.
I'm not sure about separate validation of opfamilies independent of its
naming. I'd like to get any arguments/advises about it.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] New functions

[Alvaro Herrera]

Michael Paquier wrote:

> Note for committers: attached is a small script that will generate a
> client certificate with extensions enabled. This is helpful when
> testing this patch. Once created, then simply connect with something
> like this connection string:
> "host=127.0.0.1 sslmode=verify-full sslcert=client.crt
> sslkey=client.key sslrootcert=server.crt"
> By querying the new function implemented by this patch the information
> about the client certificate extensions will show up.

Thanks, this was useful.

I made a couple extra cleanups to the patch, namely: do not call
CreateTemplateTupleDesc() just to discard the resulting tupdesc with a
subsequent get_call_result_type(); and do not write a \0 to the
BIO_s_mem, and instead use BIO_get_mem_data's return value as length
when converting str to text *.

And pushed.

FWIW I now think I made a mistake with the error checks that I
backpatched, because the wording of the error messages I used "failed to
foo" is frowned upon by our message style guidelines.  Should be "could
not do foo" instead.

Thanks,

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: contrib/sslinfo: add ssl_extension_info SRF

[Alvaro Herrera]

Michael Paquier wrote:
> On Tue, Sep 8, 2015 at 9:24 AM, Alvaro Herrera  
> wrote:
> > contrib/sslinfo: add ssl_extension_info SRF
> >
> > This new function provides information about SSL extensions present in
> > the X509 certificate used for the current connection.
> >
> > Extension version updated to version 1.1.
> 
> Nitpicky note:
> s/Do when there is no more left/Done when there is no more left/

Argh.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] One question about security label command

[Joe Conway]

On 09/07/2015 04:46 PM, Kouhei Kaigai wrote:
> 3.) Rework patch for 9.2 (Kohei)
>>
> Could you wait for the next Monday?
> I'll try to work this in the next weekend.

Sure, that would be great.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

[HACKERS] Should TIDs be typbyval = FLOAT8PASSBYVAL to speed up CREATE INDEX CONCURRENTLY?

[Peter Geoghegan]

I noticed that the TID type is cataloged as typbyval = f, despite the
fact that it is 6 bytes, and so could be made typbyval = t on 64-bit
platforms (i.e. typbyval = FLOAT8PASSBYVAL) with a little work.

This matters because a major cost during CREATE INDEX CONCURRENTLY is
a TID-based datum sort (this is probably most of the cost over and
above a conventional CREATE INDEX). Based on prior experience, I'd
guess that making the type pass-by-value on 64-bit machines would make
that sort about twice as fast. This would give CREATE INDEX
CONCURRENTLY a nice overall performance boost. SortSupport would also
help, but I would not bother with abbreviation to get some benefit on
32-bit platforms -- that would prevent 64-bit platforms from using
tuplesort's onlyKey optimization, which matters quite a bit. Given the
increasing rarity of 32-bit platforms these days, basic SortSupport
seems best.

I'm more than a little busy at the moment, so I would be happy for
someone else to write the patch. It seems worthwhile.

-- 
Peter Geoghegan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

2015-09-07 21:44 GMT+02:00 Andres Freund :

> Hi,
>
> On 2015-09-02 22:58:21 +0200, Pavel Stehule wrote:
> > > Won't that mean that enum variables don't complete to default anymore?
>
> > no, it does
> >
> > #define Query_for_enum \
> > " SELECT name FROM ( "\
> > "   SELECT unnest(enumvals) AS name "\
> > "FROM pg_catalog.pg_settings "\
> > "   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
> > "   UNION SELECT 'DEFAULT' ) ss "\
> > 
> > "  WHERE pg_catalog.substring(name,1,%%d)='%%s'"
>
> Ah.
>
> I've added a quote_ident() around unnest() - otherwise the
> auto-completions for default_transaction_isolation will mostly be wrong
> due to spaces.
>
> I also renamed get_vartype into get_guctype, changed the comment as I
> found the reference to the pg type system confusing, and more
> importantly made it not return a static buffer.
>
> The spellings for boolean values were a relatively small subset of what
> the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
> sure whether that's a good idea. Comments?
>

if somebody prefer true, false, and we will support only on, off, then the
tabcomplete will not be too user friendly :(

"1, 0" can be out - but other?

>
> Andres
>

Re: [HACKERS] Too many duplicated condition query return wrong value

[Atsushi Yoshida]

Thank you for your answer Jeff.

I have fixed it.

>> What type of index is it?  (I'm now guessing btree, but maybe not)?  Is 
>> there a defined time window during which you know the corruption occurred?  
>> If so, do you still have the server logs from that time window?  The WAL 
>> logs?

Its only btree.
Please tell me which path can I see the logs?

> arcvideo=> \d idx_attend_00
>  Index "public.idx_attend_00"
>  Column |  Type   | Definition 
> +-+
>  sid| integer | sid
> btree, for table "public.attend"
> 
> arcvideo=> \d  index_attend_on_sid_and_lid
> Index "public.index_attend_on_sid_and_lid"
>  Column |  Type   | Definition 
> +-+
>  sid| integer | sid
>  lid| text| lid
> unique, btree, for table "public.attend"

If I want to detect the same thing, should I execute REINDEX sometimes?
Do you have any good ideas?

Thanks.
---
http://github.com/yalab

Atsushi YOSHIDA 
http://twitter.com/yalab inject your heart

> 2015/09/07 7:47、Jeff Janes  のメール：
> 
> On Thu, Sep 3, 2015 at 10:55 PM, Atsushi Yoshida  wrote:
> >> Can you give an "explain (analyze, buffers)"  for each query?  Maybe you 
> >> have a corrupted index, and one query uses the index and the other does 
> >> not.
> 
> 
> >
> >  Index Scan using idx_attend_00 on attend  (cost=0.29..627.20 rows=172 
> > width=12) (actual time=5.158..10.179 rows=5 loops=1)
> >Index Cond: (sid = 325)
> >Filter: (lid = ANY ('{ABF0010,ABF0010,ABF0010,ABF0010,ABF0010 ... 
> > ABF0060,ABF0060,ABF0060,ABF0060}'::text[]))
> >Rows Removed by Filter: 414
>  
> ...
>  
> >
> >  Index Scan using index_attend_on_sid_and_lid on attend  (cost=0.42..36.32 
> > rows=3 width=12) (actual time=0.011..0.034 rows=6 loops=1)
> >Index Cond: ((sid = 325) AND (lid = ANY 
> > ('{ABF0010,ABF0020,ABF0030,ABF0040,ABF0050,ABF0060}'::text[])))
> >Buffers: shared hit=24
> 
>  
> Is this result aims idx_attend_00 corrupted?
> How to fix it?
> What countermeasure do I it?
> 
> Yes, almost certainly.  You can fix it by rebuilding the index ("REINDEX 
> INDEX idx_attend_00").  Whether this will completely fix the problem depends 
> on what caused it.  There could be a wider corruption issue of which this is 
> only a symptom.
> 
> If you would like to preserve the ability to investigate the root cause, you 
> should make a full file-level backup of the database *before* doing the 
> re-index.
> 
> What type of index is it?  (I'm now guessing btree, but maybe not)?  Is there 
> a defined time window during which you know the corruption occurred?  If so, 
> do you still have the server logs from that time window?  The WAL logs?
> 
> Do you know if the sometimes-missing tuple actually belongs in the table or 
> not?  It could be that the row was marked deleted from the table, scrubbed 
> from the index, and then inappropriately got "revived" like a zombie in the 
> table, so that the "corrupt" index is correct and it is the table that is 
> wrong.
> 
> And of course, if you are running with fsync=off or full_page_writes=off, 
> don't do that.
> 
> Cheers,
> 
> Jeff



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] 答复：[HACKERS] about fsync in CLOG buffer write

[周正中(德歌)]

>On 2015-09-10 19:39:59 +0800, 张广舟(明虚) wrote:
>> We found there is a fsync call when CLOG buffer
>> is written out in SlruPhysicalWritePage(). It is often called when a backend
>> needs to check transaction status with SimpleLruReadPage().

>That's when there's not enough buffers available some other, and your
>case dirty, needs to be written out.

>You could try increasing the max (32) in CLOGShmemBuffers() further.I think 
>increasing 
CLOGShmemBuffers have no help for our case, because PG will call pg_fsync once 
for every CLOG PAGE(BLOCKSZ) when ExtendCLOG。there is an trace:
global f_start[99]

probe 
process("/opt/pgsql/bin/postgres").function("SlruPhysicalWritePage@/opt/soft_bak/postgresql-9.4.4/src/backend/access/transam/slru.c").call
 { 
   f_start[execname(), pid(), tid(), cpu()] = gettimeofday_ms()
   printf("%s -> time:%d, pp:%s, par:%s\n", thread_indent(-1), 
gettimeofday_ms(), pp(), $$parms$$)
   # printf("%s -> time:%d, pp:%s\n", thread_indent(1), f_start[execname(), 
pid(), tid(), cpu()], pp() )
}

probe 
process("/opt/pgsql/bin/postgres").function("SlruPhysicalWritePage@/opt/soft_bak/postgresql-9.4.4/src/backend/access/transam/slru.c").return
 {
  t=gettimeofday_ms()
  a=execname()
  b=cpu()
  c=pid()
  d=pp()
  e=tid()
  if (f_start[a,c,e,b]) {
  printf("%s <- time:%d, pp:%s, par:%s\n", thread_indent(-1), t - 
f_start[a,c,e,b], d, $return$$)
  # printf("%s <- time:%d, pp:%s\n", thread_indent(-1), t - f_start[a,c,e,b], d)
  }
}

probe 
process("/opt/pgsql/bin/postgres").function("OpenTransientFile@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c").call
 {
   f_start[execname(), pid(), tid(), cpu()] = gettimeofday_ms()
   printf("%s -> time:%d, pp:%s, par:%s\n", thread_indent(-1), 
gettimeofday_ms(), pp(), $$parms$$)
   # printf("%s -> time:%d, pp:%s\n", thread_indent(1), f_start[execname(), 
pid(), tid(), cpu()], pp() )
}

probe 
process("/opt/pgsql/bin/postgres").function("OpenTransientFile@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c").return
 {
  t=gettimeofday_ms()
  a=execname()
  b=cpu()
  c=pid()
  d=pp()
  e=tid()
  if (f_start[a,c,e,b]) {
  printf("%s <- time:%d, pp:%s, par:%s\n", thread_indent(-1), t - 
f_start[a,c,e,b], d, $return$$)
  # printf("%s <- time:%d, pp:%s\n", thread_indent(-1), t - f_start[a,c,e,b], d)
  }
}

probe 
process("/opt/pgsql/bin/postgres").function("pg_fsync@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c").call
 {
   f_start[execname(), pid(), tid(), cpu()] = gettimeofday_ms()
   printf("%s -> time:%d, pp:%s, par:%s\n", thread_indent(-1), 
gettimeofday_ms(), pp(), $$parms$$)
   # printf("%s -> time:%d, pp:%s\n", thread_indent(1), f_start[execname(), 
pid(), tid(), cpu()], pp() )
}

probe 
process("/opt/pgsql/bin/postgres").function("pg_fsync@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c").return
 {
  t=gettimeofday_ms()
  a=execname()
  b=cpu()
  c=pid()
  d=pp()
  e=tid()
  if (f_start[a,c,e,b]) {
  printf("%s <- time:%d, pp:%s, par:%s\n", thread_indent(-1), t - 
f_start[a,c,e,b], d, $return$$)
  # printf("%s <- time:%d, pp:%s\n", thread_indent(-1), t - f_start[a,c,e,b], d)
  }
}
pg_fsync called every 32K xacts by 
ExtendCLOG

.
 0 postgres(2924): -> time:1441503927731, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("SlruPhysicalWritePage@/opt/soft_bak/postgresql-9.4.4/src/backend/access/transam/slru.c:699").call,
 par:ctl={.shared=0x7f74a9fe39c0, .do_fsync='\001', .PagePrecedes=0x4b1960, 
.Dir="pg_clog"} pageno=12350 slotno=10 fdata=ERROR
31 postgres(2924): -> time:1441503927731, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("OpenTransientFile@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c:1710").call,
 par:fileName="pg_clog/0181" fileFlags=66 fileMode=384
53 postgres(2924): <- time:0, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("OpenTransientFile@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c:1710").return,
 par:14
   102 postgres(2924): -> time:1441503927731, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("pg_fsync@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c:315").call,
 par:fd=14
  1096 postgres(2924): <- time:1, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("pg_fsync@/opt/soft_bak/postgresql-9.4.4/src/backend/storage/file/fd.c:315").return,
 par:0
  1113 postgres(2924): <- time:1, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("SlruPhysicalWritePage@/opt/soft_bak/postgresql-9.4.4/src/backend/access/transam/slru.c:699").return,
 par:'\001'

1105302 postgres(2924): -> time:1441503928836, 
pp:process("/opt/pgsql9.4.4/bin/postgres").function("SlruPhysicalWritePage@/opt/soft_bak/postgresql-9.4.4/src/backend/access/transam/slru.c:699").call,
 par:ctl={.shared=0x7f74a9fe39c0, .do_fsync='\001', .PagePrecedes=0x4b1960, 
.Dir="pg_clog"} pageno=12351 slotno=11 fdata=ERROR
1105329 postgres(2924): -> time:1441503928836, 

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Jeff Davis]

On Mon, 2015-09-07 at 18:28 -0300, Alvaro Herrera wrote:
> I noticed %t, but I don't think we care since the precision is so poor.
> Making m and n work in unison seems enough.  I think it would be
> reasonably simple to handle %t in the same way, but I'm not sure we
> care.

OK.

> I think the extra ugliness is warranted, since it's not THAT much
> additional ugliness, and not doing it could be considered a regression;
> apparently strftime can be slower even than snprintf, so doing it twice
> per log message might be excessive overhead.

Patch attached. Please take a quick look.

Regards,
Jeff Davis



*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
***
*** 143,152  static int	errordata_stack_depth = -1; /* index of topmost active frame */
  
  static int	recursion_depth = 0;	/* to detect actual recursion */
  
! /* buffers for formatted timestamps that might be used by both
!  * log_line_prefix and csv logs.
   */
  
  #define FORMATTED_TS_LEN 128
  static char formatted_start_time[FORMATTED_TS_LEN];
  static char formatted_log_time[FORMATTED_TS_LEN];
--- 143,156 
  
  static int	recursion_depth = 0;	/* to detect actual recursion */
  
! /*
!  * Saved timeval and buffers for formatted timestamps that might be used by
!  * both log_line_prefix and csv logs.
   */
  
+ static struct timeval	saved_timeval;
+ static boolsaved_timeval_set = false;
+ 
  #define FORMATTED_TS_LEN 128
  static char formatted_start_time[FORMATTED_TS_LEN];
  static char formatted_log_time[FORMATTED_TS_LEN];
***
*** 2195,2206  write_console(const char *line, int len)
  static void
  setup_formatted_log_time(void)
  {
- 	struct timeval tv;
  	pg_time_t	stamp_time;
  	char		msbuf[8];
  
! 	gettimeofday(, NULL);
! 	stamp_time = (pg_time_t) tv.tv_sec;
  
  	/*
  	 * Note: we expect that guc.c will ensure that log_timezone is set up (at
--- 2199,2214 
  static void
  setup_formatted_log_time(void)
  {
  	pg_time_t	stamp_time;
  	char		msbuf[8];
  
! 	if (!saved_timeval_set)
! 	{
! 		gettimeofday(_timeval, NULL);
! 		saved_timeval_set = true;
! 	}
! 
! 	stamp_time = (pg_time_t) saved_timeval.tv_sec;
  
  	/*
  	 * Note: we expect that guc.c will ensure that log_timezone is set up (at
***
*** 2213,2219  setup_formatted_log_time(void)
  pg_localtime(_time, log_timezone));
  
  	/* 'paste' milliseconds into place... */
! 	sprintf(msbuf, ".%03d", (int) (tv.tv_usec / 1000));
  	memcpy(formatted_log_time + 19, msbuf, 4);
  }
  
--- 2221,2227 
  pg_localtime(_time, log_timezone));
  
  	/* 'paste' milliseconds into place... */
! 	sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000));
  	memcpy(formatted_log_time + 19, msbuf, 4);
  }
  
***
*** 2440,2450  log_line_prefix(StringInfo buf, ErrorData *edata)
  break;
  			case 'n':
  {
- 	struct	timeval tv;
  	char	strfbuf[128];
  
! 	gettimeofday(, NULL);
! 	sprintf(strfbuf, "%ld.%03d", tv.tv_sec, (int)(tv.tv_usec / 1000));
  
  	if (padding != 0)
  		appendStringInfo(buf, "%*s", padding, strfbuf);
--- 2448,2463 
  break;
  			case 'n':
  {
  	char	strfbuf[128];
  
! 	if (!saved_timeval_set)
! 	{
! 		gettimeofday(_timeval, NULL);
! 		saved_timeval_set = true;
! 	}
! 
! 	sprintf(strfbuf, "%ld.%03d", saved_timeval.tv_sec,
! 			(int)(saved_timeval.tv_usec / 1000));
  
  	if (padding != 0)
  		appendStringInfo(buf, "%*s", padding, strfbuf);
***
*** 2825,2830  send_message_to_server_log(ErrorData *edata)
--- 2838,2844 
  
  	initStringInfo();
  
+ 	saved_timeval_set = false;
  	formatted_log_time[0] = '\0';
  
  	log_line_prefix(, edata);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] exposing pg_controldata and pg_config as functions

[Andrew Dunstan]

On 09/06/2015 11:17 PM, Peter Eisentraut wrote:

On 9/6/15 3:34 PM, Joe Conway wrote:

On 09/02/2015 02:54 PM, Alvaro Herrera wrote:

Josh Berkus wrote:

On 09/02/2015 02:34 PM, Alvaro Herrera wrote:

I think trying to duplicate the exact strings isn't too nice an
interface.

Well, for pg_controldata, no, but what else would you do for pg_config?

I was primarily looking at pg_controldata, so we agree there.

As for pg_config, I'm confused about its usefulness -- which of these
lines are useful in the SQL interface?  Anyway, I don't see anything
better than a couple of text columns for this case.

There are production environments where even the superuser has no
direct, local, command line access on production database servers

But then they also have no use for the information that pg_config prints
out.


and the
only interface for getting information from postgres is via a database
connection. So to the extent pg_config and pg_controldata command line
binaries are useful, so is the ability to get the same output via SQL.

Given that, my own feeling is that if we provide a SQL interface at all,
it ought to be pretty much the exact same output as the command line
programs produce.

That argument makes no sense to me.

Again, we need to think about what actual use there is for this
information.  Just because the information exists somewhere, but you
can't access it, doesn't mean we just need to copy it around.



I already gave a use case that you dismissed in favour of a vague 
solution that we don't actually have. You seem to be the only person 
objecting to this proposal.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] One question about security label command

[Kouhei Kaigai]

Hi Joe,

> >>> 3.) Rework patch for 9.2 (Kohei)
>
Could you wait for the next Monday?
I'll try to work this in the next weekend.
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 


> -Original Message-
> From: Joe Conway [mailto:m...@joeconway.com]
> Sent: Tuesday, September 08, 2015 6:54 AM
> To: Adam Brightwell
> Cc: Stephen Frost; Alvaro Herrera; Kohei KaiGai; Kaigai Kouhei(海外 浩平); Tom
> Lane; Robert Haas; 张元超; pgsql-hackers@postgresql.org;
> adam.brightw...@crunchydata.com
> Subject: Re: [HACKERS] One question about security label command
> 
> On 08/30/2015 11:17 AM, Joe Conway wrote:
> >>> 3.) Rework patch for 9.2 (Kohei)
> 
> >>> 4.) Finish standing up the RHEL/CentOS 7.x buildfarm member to
> >>> test sepgsql on 9.2 and up. The animal (rhinoceros) is running
> >>> already, but still needs some custom scripting. (Joe, Andrew)
> 
> >>> 5.) Additionally stand up a RHEL/CentOS 6.x buildfarm member to
> >>> test sepgsql on 9.1 (no changes) (Joe).
> 
> > Additionally I have #5 (the 6.x buildfarm member) created/working and
> > I sent in my registration form. It has not been accepted yet.
> 
> Many thanks to Andrew -- #4 and 5 now complete. Rhinoceros is now
> successfully testing sepgsql for 9.2 -> HEAD on CentOS 7.1, and a new
> animal, agama, is testing 9.1 on CentOS 6.7.
> 
> Remember, expect 9.2 to fail until/unless we get a reworked patch for it.
> 
> Joe
> 
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] New functions

[Michael Paquier]

On Tue, Sep 8, 2015 at 7:31 AM, Alvaro Herrera  wrote:
> Michael Paquier wrote:
>> On Sun, Aug 23, 2015 at 10:21 PM, Michael Paquier wrote:
>> > By the way, perhaps it would be worth doing similar things for the
>> > other calls of BIO_free and BIO_new, no? I have attached a second
>> > patch.
>>
>> And... This second patch had a stupid mistake making for example
>> ssl_client_dn() fail, so here they are again.
>
> What stupid mistake was this?  I checked your 0002 files and they looked
> identical.  I just pushed it all the way back to 9.0, after changing the
> elogs to ereports.

Never mind. I cannot recall either.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pageinspect patch, for showing tuple data

[Michael Paquier]

On Sat, Sep 5, 2015 at 1:05 AM, Nikolay Shaplov wrote:

> В письме от 4 сентября 2015 14:58:29 пользователь Michael Paquier написал:
> > Documentation is missing, that would be good to have to understand what
> > each function is intended to do.
>
> I were going to add documentation when this patch is commited, or at least
> approved for commit. So I would know for sure that function definition
> would
> not change, so had not to rewrite it again and again.
>

I doubt that this patch would be committed without documentation, this is a
requirement for any patch.


> So if it is possible I would write documentation and test when this patch
> is
> already approved.
>

I'm fine with that. Let's discuss its shape and come up with an agreement.
It would have been good to post test cases of what this stuff actually does
though, people reviewing this thread are limited to guess on what is tried
to be achieved just by reading the code. That's actually where
documentation, even a draft of documentation helps a lot in the review to
see if what is expected by the developer matches what the code actually
does.

> Code has some whitespaces.
> I've found and removed some. Hope that was all of them...
>

Yeah, it looks that you took completely rid of them.

In details, this patch introduces two independent concepts:
- add tuple data as a new bytea column of heap_page_items. This is indeed
where it should be, and note that this is where the several corruption
checks are done by checking the state of the tuple data.
- add heap_page_item_attrs and heap_page_item_detoast_attrs, which is very
similar to heap_page_items, at the difference that it can take an OID to be
able to use a TupleDesc and return a bytea array with the data of each
attribute.

Honestly, heap_page_item_attrs and heap_page_item_detoast_attrs are way too
similar to what heap_page_items does, leading to a code maze that is going
to make future extensions more difficult, which is what lead to the
refactoring your patch does.
Hence, instead of all those complications, why not simply introducing two
functions that take as input the tuple data and the OID of the relation,
returning those bytea arrays? It seems to be that this would be a more
handy interface, and as this is for educational purposes I guess that the
addition of the overhead induced by the extra function call would be
acceptable.

Regards,
-- 
Michael

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Alvaro Herrera]

Jeff Davis wrote:
> On Sun, 2015-03-22 at 19:47 +0100, Andres Freund wrote:
> > On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:
> > > from time to time I need to correlate PostgreSQL logs to other logs,
> > > containing numeric timestamps - a prime example of that is pgbench. With
> > > %t and %m that's not quite trivial, because of timezones etc.
> > 
> > I have a hard time seing this is sufficient cause for adding more format
> > codes. They're not free runtime and documentation wise. -0.5 from me.
> 
> I am about to commit this patch (the one that adds a single extra
> option). Although nothing is free, the cost seems very low, and at least
> three people have expressed interest in this patch.

Did you see my message at 
http://www.postgresql.org/message-id/20150907192719.GS2912@alvherre.pgsql ?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Jeff Davis]

On Mon, 2015-09-07 at 17:47 -0300, Alvaro Herrera wrote:
> Jeff Davis wrote:
> > On Sun, 2015-03-22 at 19:47 +0100, Andres Freund wrote:
> > > On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:
> > > > from time to time I need to correlate PostgreSQL logs to other logs,
> > > > containing numeric timestamps - a prime example of that is pgbench. With
> > > > %t and %m that's not quite trivial, because of timezones etc.
> > > 
> > > I have a hard time seing this is sufficient cause for adding more format
> > > codes. They're not free runtime and documentation wise. -0.5 from me.
> > 
> > I am about to commit this patch (the one that adds a single extra
> > option). Although nothing is free, the cost seems very low, and at least
> > three people have expressed interest in this patch.
> 
> Did you see my message at 
> http://www.postgresql.org/message-id/20150907192719.GS2912@alvherre.pgsql ?

I guess we were looking at this at the same time, and I missed your
message and committed it. I will investigate now.

Regards,
Jeff Davis




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] Proposal for \rotate in psql

[David G. Johnston]

On Mon, Sep 7, 2015 at 4:18 PM, Pavel Stehule 
wrote:

>
>
> 2015-09-07 22:14 GMT+02:00 Greg Stark :
>
>> On Fri, Sep 4, 2015 at 5:08 PM, Daniel Verite 
>> wrote:
>> > I'm not dead set on \rotate and suggested other names
>> > previously in [1], but none of them seems decisively
>> > superior.
>>
>>
>> Fwiw I like \rotate. It's pretty clear what it means and it sounds
>> similar to but not exactly the same as pivot.
>>
>
> rotate ~ sounds like transpose matrix, what is not true in this case.
>
>
So?  If PostgreSQL had any native matrix processing capabilities this would
maybe warrant a bit of consideration.

​https://github.com/hadley/tidyr

\spread
\unfold
\rotate

Given the role that psql performs I do think \rotate to be the least
problematic choice; I concur that avoiding \pivot is desirable due to SQL's
usage.

David J.

Re: [HACKERS] creating extension including dependencies

[Petr Jelinek]

On 2015-09-07 21:28, Petr Jelinek wrote:

On 2015-09-07 21:09, Alvaro Herrera wrote:

Andres Freund wrote:

On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:

Yes that sounds cleaner. Just as a side note, List is a Node and
does have
copy support (and we pass List as DefElem->arg from gram.y in several
places).


I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?


Seems slightly easier to use makeString(), no?



Yes, but I think Andres is correct when saying DefElem->arg is not
nicest place to put it to.



Attached patch uses just boolean in cascade DefElem and splits the 
CreateExtension into two functions, the cascade code now calls the 
CreateExtensionInternal. One thing though - I am passing the DefElems 
directly to the cascaded CreateExtensionStmt options, I think it's not 
problem but want to give it extra visibility.


Also the schema check was moved.

--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 5fc0c3245d82220deb7adcc3704b95b82893fcfe Mon Sep 17 00:00:00 2001
From: Petr Jelinek 
Date: Sat, 13 Jun 2015 19:49:10 +0200
Subject: [PATCH] CREATE EXTENSION CASCADE

---
 contrib/hstore_plperl/expected/hstore_plperl.out   |   6 +-
 contrib/hstore_plperl/expected/hstore_plperlu.out  |   6 +-
 contrib/hstore_plperl/sql/hstore_plperl.sql|   4 +-
 contrib/hstore_plperl/sql/hstore_plperlu.sql   |   4 +-
 .../hstore_plpython/expected/hstore_plpython.out   |   4 +-
 contrib/hstore_plpython/sql/hstore_plpython.sql|   3 +-
 contrib/ltree_plpython/expected/ltree_plpython.out |   4 +-
 contrib/ltree_plpython/sql/ltree_plpython.sql  |   3 +-
 doc/src/sgml/ref/create_extension.sgml |  42 +
 src/backend/commands/extension.c   | 209 +++--
 src/backend/parser/gram.y  |   4 +
 src/bin/psql/tab-complete.c|   7 +-
 src/test/modules/Makefile  |   1 +
 src/test/modules/test_extensions/.gitignore|   3 +
 src/test/modules/test_extensions/Makefile  |  21 +++
 .../test_extensions/expected/test_extensions.out   |  30 +++
 .../test_extensions/sql/test_extensions.sql|  18 ++
 .../modules/test_extensions/test_ext1--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext1.control |   5 +
 .../modules/test_extensions/test_ext2--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext2.control |   4 +
 .../modules/test_extensions/test_ext3--1.0.sql |   3 +
 src/test/modules/test_extensions/test_ext3.control |   3 +
 .../test_extensions/test_ext_cyclic1--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic1.control   |   4 +
 .../test_extensions/test_ext_cyclic2--1.0.sql  |   3 +
 .../test_extensions/test_ext_cyclic2.control   |   4 +
 src/tools/msvc/Mkvcbuild.pm|   3 +-
 28 files changed, 323 insertions(+), 84 deletions(-)
 create mode 100644 src/test/modules/test_extensions/.gitignore
 create mode 100644 src/test/modules/test_extensions/Makefile
 create mode 100644 src/test/modules/test_extensions/expected/test_extensions.out
 create mode 100644 src/test/modules/test_extensions/sql/test_extensions.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext1.control
 create mode 100644 src/test/modules/test_extensions/test_ext2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext2.control
 create mode 100644 src/test/modules/test_extensions/test_ext3--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext3.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic1.control
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2--1.0.sql
 create mode 100644 src/test/modules/test_extensions/test_ext_cyclic2.control

diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
index cf384eb..25fc506 100644
--- a/contrib/hstore_plperl/expected/hstore_plperl.out
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE EXTENSION plperl;
-CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperl CASCADE;
+NOTICE:  installing required extension "hstore"
+NOTICE:  installing required extension "plperl"
 SELECT transforms.udt_schema, transforms.udt_name,
routine_schema, routine_name,
group_name, transform_type
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
index 8c689ad..3fc3201 100644
--- a/contrib/hstore_plperl/expected/hstore_plperlu.out
+++ b/contrib/hstore_plperl/expected/hstore_plperlu.out
@@ -1,6 +1,6 @@
-CREATE EXTENSION hstore;
-CREATE 

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Jeff Davis]

> I wonder about this separate gettimeofday() call.  We already have
> formatted_log_time which is used for CSV logs and freeform log lines
> (stderr/syslog); if we introduce a separate gettimeofday() call here,
> and the user has %n in freeform log and CSV logging is active, the
> timings will diverge occasionally.
> 
> Maybe I'm worrying over nothing, because really what use case is there
> for having the two log formats enabled at the same time?  Yet somebody
> went some lengths to ensure they are consistent; I think we should do
> likewise here.

We now have three time-related options[1]: t, m, and n; and they each
acquire the time independently. Are you suggesting that we make all
three consistent, or only m and n?

The cleanest fix would be for the global variable to only hold the
timeval, and then format it once for the CSV log (always 'm' format) and
once for the regular log ('m', 'n', or 't'). If the regular log uses
'm', that would be some wasted cycles formatting it the same way twice.
Is it worth a little extra ugliness to cache both the timeval and the
formatted string?

Regards,
Jeff Davis

[1] As of minutes ago, after I missed your message by a few minutes.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Alvaro Herrera]

Jeff Davis wrote:
> > I wonder about this separate gettimeofday() call.  We already have
> > formatted_log_time which is used for CSV logs and freeform log lines
> > (stderr/syslog); if we introduce a separate gettimeofday() call here,
> > and the user has %n in freeform log and CSV logging is active, the
> > timings will diverge occasionally.
> > 
> > Maybe I'm worrying over nothing, because really what use case is there
> > for having the two log formats enabled at the same time?  Yet somebody
> > went some lengths to ensure they are consistent; I think we should do
> > likewise here.
> 
> We now have three time-related options[1]: t, m, and n; and they each
> acquire the time independently. Are you suggesting that we make all
> three consistent, or only m and n?

I noticed %t, but I don't think we care since the precision is so poor.
Making m and n work in unison seems enough.  I think it would be
reasonably simple to handle %t in the same way, but I'm not sure we
care.

(TBH I question that %t has any usefulness at all.  Maybe we should
phase it out ...)

> The cleanest fix would be for the global variable to only hold the
> timeval, and then format it once for the CSV log (always 'm' format) and
> once for the regular log ('m', 'n', or 't'). If the regular log uses
> 'm', that would be some wasted cycles formatting it the same way twice.
> Is it worth a little extra ugliness to cache both the timeval and the
> formatted string?

I think the extra ugliness is warranted, since it's not THAT much
additional ugliness, and not doing it could be considered a regression;
apparently strftime can be slower even than snprintf, so doing it twice
per log message might be excessive overhead.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] Proposal for \rotate in psql

[Greg Stark]

On Fri, Sep 4, 2015 at 5:08 PM, Daniel Verite  wrote:
> I'm not dead set on \rotate and suggested other names
> previously in [1], but none of them seems decisively
> superior.


Fwiw I like \rotate. It's pretty clear what it means and it sounds
similar to but not exactly the same as pivot.

-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] Proposal for \rotate in psql

[Pavel Stehule]

2015-09-07 22:14 GMT+02:00 Greg Stark :

> On Fri, Sep 4, 2015 at 5:08 PM, Daniel Verite 
> wrote:
> > I'm not dead set on \rotate and suggested other names
> > previously in [1], but none of them seems decisively
> > superior.
>
>
> Fwiw I like \rotate. It's pretty clear what it means and it sounds
> similar to but not exactly the same as pivot.
>

rotate ~ sounds like transpose matrix, what is not true in this case.

Pavel


>
> --
> greg
>

Re: [HACKERS] Waits monitoring

[Alexander Korotkov]

On Mon, Sep 7, 2015 at 6:28 AM, Amit Kapila  wrote:

> On Sun, Sep 6, 2015 at 5:58 PM, Andres Freund  wrote:
> >
> > On 2015-09-04 23:44:21 +0100, Simon Riggs wrote:
> > > I see the need for both current wait information and for cumulative
> > > historical detail.
> > >
> > > I'm willing to wait before reviewing this, but not for more than 1
> more CF.
> > >
> > > Andres, please decide whether we should punt to next CF now, based upon
> > > other developments. Thanks
> >
> > I think we can do some of the work concurrently - the whole lwlock
> > infrastructure piece is rather independent and what currently most of
> > the arguments are about. I agree that the actual interface will need to
> > be coordinated.
> >
> > Ildus, could you please review Amit & Robert's patch?
> >
>
> Are you talking about patch where I have fixed few issues in Robert's
> patch [1] or the original patch [3] written by me.
>

AFAICS, Andres meant [3].

[1] -
> http://www.postgresql.org/message-id/caa4ek1kdec1tm5ya9gkv85vtn4qqsrxzkjru-tu70g_tl1x...@mail.gmail.com
> [2] -
> http://www.postgresql.org/message-id/3f71da37-a17b-4961-9908-016e6323e...@postgrespro.ru
> [3] -
> http://www.postgresql.org/message-id/caa4ek1kt2e6xhvigisr5o1ac9nfo0j2wtb8n0ggd1_jklde...@mail.gmail.com
>

For me, major issues of [3] are:
1) We fit all information about current wait event into single byte.
2) We put this byte into PgBackendStatus.
Both of these issues are essential for design of [3].

Issue #1 means that we actually can expose to user only event type (until
there are less then 256 types) with no parameters [4].
Issue #2 means that we have to monitor only backend not auxiliary processes
[5].

Both of these issues seems to be serious limitations for me. It makes me
think we either need a different design or need to expose current wait
event in some other way additionally. Anyway, I think attracting more
attention to this problem is good. It would be very nice if Simon or Andres
give review of this.

[4] -
http://www.postgresql.org/message-id/CAPpHfdtdF8LyR0zBA_tzAwYq00GFZyVbh_XfFAABRQQ=mbn...@mail.gmail.com
[5] - http://www.postgresql.org/message-id/55c3306b.5010...@postgrespro.ru

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Jeff Davis]

On Sun, 2015-03-22 at 19:47 +0100, Andres Freund wrote:
> On 2015-03-22 00:47:12 +0100, Tomas Vondra wrote:
> > from time to time I need to correlate PostgreSQL logs to other logs,
> > containing numeric timestamps - a prime example of that is pgbench. With
> > %t and %m that's not quite trivial, because of timezones etc.
> 
> I have a hard time seing this is sufficient cause for adding more format
> codes. They're not free runtime and documentation wise. -0.5 from me.

I am about to commit this patch (the one that adds a single extra
option). Although nothing is free, the cost seems very low, and at least
three people have expressed interest in this patch.

What tips the balance is that we expose the unix epoch in the pgbench
logs, as Tomas points out.

Regards,
Jeff Davis




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Horizontal scalability/sharding

[Ahsan Hadi]

I

On Monday, September 7, 2015, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:

>
>
> On Sat, Sep 5, 2015 at 4:22 AM, Ozgun Erdogan  > wrote:
>
>> Hey Robert,
>>
>> Now the question is, where should the code that does all of this live?
>>>  postgres_fdw?  Some new, sharding-specific FDW?  In core?  I don't
>>> know for sure, but what I do know is that we could make a lot of
>>> progress over where we are today by just improving postgres_fdw, and I
>>> don't think those improvements are even all that difficult.  If we
>>> decide we need to implement something new, it's going to be a huge
>>> project that will take years to complete, with uncertain results.  I'd
>>> rather have a postgres_fdw-based implementation that is imperfect and
>>> can't handle some kinds of queries in 9.6 than a promise that by 9.9
>>> we'll have something really great that handles MPP perfectly.
>>>
>>
>> Distributed shuffles (Map/Reduce) are hard. When we looked at using FDWs
>> for pg_shard, we thought that Map/Reduce would require a comprehensive
>> revamp of the APIs.
>>
>> For Citus, a second part of the question is as FDW writers. We
>> implemented cstore_fdw, json_fdw, and mongo_fdw, and these wrappers don't
>> benefit from even the simple join pushdown that doesn't require Map/Reduce.
>>
>
> I didn't get this. Join pushdown infrastructure (chiefly set of hooks
> provided in join planning paths) is part of 9.5. Isn't that sufficient to
> implement join push-down for above FDWs? Or FDW writers are facing problems
> while implementing those hooks. In either case that should be reported on
> hackers.
>
>

I don't think any FDW writer (other the postgres_fdw) has tried to
implement join push down in the respective FDW's using the new API.


>
>> The PostgreSQL wiki lists 85 foreign data wrappers, and only 18 of these
>> have support for joins:
>> https://wiki.postgresql.org/wiki/Foreign_data_wrappers
>>
>> Best,
>> Ozgun
>>
>
>
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>


-- 
Ahsan Hadi
Snr Director Product Development
EnterpriseDB Corporation
The Enterprise Postgres Company

Phone: +92-51-8358874
Mobile: +92-333-5162114

Website: www.enterprisedb.com
EnterpriseDB Blog: http://blogs.enterprisedb.com/
Follow us on Twitter: http://www.twitter.com/enterprisedb

This e-mail message (and any attachment) is intended for the use of the
individual or entity to whom it is addressed. This message contains
information from EnterpriseDB Corporation that may be privileged,
confidential, or exempt from disclosure under applicable law. If you are
not the intended recipient or authorized to receive this for the intended
recipient, any use, dissemination, distribution, retention, archiving, or
copying of this communication is strictly prohibited. If you have received
this e-mail in error, please notify the sender immediately by reply e-mail
and delete this message.

Re: [HACKERS] creating extension including dependencies

[Petr Jelinek]

On 2015-09-02 17:31, Andres Freund wrote:

On 2015-09-02 17:27:38 +0200, Andres Freund wrote:

1) Passing the list of parents through the cascade DefElem strikes me as
incredibly ugly.

For one the cascade option really should take a true/false type option
on the C level (so you can do defGetBoolean()), for another passing
through the list of parents via DefElem->arg seems wrong. You're
supposed to be able to copy parsenodes and at the very least that's
broken by the approach.


I think the fix here is to split off the bulk of CreateExtension() into
an internal function that takes additional parameters.



Yes that sounds cleaner. Just as a side note, List is a Node and does 
have copy support (and we pass List as DefElem->arg from gram.y in 
several places).


> 2) I don't like the control flow around the schema selection.
>
> It seems to be getting a bit arcane. How about instead moving the
> "extension \"%s\" must be installed in schema \"%s\" check into the if
> (control->schema != NULL) block and check for d_schema after it? That
> should look cleaner.
>

I did something like that in one of the revisions, the complaint there 
was that it changes order of errors you get in situation when the schema 
is not the same as the one in control file and it also does not exist.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers

[Alvaro Herrera]

Andres Freund wrote:

> On 2015-09-07 10:34:10 -0300, Alvaro Herrera wrote:
> > I wonder if it would make sense to explore an idea that has been floated
> > for years now -- to have pg_clog pages be allocated as part of shared
> > buffers rather than have their own separate pool.  That way, no separate
> > hardcoded allocation limit is needed.  It's probably pretty tricky to
> > implement, though :-(
> 
> I still think that'd be a good plan, especially as it'd also let us use
> a lot of related infrastructure. I doubt we could just use the standard
> cache replacement mechanism though - it's not particularly efficient
> either... I also have my doubts that a hash table lookup at every clog
> lookup is going to be ok performancewise.

Yeah.  I guess we'd have to mark buffers as unusable for regular pages
("somehow"), and have a separate lookup mechanism.  As I said, it is
likely to be tricky.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Rework access method interface

[Alexander Korotkov]

On Mon, Sep 7, 2015 at 9:17 PM, Petr Jelinek  wrote:

> On 2015-09-04 16:26, Alexander Korotkov wrote:
>
>>
>> Attached patch is implementing this. It doesn't pretend to be fully
>> correct implementation, but it should be enough for proof the concept.
>> In this patch access method exposes another function: amvalidate. It
>> takes data structure representing opclass and throws error if it finds
>> it invalid.
>> This method is used on new opclass definition (alter operator family
>> etc. are not yet implemented but planned). Also, there is SQL function
>> validate_opclass(oid) which is used in regression tests.
>> Any thoughts?
>>
>>
> This is starting to look good
>

Thanks!


> However I don't like the naming differences between validate_opclass and
> amvalidate. If you expect that the current amvalidate will only be used for
> opclass validation then it should be renamed accordingly.
>

I'm not yet sure if we need separate validation of opfamilies.

Also GetIndexAmRoutine should check the return type of the amhandler.


Will be fixed.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] creating extension including dependencies

[Andres Freund]

On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> Yes that sounds cleaner. Just as a side note, List is a Node and does have
> copy support (and we pass List as DefElem->arg from gram.y in several
> places).

I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?

> > 2) I don't like the control flow around the schema selection.
> >
> > It seems to be getting a bit arcane. How about instead moving the
> > "extension \"%s\" must be installed in schema \"%s\" check into the if
> > (control->schema != NULL) block and check for d_schema after it? That
> > should look cleaner.
> >
> 
> I did something like that in one of the revisions, the complaint there was
> that it changes order of errors you get in situation when the schema is not
> the same as the one in control file and it also does not exist.

So what? That seems like a pretty harmless change - it's not like this
is something being hit day in/out right now.

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] WIP: Rework access method interface

[Petr Jelinek]

On 2015-09-07 20:56, Alexander Korotkov wrote:

On Mon, Sep 7, 2015 at 9:17 PM, Petr Jelinek 

Re: [HACKERS] creating extension including dependencies

[Andres Freund]

On 2015-09-07 16:09:27 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> > > Yes that sounds cleaner. Just as a side note, List is a Node and does have
> > > copy support (and we pass List as DefElem->arg from gram.y in several
> > > places).
> > 
> > I know - but the list element in this case don't have copy support, no?
> > You seem to have put plain C strings in there, right?
> 
> Seems slightly easier to use makeString(), no?

It'd still be a god forsakenly ugly API to use DefElems for this. Imo
not being able to specify a boolean argument for a boolean value pretty
much hints at it being the wrong approach. I don't see any reason to go
that way rather than split the 'cycle detection' state into an argument
to CreateExtensionInternal() or something.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PATCH: numeric timestamp in log_line_prefix

[Alvaro Herrera]

> + case 'n':
> + {
> + struct  timeval tv;
> + charstrfbuf[128];
> +
> + gettimeofday(, NULL);
> + sprintf(strfbuf, "%ld.%03d", tv.tv_sec, 
> (int)(tv.tv_usec / 1000));
> +
> + if (padding != 0)
> + appendStringInfo(buf, "%*s", 
> padding, strfbuf);
> + else
> + appendStringInfoString(buf, 
> strfbuf);
> + }
> + break;

I wonder about this separate gettimeofday() call.  We already have
formatted_log_time which is used for CSV logs and freeform log lines
(stderr/syslog); if we introduce a separate gettimeofday() call here,
and the user has %n in freeform log and CSV logging is active, the
timings will diverge occasionally.

Maybe I'm worrying over nothing, because really what use case is there
for having the two log formats enabled at the same time?  Yet somebody
went some lengths to ensure they are consistent; I think we should do
likewise here.

I'm happy to be outvoted on this, so consider this a -0.5 for this
particular implementation.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] creating extension including dependencies

[Petr Jelinek]

On 2015-09-07 21:09, Alvaro Herrera wrote:

Andres Freund wrote:

On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:

Yes that sounds cleaner. Just as a side note, List is a Node and does have
copy support (and we pass List as DefElem->arg from gram.y in several
places).


I know - but the list element in this case don't have copy support, no?
You seem to have put plain C strings in there, right?


Seems slightly easier to use makeString(), no?



Yes, but I think Andres is correct when saying DefElem->arg is not 
nicest place to put it to.


Looking at the code again, splitting the function is actually not that 
easy since the cascaded extension creation has to execute all the same 
code/checks as CreateExtension does; It might be better to just add the 
parameter to the CreateExtension and call it with NIL value from utility.c.


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] creating extension including dependencies

[Alvaro Herrera]

Andres Freund wrote:
> On 2015-09-07 20:56:50 +0200, Petr Jelinek wrote:
> > Yes that sounds cleaner. Just as a side note, List is a Node and does have
> > copy support (and we pass List as DefElem->arg from gram.y in several
> > places).
> 
> I know - but the list element in this case don't have copy support, no?
> You seem to have put plain C strings in there, right?

Seems slightly easier to use makeString(), no?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pgbench progress with timestamp

[Jeff Janes]

On Sun, Aug 23, 2015 at 4:25 AM, Fabien COELHO  wrote:

>
> It is not easy to compare events on a pgbench runs (oops, the tps is down)
> with for instance events in postgres log, so as to figure out what may have
> cause a given glitch.
>
> This patches adds an option to replace the "time since pgbench run
> started" with a timestamp in the progress report so that it is easier to
> compare timelines.
>
> Use milliseconds for consistency with the '%n' log_prefix patch currently
> submitted by Tomas Vondra in the CF.
>
>   sh> ./pgbench -P 1 -N -T 100 -c 2
>   starting vacuum...end.
>   progress: 1.0 s, 546.0 tps, lat 3.619 ms stddev 4.426
>   progress: 2.0 s, 575.0 tps, lat 3.480 ms stddev 1.705
>
>   sh> ./pgbench -P 1 --progress-timestamp -N -T 100 -c 2
>   starting vacuum...end.
>   progress: 1440328800.064 s, 549.0 tps, lat 3.602 ms stddev 1.698
>   progress: 1440328801.064 s, 570.0 tps, lat 3.501 ms stddev 1.704
>


I like the idea of the timestamp.  But could just always print both the
timestamp and the elapsed time, rather than adding another switch to decide
between them?

Cheers,

Jeff

Re: [HACKERS] WIP: Access method extendability

[Teodor Sigaev]

Petr Jelinek wrote:

On 2015-09-07 17:41, Teodor Sigaev wrote:

Some notices:

1) create-am.3.patch.gz
   As I understand, you didn't add schema name to access method. Why?
Suppose, if we implement SQL-like interface for AM screation/dropping
then we should provide a schema qualification for them


Why would access methods need to be schema qualified?


Opfamily and opclass could be schema-qualified.

--
Teodor Sigaev  E-mail: teo...@sigaev.ru
  WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Andres Freund]

Hi,

On 2015-09-02 22:58:21 +0200, Pavel Stehule wrote:
> > Won't that mean that enum variables don't complete to default anymore?

> no, it does
> 
> #define Query_for_enum \
> " SELECT name FROM ( "\
> "   SELECT unnest(enumvals) AS name "\
> "FROM pg_catalog.pg_settings "\
> "   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
> "   UNION SELECT 'DEFAULT' ) ss "\
> 
> "  WHERE pg_catalog.substring(name,1,%%d)='%%s'"

Ah.

I've added a quote_ident() around unnest() - otherwise the
auto-completions for default_transaction_isolation will mostly be wrong
due to spaces.

I also renamed get_vartype into get_guctype, changed the comment as I
found the reference to the pg type system confusing, and more
importantly made it not return a static buffer.

The spellings for boolean values were a relatively small subset of what
the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
sure whether that's a good idea. Comments?

Andres
>From 279cdbdaed568a9dd95e18b4bb5c3098a0791008 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 7 Sep 2015 21:28:10 +0200
Subject: [PATCH] psql: Generic tab completion support for enum and bool GUCs.

Author: Pavel Stehule
Reviewed-By: Andres Freund
Discussion: 5594fe7a.5050...@iki.fi
---
 src/bin/psql/tab-complete.c | 132 
 1 file changed, 97 insertions(+), 35 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9303f6a..85207cc 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -757,6 +757,15 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   (SELECT polrelid FROM pg_catalog.pg_policy "\
 " WHERE pg_catalog.quote_ident(polname)='%s')"
 
+#define Query_for_enum \
+" SELECT name FROM ( "\
+"   SELECT pg_catalog.quote_ident(pg_catalog.unnest(enumvals)) AS name "\
+" FROM pg_catalog.pg_settings "\
+"WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
+"UNION ALL " \
+"   SELECT 'DEFAULT' ) ss "\
+"  WHERE pg_catalog.substring(name,1,%%d)='%%s'"
+
 /*
  * This is a list of all "things" in Pgsql, which can show up after CREATE or
  * DROP; and there is also a query to get a list of them.
@@ -843,10 +852,13 @@ static char **complete_from_variables(const char *text,
 static char *complete_from_files(const char *text, int state);
 
 static char *pg_strdup_keyword_case(const char *s, const char *ref);
+static char *escape_string(const char *text);
 static PGresult *exec_query(const char *query);
 
 static void get_previous_words(int point, char **previous_words, int nwords);
 
+static char *get_guctype(const char *varname);
+
 #ifdef NOT_USED
 static char *quote_file_name(char *text, int match_type, char *quote_pointer);
 static char *dequote_file_name(char *text, char quote_char);
@@ -3594,6 +3606,7 @@ psql_completion(const char *text, int start, int end)
 	else if (pg_strcasecmp(prev3_wd, "SET") == 0 &&
 			 (pg_strcasecmp(prev_wd, "TO") == 0 || strcmp(prev_wd, "=") == 0))
 	{
+		/* special cased code for individual GUCs */
 		if (pg_strcasecmp(prev2_wd, "DateStyle") == 0)
 		{
 			static const char *const my_list[] =
@@ -3604,20 +3617,6 @@ psql_completion(const char *text, int start, int end)
 
 			COMPLETE_WITH_LIST(my_list);
 		}
-		else if (pg_strcasecmp(prev2_wd, "IntervalStyle") == 0)
-		{
-			static const char *const my_list[] =
-			{"postgres", "postgres_verbose", "sql_standard", "iso_8601", NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
-		else if (pg_strcasecmp(prev2_wd, "GEQO") == 0)
-		{
-			static const char *const my_list[] =
-			{"ON", "OFF", "DEFAULT", NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
 		else if (pg_strcasecmp(prev2_wd, "search_path") == 0)
 		{
 			COMPLETE_WITH_QUERY(Query_for_list_of_schemas
@@ -3627,10 +3626,34 @@ psql_completion(const char *text, int start, int end)
 		}
 		else
 		{
-			static const char *const my_list[] =
-			{"DEFAULT", NULL};
+			/* generic, type based, GUC support */
 
-			COMPLETE_WITH_LIST(my_list);
+			char	   *guctype = get_guctype(prev2_wd);
+
+			if (guctype && strcmp(guctype, "enum") == 0)
+			{
+char		querybuf[1024];
+
+snprintf(querybuf, 1024, Query_for_enum, prev2_wd);
+COMPLETE_WITH_QUERY(querybuf);
+			}
+			else if (guctype && strcmp(guctype, "bool") == 0)
+			{
+static const char *const my_list[] =
+{"on", "off", "true", "false", "yes", "no", "1", "0", "DEFAULT", NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
+			else
+			{
+static const char *const my_list[] =
+{"DEFAULT", NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
+
+			if (guctype)
+free(guctype);
 		}
 	}
 
@@ -4173,30 +4196,15 @@ _complete_from_query(int is_schema_query, const char *text, int state)
 		result = NULL;
 
 		/* Set up suitably-escaped copies of textual inputs */
-		e_text = pg_malloc(string_length * 2 + 1);
-		PQescapeString(e_text, text, string_length);
+		e_text = escape_string(text);
 
 		if