Re: [HACKERS] pg_stat_activity.waiting_start

[Joel Jacobson]

Attached is a patch implementing the seconds-resolution wait_start, but
presented as a timestamptz to the user, just like the other *_start fields:

commit c001e5c537e36d2683a7e55c7c8bfcc154de4c9d
Author: Joel Jacobson 
Date:   Sat Dec 24 13:20:09 2016 +0700

Add OUT parameter "wait_start" timestamptz to pg_stat_get_activity()
and pg_catalog.pg_stat_activity

This is set to the timestamptz with seconds resolution
when the process started waiting, and reset to NULL
when it's not waiting any longer.

This is useful if you want to know not only what the process is
waiting for, but also how long it has been waiting,
which can be useful in situations when it might be
normal for different users/applications to wait for some amount of time,
but abnormal if they are waiting longer than some threshold.

When such a threshold is exceeded, monitoring applications
could then alert the user or possibly cancel/terminate
the blocking processes.

Without information on how long time processes have been waiting,
the monitoring applications would have no other option than
to cancel/terminate a process as soon as something is waiting,
or keep track of how long time processes have been waiting
by polling and keeping track on a per process basis,
which is less user-friendly than if PostgreSQL would provide
the information directly to the user.

 src/backend/catalog/system_views.sql | 3 ++-
 src/backend/postmaster/pgstat.c  | 1 +
 src/backend/storage/lmgr/proc.c  | 1 +
 src/backend/utils/adt/pgstatfuncs.c  | 7 ++-
 src/include/catalog/pg_proc.h| 2 +-
 src/include/pgstat.h | 6 +-
 src/include/storage/proc.h   | 3 +++
 7 files changed, 19 insertions(+), 4 deletions(-)



On Sat, Dec 24, 2016 at 12:32 PM, Joel Jacobson  wrote:

> On Sat, Dec 24, 2016 at 9:56 AM, Joel Jacobson  wrote:
> >> The difficulty with that is it'd require a gettimeofday() call for
> >> every wait start.  Even on platforms where those are relatively cheap,
>
> I just realized how this can be optimized.
> We only need to set wait_start for every new waiting period,
> not for every wait start, i.e. not for every call to
> pgstat_report_wait_start():
>
> Example:
>
> In pgstat_report_wait_start():
> if (proc->wait_start == 0)
>proc->wait_start = (pg_time_t) time(NULL);
>
> And then in pgstat_report_wait_end():
> proc->wait_start = 0;
>
> This means we only need to call time() or gettimeofday() once per
> waiting period.
>
>
> --
> Joel Jacobson
>



-- 
Joel Jacobson

Mobile: +46703603801
*Trustly.com  | Newsroom
 | LinkedIn
 | **Twitter
*


* *


0001-pg_stat_get_activity_wait_start.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] pg_stat_activity.waiting_start

[Joel Jacobson]

On Sat, Dec 24, 2016 at 9:56 AM, Joel Jacobson  wrote:
>> The difficulty with that is it'd require a gettimeofday() call for
>> every wait start.  Even on platforms where those are relatively cheap,

I just realized how this can be optimized.
We only need to set wait_start for every new waiting period,
not for every wait start, i.e. not for every call to pgstat_report_wait_start():

Example:

In pgstat_report_wait_start():
if (proc->wait_start == 0)
   proc->wait_start = (pg_time_t) time(NULL);

And then in pgstat_report_wait_end():
proc->wait_start = 0;

This means we only need to call time() or gettimeofday() once per
waiting period.


-- 
Joel Jacobson


-- 
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_stat_activity.waiting_start

[Joel Jacobson]

On Sat, Dec 24, 2016 at 9:00 AM, Tom Lane  wrote:
>> I would like to propose adding a fourth such column, "waiting_start",
>> which would tell how long time a backend has been waiting.
>
> The difficulty with that is it'd require a gettimeofday() call for
> every wait start.  Even on platforms where those are relatively cheap,
> the overhead would be nasty --- and on some platforms, it'd be
> astonishingly bad.  We sweated quite a lot to get the overhead of
> pg_stat_activity wait monitoring down to the point where it would be
> tolerable for non-heavyweight locks, but I'm afraid this would push
> it back into the not-tolerable range.

I don't think we need the microsecond resolution provided by
gettimeofday() via GetCurrentTimestamp()
It would be enough to know which second the waiting started, so we
could use time().

gettimeofday() takes 42 cycles.
time() only takes 3 cycles. [1]

[1] http://stackoverflow.com/questions/6498972/faster-equivalent-of-gettimeofday


-- 
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] Indirect indexes

[Alvaro Herrera]

Alvaro Herrera wrote:

> There are a few broken things yet, such as "REINDEX TABLE pg_class" and
> some other operations specifically on pg_class.  This one in particular
> breaks the regression tests, but that shouldn't be terribly difficult to
> fix.

This version fixes this problem, so the regression tests now pass.
I fixed it by adding yet another index attribute bitmapset to
RelationData, so we keep track of "all indexed columns" separately from
"columns used by regular indexes" and "columns used by indirect
indexes".  A possible optimization is to remove the first list and just
keep "indirect" and "direct", and in the only case where we need all of
them, do a bms_union -- it's not performance-critical anyway.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1b45a4c..9f899c7 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -92,6 +92,7 @@ brinhandler(PG_FUNCTION_ARGS)
amroutine->amstorage = true;
amroutine->amclusterable = false;
amroutine->ampredlocks = false;
+   amroutine->amcanindirect = false;
amroutine->amkeytype = InvalidOid;
 
amroutine->ambuild = brinbuild;
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index f07eedc..1bc91d2 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -49,6 +49,7 @@ ginhandler(PG_FUNCTION_ARGS)
amroutine->amstorage = true;
amroutine->amclusterable = false;
amroutine->ampredlocks = false;
+   amroutine->amcanindirect = false;
amroutine->amkeytype = InvalidOid;
 
amroutine->ambuild = ginbuild;
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index b8aa9bc..4ec34d5 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -69,6 +69,7 @@ gisthandler(PG_FUNCTION_ARGS)
amroutine->amstorage = true;
amroutine->amclusterable = true;
amroutine->ampredlocks = false;
+   amroutine->amcanindirect = false;
amroutine->amkeytype = InvalidOid;
 
amroutine->ambuild = gistbuild;
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 0eeb37d..7cfb776 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -66,6 +66,7 @@ hashhandler(PG_FUNCTION_ARGS)
amroutine->amstorage = false;
amroutine->amclusterable = false;
amroutine->ampredlocks = false;
+   amroutine->amcanindirect = false;
amroutine->amkeytype = INT4OID;
 
amroutine->ambuild = hashbuild;
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b019bc1..76518cc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -96,10 +96,10 @@ static XLogRecPtr log_heap_update(Relation reln, Buffer 
oldbuf,
HeapTuple newtup, HeapTuple old_key_tup,
bool all_visible_cleared, bool 
new_all_visible_cleared);
 static void HeapSatisfiesHOTandKeyUpdate(Relation relation,
-Bitmapset *hot_attrs,
-Bitmapset *key_attrs, 
Bitmapset *id_attrs,
+Bitmapset *hot_attrs, 
Bitmapset *key_attrs,
+Bitmapset *id_attrs, 
Bitmapset *indirect_attrs,
 bool *satisfies_hot, 
bool *satisfies_key,
-bool *satisfies_id,
+bool *satisfies_id, 
Bitmapset **unchanged_attrs,
 HeapTuple oldtup, 
HeapTuple newtup);
 static bool heap_acquire_tuplock(Relation relation, ItemPointer tid,
 LockTupleMode mode, LockWaitPolicy 
wait_policy,
@@ -3414,6 +3414,8 @@ simple_heap_delete(Relation relation, ItemPointer tid)
  * crosscheck - if not InvalidSnapshot, also check old tuple against this
  * wait - true if should wait for any conflicting update to commit/abort
  * hufd - output parameter, filled in failure cases (see below)
+ * unchanged_ind_cols - output parameter; bits set for unmodified columns
+ * that are indexed by indirect indexes
  * lockmode - output parameter, filled with lock mode acquired on tuple
  *
  * Normal, successful return value is HeapTupleMayBeUpdated, which
@@ -3436,13 +3438,15 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 HTSU_Result
 heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
CommandId cid, Snapshot crosscheck, bool 

Re: [HACKERS] pg_stat_activity.waiting_start

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Joel Jacobson  writes:
> > We already have xact_start, query_start and backend_start
> > to get the timestamptz for when different things happened.
> 
> > I would like to propose adding a fourth such column, "waiting_start",
> > which would tell how long time a backend has been waiting.
> 
> The difficulty with that is it'd require a gettimeofday() call for
> every wait start.  Even on platforms where those are relatively cheap,
> the overhead would be nasty --- and on some platforms, it'd be
> astonishingly bad.  We sweated quite a lot to get the overhead of
> pg_stat_activity wait monitoring down to the point where it would be
> tolerable for non-heavyweight locks, but I'm afraid this would push
> it back into the not-tolerable range.

Could we handle this like log_lock_waits..?

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_stat_activity.waiting_start

[Tom Lane]

Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> The difficulty with that is it'd require a gettimeofday() call for
>> every wait start.  Even on platforms where those are relatively cheap,
>> the overhead would be nasty --- and on some platforms, it'd be
>> astonishingly bad.  We sweated quite a lot to get the overhead of
>> pg_stat_activity wait monitoring down to the point where it would be
>> tolerable for non-heavyweight locks, but I'm afraid this would push
>> it back into the not-tolerable range.

> Could we handle this like log_lock_waits..?

Well, that only applies to heavyweight locks, which do a gettimeofday
anyway in order to schedule the deadlock-check timeout.  If you were
willing to populate this new column only for heavyweight locks, maybe it
could be done for minimal overhead.  But that would be backsliding
quite a lot compared to what we just did to extend pg_stat_activity's
coverage of lock types.

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] pg_stat_activity.waiting_start

[Tom Lane]

Joel Jacobson  writes:
> We already have xact_start, query_start and backend_start
> to get the timestamptz for when different things happened.

> I would like to propose adding a fourth such column, "waiting_start",
> which would tell how long time a backend has been waiting.

The difficulty with that is it'd require a gettimeofday() call for
every wait start.  Even on platforms where those are relatively cheap,
the overhead would be nasty --- and on some platforms, it'd be
astonishingly bad.  We sweated quite a lot to get the overhead of
pg_stat_activity wait monitoring down to the point where it would be
tolerable for non-heavyweight locks, but I'm afraid this would push
it back into the not-tolerable range.

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] pg_stat_activity.waiting_start

[Joel Jacobson]

Actually, "wait_start" is a better name to match the others
("wait_event_type" and "wait_event").


On Sat, Dec 24, 2016 at 8:20 AM, Joel Jacobson  wrote:
> Hi hackers,
>
> We already have xact_start, query_start and backend_start
> to get the timestamptz for when different things happened.
>
> I would like to propose adding a fourth such column, "waiting_start",
> which would tell how long time a backend has been waiting.
>
> The column would be NULL when waiting=FALSE.
>
> While it's trivial to write a script that just polls pg_stat_activity
> every second and keeps tack of when a backend started
> waiting by just checking for any new waiting=TRUE rows,
> it would be more convenient to just get the information from
> pg_stat_activity directly.
>
> The use-case would be e.g. monitoring tools
> when you want to know how long time queries are waiting.
>
> --
> Joel Jacobson



-- 
Joel Jacobson

Mobile: +46703603801
Trustly.com | Newsroom | LinkedIn | Twitter


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

[HACKERS] pg_stat_activity.waiting_start

[Joel Jacobson]

Hi hackers,

We already have xact_start, query_start and backend_start
to get the timestamptz for when different things happened.

I would like to propose adding a fourth such column, "waiting_start",
which would tell how long time a backend has been waiting.

The column would be NULL when waiting=FALSE.

While it's trivial to write a script that just polls pg_stat_activity
every second and keeps tack of when a backend started
waiting by just checking for any new waiting=TRUE rows,
it would be more convenient to just get the information from
pg_stat_activity directly.

The use-case would be e.g. monitoring tools
when you want to know how long time queries are waiting.

-- 
Joel Jacobson


-- 
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] Remove lower limit on checkpoint_timeout?

[David Steele]

On 12/23/16 7:26 PM, Tom Lane wrote:
> David Steele  writes:
>> What about a ./configure option that basically removes the min/max
>> limits of every setting where it makes sense?
> 
> It's pretty much never the case that anything goes; for example,
> are we going to insist that the code be able to respond sanely to
> negative checkpoint_timeout?  The GUC limit mechanism was really
> invented to avoid having to do that, as much or more than preventing
> users from picking "bad" values.

Well, this option would only be available if you built Postgres from
source which I doubt most users are doing.  I had originally thought it
would be best to enforce some sanity on the values, but then thought it
might be useful to give settings completely bogus values for testing
purposes.

I'd be OK with some sensible limits, but I think that defeats to point
to some extent.

> And I don't want to maintain two sets of limits, so I'm not for
> some sort of "training wheels off" vs "training wheels on" GUC.

I wasn't proposing that this be a GUC.  If I gave that impression it's
because I didn't convey my idea accurately.

> We could move towards a project policy that limits be set according
> to what's sensible for the code to support rather than what seems
> like useful ranges.  Again though, most of the ensuing work needs to
> be documentation not code changes.

I'm honestly not sure I would want to kick off all the training wheels.
For instance, a poorly considered checkpoint_timeout setting may lead to
terrible performance, but should still leave the user with a consistent
database, and hopefully some wisdom in the bargain.

-- 
-David
da...@pgmasters.net


-- 
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] Remove lower limit on checkpoint_timeout?

[Tom Lane]

David Steele  writes:
> What about a ./configure option that basically removes the min/max
> limits of every setting where it makes sense?

It's pretty much never the case that anything goes; for example,
are we going to insist that the code be able to respond sanely to
negative checkpoint_timeout?  The GUC limit mechanism was really
invented to avoid having to do that, as much or more than preventing
users from picking "bad" values.

And I don't want to maintain two sets of limits, so I'm not for
some sort of "training wheels off" vs "training wheels on" GUC.

We could move towards a project policy that limits be set according
to what's sensible for the code to support rather than what seems
like useful ranges.  Again though, most of the ensuing work needs to
be documentation not code changes.

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] Clarifying "server starting" messaging in pg_ctl start without --wait

[Jim Nasby]

On 12/23/16 6:10 PM, Tom Lane wrote:

Michael Paquier  writes:

Is there still a use case for --no-wait in the real world?


Sure.  Most system startup scripts aren't going to want to wait.
If we take it out those people will go back to starting the postmaster
by hand.


Presumably they could just background it... since it's not going to be 
long-lived it's presumably not that big a deal. Though, seems like many 
startup scripts like to make sure what they're starting is actually working.


What might be interesting is a mode that waited for everything but 
recovery so at least you know the config is valid, the port is 
available, etc. That would be much harder to handle externally.



--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


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

[HACKERS] Compiler warning

[Bruce Momjian]

I am seeing this compiler warning in the 9.4 branch:

 9.4:  basebackup.c:1284:6: warning: variable 'wait_result' set but not used 
[-Wunused-but-set-variable]

This is on Debian Jessie with gcc version 4.9.2.  It is from this commit:

commit f6508827afe76b2c3735a9ce073620e708d60c79
Author: Magnus Hagander 
Date:   Mon Dec 19 10:11:04 2016 +0100

Fix base backup rate limiting in presence of slow i/o

When source i/o on disk was too slow compared to the rate limiting
specified, the system could end up with a negative value for sleep 
that
it never got out of, which caused rate limiting to effectively be
turned off.

Discussion: 
https://postgr.es/m/CABUevEy_-e0YvL4ayoX8bH_Ja9w%2BBHoP6jUgdxZuG2nEj3uAfQ%40mail.gmail.com

Analysis by me, patch by Antonin Houska

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] Time to retire Windows XP buildfarm host?

[Tom Lane]

Alvaro Herrera  writes:
> Andrew Dunstan wrote:
>> ... and miraculously it has fixed itself.

> And it failed again today, once.

> Today I noticed that it's running gcc 4.5.0.  But for the 4.5 branch,
> the GCC guys put out a few releases before abandoning it, and there are
> some compiler segmentation faults fixed in some of these releases,

Meh... did they fix any nondeterministic bugs?  My money is on the
hardware getting flaky.

> I think it'd be worthwhile to upgrade to the latest in that branch.

Still, that might be worth doing if it's not too painful.

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] Clarifying "server starting" messaging in pg_ctl start without --wait

[Tom Lane]

Michael Paquier  writes:
> Is there still a use case for --no-wait in the real world?

Sure.  Most system startup scripts aren't going to want to wait.
If we take it out those people will go back to starting the postmaster
by hand.

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] proposal: session server side variables

[Jim Nasby]

On 12/23/16 4:24 PM, Fabien COELHO wrote:

I think that a special purpose variable infrastructure implied by your
remark is just starting from the end point. The first three points seem
relevant too because they help focus on other issues.


If you want to ignore performance, there are things you can do with 
non-transactional variables that are simply not possible with tables. 
But even ignoring that, the performance cost of temp tables is massive 
compared to variables. Not only is the access far more complex, but 
bloating is a major problem (both in the table itself as well as in the 
catalog). That's part of the driver for all the discussion about things 
like permanent temp tables (which still leaves a bloat and performance 
problem in the table itself).

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
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] propose to pushdown qual into EXCEPT

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> That is not an adequate argument for such a change being okay.  Postgres,
>> with its extensible set of datatypes, has to be much more careful about
>> the semantic soundness of optimizations than some other DBs do.

> Can we use the record_image_ops mechanism here?

Don't see how that helps?

But I went through the logic and think I convinced myself that pushdown
to LHS only is okay, and no more questionable than what happens for
INTERSECT.  Argue thus:

Assume a group of non-distinct-according-to-the-setop rows includes m rows
from left-hand side, n rows from right-hand side, of which the upper qual
would pass m' <= m and n' <= n rows respectively.

UNION: unoptimized setop produces 1 row (since we may assume the group is
nonempty).  That row might or might not pass the upper qual, so we get
0 or 1 row out.  User can be certain about what will happen only if
m'=m and n'=n (row must pass qual) or m'=n'=0 (row must not pass qual).

After optimizing by pushing qual down: we get one row if m' > 0 or n' > 0.
This is equivalent to the unoptimized behavior if we assume that the setop
always magically selects a representative row passing the qual if
possible, so OK.

INTERSECT: unoptimized setop produces 1 row if m>0 and n>0, which again
might or might not pass the upper qual, so 0 or 1 row out.

Optimized case: we get 1 row if m' > 0 and n' > 0.  This is equivalent to
unoptimized behavior if you allow that the representative row could have
been taken from either input.  (For example, if m' = m, n > 0, and n' = 0,
producing 0 rows is legit only if the hypothetical representative row was
taken from RHS.)  That's legal per SQL, though I'm unsure offhand if our
implementation actually would do that.

INTERSECT ALL: unoptimized setop produces min(m,n) rows, which might or
might not pass the upper qual, so anywhere from 0 to min(m,n) rows out.
(It is probably possible to set a tighter upper bound when m', n' are
small, but I've not bothered to reason that out.)

Optimized case: we get min(m', n') rows out, which seems fine; again
it could be produced by the unoptimized implementation selecting
representative row(s) that happen to have that many members passing the
upper qual.

But actually, our implementation doesn't produce some random collection
of min(m,n) elements of the row group.  What you get is min(m,n) copies
of a single representative row; therefore, assuming the upper qual is
nonvolatile, either they all pass or none do, so the actual output is
either 0 or min(m,n) duplicates.  (The spec says "Let R be a row that is a
duplicate of some row in ET1 or of some row in ET2 or both.  Let m be the
number of duplicates of R in ET1 and let n be the number of duplicates of
R in ET2, where m >= 0 and n >= 0.  [For INTERSECT ALL] the number of
duplicates of R that T contains is the minimum of m and n."  So depending
on what you want to assume that "duplicates of R" means, you could argue
that the spec actually requires our implementation's behavior here.
It certainly doesn't appear to forbid it.)  The optimized output is
therefore distinguishable from unoptimized, because it could produce a
number of rows that the unoptimized implementation cannot.  But nobody's
complained about it, and it's hard to see there being a legitimate beef.

EXCEPT: unoptimized setop produces 1 row if m>0 and n=0, which again
might or might not pass the upper qual, so 0 or 1 row out.

If we were to push qual into both sides: we'd get one row if m' > 0
and n' = 0.  So if the qual passes some of LHS and none of RHS, we'd
get a row, breaking SQL semantics.

If we push into LHS only: we get one row if m' > 0 and n = 0.  Certainly
then m > 0.  So this is equivalent to behavior where the setop magically
selects a representative LHS row passing the qual.  OTOH, if m' = 0, then
the setop wouldn't produce a row, but if it had that row would have failed
the upper qual, so behavior is still equivalent.

EXCEPT ALL: unoptimized setop produces max(m-n,0) rows, which might or
might not pass the upper qual, so anywhere from 0 to max(m-n,0) rows out.
(Again it is probably possible to set a tighter upper bound when m', n'
are small.)

Again, pushing into RHS would allow n to be reduced, perhaps allowing more
rows to be returned than SQL would allow, so we can't do that.

If we push into LHS only: we get max(m'-n,0) rows.  Again, it would be
possible for the unoptimized setop to select its representative rows in
such a way that this happens (it just has to prefer LHS rows passing the
qual over those that don't).

Again, this doesn't quite match reality of the current setop
implementation, since it returns max(m-n,0) identical rows, not that
many random members of the current group.  But it's still hard to believe
anyone would have a legitimate beef about it.


tl;dr: I now think what the patch proposes to do is legit.  There's a heck
of a lot of work to be done on the comments it's 

Re: [HACKERS] Remove lower limit on checkpoint_timeout?

[David Steele]

On 12/23/16 6:12 PM, Michael Paquier wrote:
> On Sat, Dec 24, 2016 at 6:02 AM, David Steele  wrote:
>> On 12/23/16 10:24 AM, Tom Lane wrote:
>>> Andres Freund  writes:
 While it's not a particularly good idea to set it to 1s on a production
 system, I don't see why we need to prevent that. It's not like 30s is
 likely to be a good idea either.
>>>
 Hence I'd like to set the lower limit to 1s.
>>>
>>> OK, but the documentation for it needs some work if you're going to
>>> do that.  It only warns against making the timeout too large, not
>>> too small.
>>
>> +1 for the lower limit and the docs.
> 
> I wish we were more loose regarding the limits of some parameters, see
> for example this recent thread about bgwriter ones:
> https://www.postgresql.org/message-id/f6e58a22-030b-eb8a-5457-f62fb08d7...@bluetreble.com
> So +1 for lowering checkpoint_timeout, lower values are stupid for
> production systems, but not for developers.

What about a ./configure option that basically removes the min/max
limits of every setting where it makes sense?  We can discuss what's
good for the user without being inconvenienced ourselves.

It's something I'd be willing to write a patch for if there's interest.
We can start with a few that have been discussed and work our way out
from there.

I agree with Andres about lowering the checkpoint_timeout limit in
general, but it seems like this would allow us to make development more
convenient even when we can't agree on changing limits for production
builds.

-- 
-David
da...@pgmasters.net


-- 
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] Time to retire Windows XP buildfarm host?

[Michael Paquier]

On Sat, Dec 24, 2016 at 7:48 AM, Alvaro Herrera
 wrote:
> Andrew Dunstan wrote:
>
>> ... and miraculously it has fixed itself.
>
> And it failed again today, once.
>
> Today I noticed that it's running gcc 4.5.0.  But for the 4.5 branch,
> the GCC guys put out a few releases before abandoning it, and there are
> some compiler segmentation faults fixed in some of these releases,
> visible here:
>
> https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.1
> https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.2
> https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.3
> https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.4
>
> I think it'd be worthwhile to upgrade to the latest in that branch.

FWIW, I also saw segmentation faults with MinGW using gcc 4.8.1 in my
environments, after fetching the newest supported by the builds :(
Any configuration on Windows out of MSVC is depressing.
-- 
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] Remove lower limit on checkpoint_timeout?

[Michael Paquier]

On Sat, Dec 24, 2016 at 6:02 AM, David Steele  wrote:
> On 12/23/16 10:24 AM, Tom Lane wrote:
>> Andres Freund  writes:
>>> While it's not a particularly good idea to set it to 1s on a production
>>> system, I don't see why we need to prevent that. It's not like 30s is
>>> likely to be a good idea either.
>>
>>> Hence I'd like to set the lower limit to 1s.
>>
>> OK, but the documentation for it needs some work if you're going to
>> do that.  It only warns against making the timeout too large, not
>> too small.
>
> +1 for the lower limit and the docs.

I wish we were more loose regarding the limits of some parameters, see
for example this recent thread about bgwriter ones:
https://www.postgresql.org/message-id/f6e58a22-030b-eb8a-5457-f62fb08d7...@bluetreble.com
So +1 for lowering checkpoint_timeout, lower values are stupid for
production systems, but not for developers.
-- 
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] Time to retire Windows XP buildfarm host?

[Alvaro Herrera]

Andrew Dunstan wrote:

> ... and miraculously it has fixed itself.

And it failed again today, once.

Today I noticed that it's running gcc 4.5.0.  But for the 4.5 branch,
the GCC guys put out a few releases before abandoning it, and there are
some compiler segmentation faults fixed in some of these releases,
visible here:

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.1
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.2
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.3
https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=RESOLVED=FIXED_milestone=4.5.4

I think it'd be worthwhile to upgrade to the latest in that branch.

-- 
Álvaro Herrerahttps://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] Incautious handling of overlength identifiers

[Joe Conway]

On 12/23/2016 12:44 PM, Tom Lane wrote:
> I wrote:
>> So what to do?  We could run around and fix these individual cases
>> and call it good, but if we do, I will bet a very fine dinner that
>> more such errors will sneak in before long.  Seems like we need a
>> coding convention that discourages just randomly treating a C string
>> as a valid value of type NAME.  Not sure how to get there though.
> 
> An alternative worth considering, especially for the back branches,
> is simply to remove the Assert in hashname().  That would give us
> the behavior that non-developers see anyway, which is that these
> functions always fail to match overlength names, whether or not
> the names would have matched after truncation.  Trying to apply
> truncation more consistently could be left as an improvement
> project for later.

That sounds reasonable to me.

Joe

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] Clarifying "server starting" messaging in pg_ctl start without --wait

[Michael Paquier]

On Fri, Dec 23, 2016 at 10:47 PM, Peter Eisentraut
 wrote:
> On 12/20/16 3:43 PM, Peter Eisentraut wrote:
>> On 12/20/16 3:31 PM, Ryan Murphy wrote:
>>> I'm concerned some new users may not understand this behavior of pg_ctl,
>>> so I wanted to suggest that we add some additional messaging after
>>> "server starting" - something like:
>>>
>>> $ pg_ctl -D datadir -l logfile start
>>> server starting
>>> (to wait for confirmation that server actually started, try pg_ctl again
>>> with --wait)
>>
>> Maybe the fix is to make --wait the default?
>
> Here is a patch for that.

Is there still a use case for --no-wait in the real world? Why not
simply ripping it out?
-- 
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] proposal: session server side variables

[Fabien COELHO]

Hello Tom,


Hmmm. If a variable is implemented as a one row table, then constraints
are already available there, as well as grant & revoke, they can be any
type including composite, nearly nothing to implement to get...



A "one row" table would be a CREATE + one INSERT, UPDATE allowed, further
INSERT and DELETE are disallowed by construction. Then some syntactic
sugar for variables (session => temporary table, persistent => standard
table). Note sure about a "transaction variable", though... maybe an
[unlogged] table automatically dropped on commit?


I think it's entirely silly


Thanks for "silly". Last time it was "academic". Is it better? :-)

to be inventing something that's morally a one-row table, when we 
already have perfectly good one-row tables.


Hmmm. Although I can think of ways to ensure that a table is one row 
(unique, check, trigger, rules, whatever...), none are straightforward.



The value of a server-side variable facility would be mostly that it
doesn't have all the overhead implied by tables.


I do not know that. I think that discussing semantics and syntax does
have value as well.

ISTM that there are 4 intermixed issues related to server-side variables:

 (1) what should be their possible semantics and capabilities
 e.g. with respect to transactions, permissions, namespace, types,
   constraints, ...

 => what is the use cases for them?

 (2) what should be their syntax

 => what do you want do write to use them?

 (3) how to implement them

 (4) how to optimize the implementation, eventually

I think that a special purpose variable infrastructure implied by your 
remark is just starting from the end point. The first three points seem 
relevant too because they help focus on other issues.


I'm not claiming that having variables as one-row tables is the best ever 
solution, but I do not think that it is such a terrible idea either. At 
least it provides a clear and consistent range of existing semantics out 
of the box (unlogged/temporary/standard, permissions...), thus provide a 
clean model for discussion, even if it is rejected for some reason 
afterwards.


Also it seems easy to implement (as syntactic sugar) and play with, and 
I'm not sure that sure that performance would be that bad, as for session 
(temporary unlogged?) variables they would probably simply stay in cache. 
If the performance is an issue in concrete use cases they could be 
optimized in the end, to reduce time and space, which would be great.


I think that is a direct reason not to think about overhead like 
constraints, as well.


Hmmm. If there is no constraint, the overhead is limited? If one has use 
for constraints, then they can pay the overhead?


--
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] propose to pushdown qual into EXCEPT

[Alvaro Herrera]

Tom Lane wrote:
> "=?ISO-8859-1?B?QXJtb3I=?="  writes:
> > Because PG does not pushdown qual to the none of the subquery. And I 
> > check the source code, find some comments in 
> > src/backend/optimizer/path/allpaths.c, which says "If the subquery 
> > contains EXCEPT or EXCEPT ALL set ops we cannot push quals into it, because 
> > that could change the results".
> > However, for this case, I think we can pushdown qual to the  left most 
> > subquery of EXCEPT, just like other database does.
> 
> That is not an adequate argument for such a change being okay.  Postgres,
> with its extensible set of datatypes, has to be much more careful about
> the semantic soundness of optimizations than some other DBs do.

Can we use the record_image_ops mechanism here?

-- 
Álvaro Herrerahttps://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] propose to pushdown qual into EXCEPT

[Tom Lane]

"=?ISO-8859-1?B?QXJtb3I=?="  writes:
> Because PG does not pushdown qual to the none of the subquery. And I 
> check the source code, find some comments in 
> src/backend/optimizer/path/allpaths.c, which says "If the subquery 
> contains EXCEPT or EXCEPT ALL set ops we cannot push quals into it, because 
> that could change the results".
> However, for this case, I think we can pushdown qual to the  left most 
> subquery of EXCEPT, just like other database does.

That is not an adequate argument for such a change being okay.  Postgres,
with its extensible set of datatypes, has to be much more careful about
the semantic soundness of optimizations than some other DBs do.

The existing behavior here dates to commit 0201dac1c319599a, which was
inspired by this thread:
https://www.postgresql.org/message-id/flat/46C15C39FEB2C44BA555E356FBCD6FA48879A6%40m0114.s-mxs.net
(BTW, several of the concrete examples discussed in that thread no longer
apply because of later changes.  But the problem still exists.)

We had convinced ourselves that pushing down a qual into UNION and
INTERSECT cases is okay even if the qual can distinguish rows that the
setop comparisons see as equal, because you would get results consistent
with the setop having chosen some legitimate set of representative row(s)
for each group of "duplicate" rows.  However that did not seem to apply to
EXCEPT, or at least I wasn't convinced enough to risk it.  I'm still not,
without a worked-out argument as to why it's okay.  In particular I'd like
to see a proof that establishes (a) why it is or is not okay to push into
the right-hand side of EXCEPT, and (b) whether ALL does or does not make a
difference.

Now you could argue that we threw all these fine points out the window
with respect to window functions in commit d222585a9f7a18f2, so maybe
it's okay to do it with respect to EXCEPT as well.  But that would lead
to deciding it's okay to push into both sides of EXCEPT, which is still
not what this patch does.  Anyway I'm not very pleased by the idea that
we'd hold EXCEPT to a weaker semantic standard than UNION and INTERSECT.

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] Improvements in psql hooks for variables

[Daniel Verite]

Rahila Syed wrote:

> Kindly consider following comments,

Sorry for taking so long to post an update.

> There should not be an option to the caller to not follow the behaviour of
> setting valid to either true/false.

OK, fixed.

> In following examples, incorrect error message is begin displayed.
> “ON_ERROR_ROLLBACK” is an enum and also
> accepts value 'interactive' .  The error message says boolean expected.

Indeed. Fixed for all callers of ParseVariableBool() than can accept 
non-boolean arguments too.


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a9a2fdb..46bcf19 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -254,25 +254,30 @@ exec_command(const char *cmd,
if (opt1 != NULL && strncmp(opt1, prefix, sizeof(prefix) - 1) 
== 0)
{
reuse_previous =
-   ParseVariableBool(opt1 + sizeof(prefix) - 1, 
prefix) ?
+   ParseVariableBool(opt1 + sizeof(prefix) - 1, 
prefix, ) ?
TRI_YES : TRI_NO;
-
-   free(opt1);
-   opt1 = read_connect_arg(scan_state);
+   if (success)
+   {
+   free(opt1);
+   opt1 = read_connect_arg(scan_state);
+   }
}
else
reuse_previous = TRI_DEFAULT;
 
-   opt2 = read_connect_arg(scan_state);
-   opt3 = read_connect_arg(scan_state);
-   opt4 = read_connect_arg(scan_state);
+   if (success)/* do not attempt to connect if 
reuse_previous argument was invalid */
+   {
+   opt2 = read_connect_arg(scan_state);
+   opt3 = read_connect_arg(scan_state);
+   opt4 = read_connect_arg(scan_state);
 
-   success = do_connect(reuse_previous, opt1, opt2, opt3, opt4);
+   success = do_connect(reuse_previous, opt1, opt2, opt3, 
opt4);
 
+   free(opt2);
+   free(opt3);
+   free(opt4);
+   }
free(opt1);
-   free(opt2);
-   free(opt3);
-   free(opt4);
}
 
/* \cd */
@@ -1548,7 +1553,11 @@ exec_command(const char *cmd,

 OT_NORMAL, NULL, false);
 
if (opt)
-   pset.timing = ParseVariableBool(opt, "\\timing");
+   {
+   boolnewval = ParseVariableBool(opt, "\\timing", 
);
+   if (success)
+   pset.timing = newval;
+   }
else
pset.timing = !pset.timing;
if (!pset.quiet)
@@ -2660,7 +2669,18 @@ do_pset(const char *param, const char *value, 
printQueryOpt *popt, bool quiet)
if (value && pg_strcasecmp(value, "auto") == 0)
popt->topt.expanded = 2;
else if (value)
-   popt->topt.expanded = ParseVariableBool(value, param);
+   {
+   boolvalid;
+   boolnewval = ParseVariableBool(value, NULL, );
+   if (valid)
+   popt->topt.expanded = newval;
+   else
+   {
+   psql_error("unrecognized value \"%s\" for 
\"%s\"\n",
+  value, param);
+   return false;
+   }
+   }
else
popt->topt.expanded = !popt->topt.expanded;
}
@@ -2669,7 +2689,14 @@ do_pset(const char *param, const char *value, 
printQueryOpt *popt, bool quiet)
else if (strcmp(param, "numericlocale") == 0)
{
if (value)
-   popt->topt.numericLocale = ParseVariableBool(value, 
param);
+   {
+   boolvalid;
+   boolnewval = ParseVariableBool(value, param, 
);
+   if (valid)
+   popt->topt.numericLocale = newval;
+   else
+   return false;
+   }
else
popt->topt.numericLocale = !popt->topt.numericLocale;
}
@@ -2724,7 +2751,18 @@ do_pset(const char *param, const char *value, 
printQueryOpt *popt, bool quiet)
else if (strcmp(param, "t") == 0 || strcmp(param, "tuples_only") == 0)
{
if (value)

Re: [HACKERS] Remove lower limit on checkpoint_timeout?

[David Steele]

On 12/23/16 10:24 AM, Tom Lane wrote:
> Andres Freund  writes:
>> While it's not a particularly good idea to set it to 1s on a production
>> system, I don't see why we need to prevent that. It's not like 30s is
>> likely to be a good idea either.
> 
>> Hence I'd like to set the lower limit to 1s.
> 
> OK, but the documentation for it needs some work if you're going to
> do that.  It only warns against making the timeout too large, not
> too small.

+1 for the lower limit and the docs.

-- 
-David
da...@pgmasters.net


-- 
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] Incautious handling of overlength identifiers

[Tom Lane]

I wrote:
> So what to do?  We could run around and fix these individual cases
> and call it good, but if we do, I will bet a very fine dinner that
> more such errors will sneak in before long.  Seems like we need a
> coding convention that discourages just randomly treating a C string
> as a valid value of type NAME.  Not sure how to get there though.

An alternative worth considering, especially for the back branches,
is simply to remove the Assert in hashname().  That would give us
the behavior that non-developers see anyway, which is that these
functions always fail to match overlength names, whether or not
the names would have matched after truncation.  Trying to apply
truncation more consistently could be left as an improvement
project for later.

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] Parallel Index-only scan

[Robert Haas]

On Fri, Dec 23, 2016 at 3:03 PM, Tom Lane  wrote:
> Or in words of one syllable: please do not use nabble to post to the
> community mailing lists.

Many of those words have two syllables, and one has four.

Anyhow, I think three castigating emails from committers in a span of
62 minutes is probably enough for the OP to get the point, unless
someone else REALLY feels the need to pile on.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Parallel Index-only scan

[Tom Lane]

Robert Haas  writes:
> On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
>  wrote:
>> Please find the attached file for the latest version of patch.
>> parallel_index_only_v2.patch
>> 

> We want to have the patch actually attached, not just stored on nabble.

Or in words of one syllable: please do not use nabble to post to the
community mailing lists.  Their habit of stripping attachments is not
the only evil thing about them (although it's the only one I remember
details of ATM).

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

[HACKERS] Incautious handling of overlength identifiers

[Tom Lane]

Pursuant to the report here:
https://www.postgresql.org/message-id/7d0809ee-6f25-c9d6-8e74-5b2967830...@enterprisedb.com
I tried to test all the built-in functions that take "text" (rather
than "name") arguments representing cataloged objects.  I was able
to provoke the same assertion failure in hashname() from these:

regression=# select has_column_privilege('postgres','tenk1',repeat('z', 
200),repeat('q', 200));
server closed the connection unexpectedly
regression=# select has_language_privilege('postgres',repeat('y', 
200),repeat('z', 200));
server closed the connection unexpectedly
regression=# select has_schema_privilege('postgres',repeat('y', 
200),repeat('z', 200));
server closed the connection unexpectedly
regression=# select has_foreign_data_wrapper_privilege('postgres',repeat('y', 
200),repeat('z', 200));
server closed the connection unexpectedly
regression=# select has_server_privilege('postgres',repeat('y', 
200),repeat('z', 200));
server closed the connection unexpectedly
regression=# select pg_get_serial_sequence('tenk1',repeat('x', 200));
server closed the connection unexpectedly
regression=# select pg_get_object_address('table',array[repeat('x', 
200)],array[repeat('y', 200)]);
server closed the connection unexpectedly

(Probably many of the code paths in pg_get_object_address can be made to
crash like this, but I stopped after finding one.)

Fortunately, a non-assert build will not crash like this, it'll just
produce a name-not-found failure.

Quite a lot of other functions that didn't crash produced errors
suggesting that they aren't truncating their inputs to NAMEDATALEN
before doing the lookup.  I think this is not expected behavior
either, since direct SQL references to such objects would always
be truncated.

Of the functions that did truncate, there was a remarkable lack
of consistency about whether they produced NOTICEs warning of
the truncation.  I'm not as concerned about that, but I wonder
whether we ought to have a uniform policy about it.

So what to do?  We could run around and fix these individual cases
and call it good, but if we do, I will bet a very fine dinner that
more such errors will sneak in before long.  Seems like we need a
coding convention that discourages just randomly treating a C string
as a valid value of type NAME.  Not sure how to get there though.

BTW, I also notice that parse_ident() doesn't truncate the identifiers
it parses.  ISTM this is a bad thing; isn't more or less the whole
point of that function to convert a string to a name as the Postgres
parser would do?

Comments?

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] Vacuum: allow usage of more than 1GB of work mem

[Claudio Freire]

On Fri, Dec 23, 2016 at 1:39 PM, Anastasia Lubennikova
 wrote:
>> On Thu, Dec 22, 2016 at 12:22 PM, Claudio Freire 
>> wrote:
>>>
>>> On Thu, Dec 22, 2016 at 12:15 PM, Anastasia Lubennikova
>>>  wrote:

 The following review has been posted through the commitfest application:
 make installcheck-world:  tested, failed
 Implements feature:   not tested
 Spec compliant:   not tested
 Documentation:not tested

 Hi,
 I haven't read through the thread yet, just tried to apply the patch and
 run tests.
 And it seems that the last attached version is outdated now. It doesn't
 apply to the master
 and after I've tried to fix merge conflict, it segfaults at initdb.
>>>
>>>
>>> I'll rebase when I get some time to do it and post an updated version
>>
>> Attached rebased patches. I called them both v3 to be consistent.
>>
>> I'm not sure how you ran it, but this works fine for me:
>>
>> ./configure --enable-debug --enable-cassert
>> make clean
>> make check
>>
>> ... after a while ...
>>
>> ===
>>   All 168 tests passed.
>> ===
>>
>> I reckon the above is equivalent to installcheck, but doesn't require
>> sudo or actually installing the server, so installcheck should work
>> assuming the install went ok.
>
>
> I found the reason. I configure postgres with CFLAGS="-O0" and it causes
> Segfault on initdb.
> It works fine and passes tests with default configure flags, but I'm pretty
> sure that we should fix segfault before testing the feature.
> If you need it, I'll send a core dump.

I just ran it with CFLAGS="-O0" and it passes all checks too:

CFLAGS='-O0' ./configure --enable-debug --enable-cassert
make clean && make -j8 && make check-world

A stacktrace and a thorough description of your build environment
would be helpful to understand why it breaks on your system.


-- 
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: Write Amplification Reduction Method (WARM)

[Alvaro Herrera]

Alvaro Herrera wrote:

> With your WARM and my indirect indexes, plus the additions for for-key
> locks, plus identity columns, there is no longer a real expectation that
> we can exit early from the function.  In your patch, as well as mine,
> there is a semblance of optimization that tries to avoid computing the
> updated_attrs output bitmapset if the pointer is not passed in, but it's
> effectively pointless because the only interesting use case is from
> ExecUpdate() which always activates the feature.  Can we just agree to
> drop that?

I think the only case that gets worse is the path that does
simple_heap_update, which is used for DDL.  I would be very surprised if
a change there is noticeable, when compared to the rest of the stuff
that goes on for DDL commands.

Now, after saying that, I think that a table with a very large number of
columns is going to be affected by this.  But we don't really need to
compute the output bits for every single column -- we only care about
those that are covered by some index.  So we should pass an input
bitmapset comprising all such columns, and the output bitmapset only
considers those columns, and ignores columns not indexed.  My patch for
indirect indexes already does something similar (though it passes a
bitmapset of columns indexed by indirect indexes only, so it needs a
tweak there.)

-- 
Álvaro Herrerahttps://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] Parallel Index-only scan

[Alvaro Herrera]

Robert Haas wrote:
> On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
>  wrote:
> > Please find the attached file for the latest version of patch.
> >
> > parallel_index_only_v2.patch
> > 
> 
> We want to have the patch actually attached, not just stored on nabble.

I think the problem is that Nabble removes the attachment before
resending the email to the list.  The OP should be posting directly
instead of via Nabble.

-- 
Álvaro Herrerahttps://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: Write Amplification Reduction Method (WARM)

[Alvaro Herrera]

I noticed that this patch changes HeapSatisfiesHOTAndKeyUpdate() by
adding one more set of attributes to check, and one more output boolean
flag.  My patch to add indirect indexes also modifies that routine to
add the same set of things.  I think after committing both these
patches, the API is going to be fairly ridiculous.  I propose to use a
different approach.

With your WARM and my indirect indexes, plus the additions for for-key
locks, plus identity columns, there is no longer a real expectation that
we can exit early from the function.  In your patch, as well as mine,
there is a semblance of optimization that tries to avoid computing the
updated_attrs output bitmapset if the pointer is not passed in, but it's
effectively pointless because the only interesting use case is from
ExecUpdate() which always activates the feature.  Can we just agree to
drop that?

If we do drop that, then the function can become much simpler: compare
all columns in new vs. old, return output bitmapset of changed columns.
Then "satisfies_hot" and all the other boolean output flags can be
computed simply in the caller by doing bms_overlap().

Thoughts?

-- 
Álvaro Herrerahttps://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] Parallel Index-only scan

[Robert Haas]

On Fri, Dec 23, 2016 at 12:04 AM, rafia.sabih
 wrote:
> Please find the attached file for the latest version of patch.
>
> parallel_index_only_v2.patch
> 

We want to have the patch actually attached, not just stored on nabble.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] proposal: session server side variables

[Pavel Stehule]

2016-12-23 19:28 GMT+01:00 Fabien COELHO :

>
> because MySQL variables are not declared - and allows assign everywhere -
>>
>
> Ok. I do not do MySQL.
>
> and MSSQL variables are not persistent.
>>
>
> Yep, but they might be?
>
> In one session you can use lot of roles - some code can be used for
>> securing interactive work, some can be for securing some API, sometimes
>> you
>> can secure a access to some sources. You can switch lot of roles by using
>> security definer functions.
>>
>
> Hmmm. Switching role within a transaction. I never did need that... but
> that is a use case.
>

Any application with security definer functions - depends on different
communities - it is used sometimes strongly.


>
> If you need transactional content - then you should to use tables.
>>>
>>> Why not.
>>>
>>> Maybe variables just need be a syntactic convenience around that?
>>>
>>
>> There is pretty similar relation between sequences and tables and
>> variables
>> and tables.
>>
>
> Yep. A sequence is a one row table, so a variable may be also a one row
> table as well, but with more flexibility about its type, and some nice
> syntactic sugar (like SERIAL which is syntactic sugar for CREATE SEQUENCE
> ...).
>
> In first iteration the constraint can be implemented with domains - but
>> there is not any break to implement constraints directly on variables.
>>
>
> Hmmm. If a variable is implemented as a one row table, then constraints
> are already available there, as well as grant & revoke, they can be any
> type including composite, nearly nothing to implement to get...
>
> A "one row" table would be a CREATE + one INSERT, UPDATE allowed, further
> INSERT and DELETE are disallowed by construction. Then some syntactic sugar
> for variables (session => temporary table, persistent => standard table).
> Note sure about a "transaction variable", though... maybe an [unlogged]
> table automatically dropped on commit?
>

Probably we have different expectation from variables. I don't expect so
variable can be changed by any rollback.

What is use case for transactional variables? I miss any experience - I
wrote lot plpgsql lines and newer would it.

When I remove ACID, and allow only one value - then the implementation can
be simple and fast - some next step can be support of expandable types.
Sure - anybody can use temporary tables now and in future. But it is slow -
more now, because we doesn't support global temporary tables. But ACID
needs lot of CPU times, needs possible VACUUM, ...

No ACID variables are simple to implement, simple to directly accessible
from any PL (although I am thinking about better support in 2nd phase for
PLpgSQL).


>
> --
> Fabien.
>

Re: [HACKERS] proposal: session server side variables

[Tom Lane]

Fabien COELHO  writes:
>> In first iteration the constraint can be implemented with domains - but
>> there is not any break to implement constraints directly on variables.

> Hmmm. If a variable is implemented as a one row table, then constraints 
> are already available there, as well as grant & revoke, they can be any 
> type including composite, nearly nothing to implement to get...

> A "one row" table would be a CREATE + one INSERT, UPDATE allowed, further 
> INSERT and DELETE are disallowed by construction. Then some syntactic 
> sugar for variables (session => temporary table, persistent => standard 
> table). Note sure about a "transaction variable", though... maybe an 
> [unlogged] table automatically dropped on commit?

I think it's entirely silly to be inventing something that's morally a
one-row table, when we already have perfectly good one-row tables.
The value of a server-side variable facility would be mostly that it
doesn't have all the overhead implied by tables.  I think that is a
direct reason not to think about overhead like constraints, as well.

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] proposal: session server side variables

[Fabien COELHO]

because MySQL variables are not declared - and allows assign everywhere -


Ok. I do not do MySQL.


and MSSQL variables are not persistent.


Yep, but they might be?


In one session you can use lot of roles - some code can be used for
securing interactive work, some can be for securing some API, sometimes you
can secure a access to some sources. You can switch lot of roles by using
security definer functions.


Hmmm. Switching role within a transaction. I never did need that... but 
that is a use case.



If you need transactional content - then you should to use tables.

Why not.

Maybe variables just need be a syntactic convenience around that?


There is pretty similar relation between sequences and tables and variables
and tables.


Yep. A sequence is a one row table, so a variable may be also a one row 
table as well, but with more flexibility about its type, and some nice 
syntactic sugar (like SERIAL which is syntactic sugar for CREATE SEQUENCE 
...).



In first iteration the constraint can be implemented with domains - but
there is not any break to implement constraints directly on variables.


Hmmm. If a variable is implemented as a one row table, then constraints 
are already available there, as well as grant & revoke, they can be any 
type including composite, nearly nothing to implement to get...


A "one row" table would be a CREATE + one INSERT, UPDATE allowed, further 
INSERT and DELETE are disallowed by construction. Then some syntactic 
sugar for variables (session => temporary table, persistent => standard 
table). Note sure about a "transaction variable", though... maybe an 
[unlogged] table automatically dropped on commit?


--
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] proposal: session server side variables

[Joe Conway]

On 12/23/2016 08:20 AM, Pavel Stehule wrote:
> 2016-12-23 16:27 GMT+01:00 Fabien COELHO:
>> I have often wished I had such a feature, psql client side :-variables are
>> just awful raw text things.

Agreed.

>> A few comments, mostly about the design:
>>
>> 1. persistent objects with temporal unshared typed content. The life of
>>> content should be limited by session or by transaction. The content is
>>> initialized to default (when it is defined) or to NULL when variable is
>>> first accessed in variable' time scope (session, transaction).
>>>
>>> CREATE VARIABLE [schema.]variable type [DEFAULT default_value]
>>> [TRANSACTION|SESION SCOPE]


I haven't looked, but I take it the SQL standard is silent on the issue
of variables?

> I really would to use pg_class as base for metadata of variables -
> conflicts are not possible. I can reuse safe GRANT/REVOKE mechanism ..

That would be very useful.

>> In the long term, What would be the possible scopes?
>>
>> TRANSACTION, SESSION, PERSISTANT ?
>>
>> Would some scopes orthogonal (eg SHARED between sessions for a USER in a
>> DATABASE, SHARED at the cluster level?).
> 
> I have a plan to support TRANSACTION and SESSION scope. Persistent or
> shared scope needs much more complex rules, and some specialized extensions
> will be better.


I can see where persistent variables would be very useful though.


>> 2. accessed with respecting access rights:
>>>
>>> GRANT SELECT|UPDATE|ALL ON VARIABLE variable TO role
>>> REVOKE SELECT|UPDATE|ALL ON VARIABLE variable FROM role
>>
>> At least for transaction and session scopes it does not make sense that
>> they would be accessible outside the session/transaction, so grant/revoke
>> do not seem necessary?
> 
> It is necessary - and I think so it is fundamental feature - any other
> features can be more or less replaced by extensions, but this one cannot or
> not simply  - you have to protect content against some users - some
> cookies, ids have to be protected. It can be used well with RLS.

How would this work for transaction and session scopes though? What
would be the point -- no other access is possible other than what
happens in the session. Do you envision something like

 CREATE VARIABLE foo ...;
 GRANT SELECT ON VARIABLE foo TO bob;
 SET ROLE bob;

?

>> 3. accessed/updated with special function "getvar", "setvar":
>>>
>>> FUNCTION getvar(regclass) RETURNS type
>>> FUNCTION setvar(regclass, type) RETURNS void
>>>
>>
>> From an aesthetical point of view, I do not like that much.
>>
>> If you use CREATE & DROP, then logically you should use ALTER:
>>
>>   CREATE VARIABLE @name TEXT DEFAULT 'calvin';
>> CREATE VARIABLE @name TEXT = 'calvin';
>>   ALTER VARIABLE @name SET VALUE TO 'hobbes';
>> ALTER VARIABLE @name = 'hoobes';
>>   DROP VARIABLE @name;

Makes sense.

>> Maybe "SET" could be an option as well, but it is less logical:
>>
>>   SET @name = 'susie';
>>
>> But then "SET @..." would just be a shortcut for ALTER VARIABLE.

Maybe. Not sure I like that.

> I would to use a SET statement too. But it is used for another target now.
> Using ALTER in this content looks strange to me. It is used for changing
> metadata not a value.
> 
> Next step can be support of SQL statements
> 
> With SQL support you can do
> 
> SELECT varname;

+1

> SELECT * FROM compositevarname;

+1

> UPDATE varname SET value TO xxx;
> UPDATE compositevarname SET field TO xxx;

These need more thought I think.

>> Also a nicer way to reference them would be great, like SQL server.
>>
>>   SELECT * FROM SomeTable WHERE name = @name;
>>
>> A function may be called behind the scene, I'm just arguing about the
>> syntax here...
>>
>> Important question, what nice syntax to assign the result of a query to a
>> variable? Maybe it could be:
>>
>>   SET @name = query-returning-one-row; -- hmmm
>>   SET @name FROM query-returning-one-row; -- maybe better
>>
>> Or:
>>
>>   ALTER VARIABLE @name WITH one-row-query;
>>
>> Special variables could allow to get the number of rows modified by the
>> last option, like in PL/pgSQL but at the SQL level?

I think the SET syntax is growing on me, but I suspect there may be push
back on overloading that syntax.

>> 4. non transactional  - the metadata are transactional, but the content is
>>> not.
>>>
>>
>> Hmmm... Do you mean:
>>
>> CREATE VARIABLE foo INT DEFAULT 1 SCOPE SESSION;
>> BEGIN;
>>   SET @foo = 2;
>> ROLLBACK;
>>
>> Then @foo is 2 despite the roolback? Yuk!

Agreed

> This is similar to sequences.

I don't see how variables really have anything to do with sequences.

> If you need transactional content - then you should to use tables.

I definitely have use-cases where transactional variables would be useful.


>> I think that if the implementation is based on some system table for
>> storage, then you could get the transaction properties for free, and it
>> seems more logical to do so:
>>
>> CREATE TEMPORARY TABLE pg_session_variables(name TEXT PRIMARY KEY, value
>> TEXT, oidtype, ...);
>>

Re: [HACKERS] proposal: session server side variables

[Pavel Stehule]

2016-12-23 18:46 GMT+01:00 Fabien COELHO :

>
> Hello,
>
> I little bit dislike this style - in my proposal the session variables are
>> very near to a sequences - and we have not any special symbols for
>> sequences.
>>
>
> Yep, but we do not need a syntax to reference a sequence either... it is
> automatic and usually hidden behind SERIAL. I know there is a NEXTVAL
> function, I just never call it, so it is fine... If I define a variable I
> expect to have to use it.
>
> Session secure variables are some different than in MSSQL or MySQL - so I
>> would not to use same syntax.
>>
>
> I'm not sure why pg variables should be different from these other tools.
>

because MySQL variables are not declared - and allows assign everywhere -
and MSSQL variables are not persistent. Its total different creatures.


> What is the use case to cover? The few times I wished I had variables
> would have been covered by session-limited variables, for which
> grant/revoke do not make sense.
>
> I really would to use pg_class as base for metadata of variables -
>> conflicts are not possible. I can reuse safe GRANT/REVOKE mechanism ..
>> With different syntax it all lost sense - and I'll to implement it again.
>>
>
> I also hate having my time going down the drain, but this cannot be the
> justification for a feature.
>
> I have a plan to support TRANSACTION and SESSION scope.
>>
>
> That looks ok to me.
>
> Persistent or shared scope needs much more complex rules, and some
>> specialized extensions will be better.
>>
>
> Or maybe they should be avoided altogether?
>
> [GRANT].
>> It is necessary - and I think so it is fundamental feature - any other
>> features can be more or less replaced by extensions, but this one cannot
>> or
>> not simply  - you have to protect content against some users - some
>> cookies, ids have to be protected. It can be used well with RLS.
>> Ada language has packages, package variables. I would not to introduce
>> packages because are redundant to schemas, but I need some mechanism for
>> content protecting.
>>
>
> I do not understand why GRANT make sense. If a variable is set by a
> session/tx and only accessible to this session/tx, then only the client who
> put it can get it back, so it is more of a syntactic commodity?
>

In one session you can use lot of roles - some code can be used for
securing interactive work, some can be for securing some API, sometimes you
can secure a access to some sources. You can switch lot of roles by using
security definer functions.


>
> What appropriate use case would need more?
>
> I would not to introduce packages, because than I will have problem with
>> joining ADA packages with Perl, Python.  Instead I introduce secure granted
>> access. More - I don't need to solve lexical scope - and I can use a wide
>> used mechanism.
>>
>
> 3. accessed/updated with special function "getvar", "setvar":
>>>

 FUNCTION getvar(regclass) RETURNS type
 FUNCTION setvar(regclass, type) RETURNS void

>>>
>>> From an aesthetical point of view, I do not like that much.
>>>
>>> If you use CREATE & DROP, then logically you should use ALTER:
>>>
>>>   CREATE VARIABLE @name TEXT DEFAULT 'calvin';
>>> CREATE VARIABLE @name TEXT = 'calvin';
>>>   ALTER VARIABLE @name SET VALUE TO 'hobbes';
>>> ALTER VARIABLE @name = 'hoobes';
>>>   DROP VARIABLE @name;
>>>
>>> Maybe "SET" could be an option as well, but it is less logical:
>>>
>>>   SET @name = 'susie';
>>>
>>> But then "SET @..." would just be a shortcut for ALTER VARIABLE.
>>>
>>
>> I would to use a SET statement too. But it is used for another target now.
>> Using ALTER in this content looks strange to me. It is used for changing
>> metadata not a value.
>>
>
> ALTER SEQUENCE does allow to change its value? Or maybe use UPDATE, as you
> suggest below...
>
> Next step can be support of SQL statements
>> With SQL support you can do
>>
>> SELECT varname;
>> UPDATE varname SET value TO xxx;
>>
>
> SELECT * FROM compositevarname;
>> UPDATE compositevarname SET field TO xxx;
>>
>
> I'm not at ease with the syntax because varname is both a value and a
> relation somehow... But maybe that make sense? Not sure, I'll think about
> it.
>
> Hmmm... Do you mean:
>>>
>>> CREATE VARIABLE foo INT DEFAULT 1 SCOPE SESSION;
>>> BEGIN;
>>>   SET @foo = 2;
>>> ROLLBACK;
>>>
>>> Then @foo is 2 despite the roolback? Yuk!
>>>
>>
>> This is similar to sequences.
>>
>
> That is not a good reason to do the same. Sequences are special objects
> for which the actual value is expected to be of no importance, only that it
> is different from the previous and the next. I do not think that
> "variables" should behave like that, because their value is important.
>
> If you need transactional content - then you should to use tables.
>>
>
> Why not.
>
> Maybe variables just need be a syntactic convenience around that?
>

There is pretty similar relation between sequences and tables and variables
and tables.


>
> A 

Re: [HACKERS] proposal: session server side variables

[Fabien COELHO]

Hello,


I little bit dislike this style - in my proposal the session variables are
very near to a sequences - and we have not any special symbols for
sequences.


Yep, but we do not need a syntax to reference a sequence either... it is 
automatic and usually hidden behind SERIAL. I know there is a NEXTVAL 
function, I just never call it, so it is fine... If I define a variable I 
expect to have to use it.



Session secure variables are some different than in MSSQL or MySQL - so I
would not to use same syntax.


I'm not sure why pg variables should be different from these other tools.

What is the use case to cover? The few times I wished I had variables 
would have been covered by session-limited variables, for which 
grant/revoke do not make sense.



I really would to use pg_class as base for metadata of variables -
conflicts are not possible. I can reuse safe GRANT/REVOKE mechanism ..
With different syntax it all lost sense - and I'll to implement it again.


I also hate having my time going down the drain, but this cannot be the 
justification for a feature.



I have a plan to support TRANSACTION and SESSION scope.


That looks ok to me.

Persistent or shared scope needs much more complex rules, and some 
specialized extensions will be better.


Or maybe they should be avoided altogether?


[GRANT].
It is necessary - and I think so it is fundamental feature - any other
features can be more or less replaced by extensions, but this one cannot or
not simply  - you have to protect content against some users - some
cookies, ids have to be protected. It can be used well with RLS.
Ada language has packages, package variables. I would not to introduce
packages because are redundant to schemas, but I need some mechanism for
content protecting.


I do not understand why GRANT make sense. If a variable is set by a 
session/tx and only accessible to this session/tx, then only the client 
who put it can get it back, so it is more of a syntactic commodity?


What appropriate use case would need more?

I would not to introduce packages, because than I will have problem with 
joining ADA packages with Perl, Python.  Instead I introduce secure 
granted access. More - I don't need to solve lexical scope - and I can 
use a wide used mechanism.



3. accessed/updated with special function "getvar", "setvar":


FUNCTION getvar(regclass) RETURNS type
FUNCTION setvar(regclass, type) RETURNS void


From an aesthetical point of view, I do not like that much.

If you use CREATE & DROP, then logically you should use ALTER:

  CREATE VARIABLE @name TEXT DEFAULT 'calvin';
CREATE VARIABLE @name TEXT = 'calvin';
  ALTER VARIABLE @name SET VALUE TO 'hobbes';
ALTER VARIABLE @name = 'hoobes';
  DROP VARIABLE @name;

Maybe "SET" could be an option as well, but it is less logical:

  SET @name = 'susie';

But then "SET @..." would just be a shortcut for ALTER VARIABLE.


I would to use a SET statement too. But it is used for another target now.
Using ALTER in this content looks strange to me. It is used for changing
metadata not a value.


ALTER SEQUENCE does allow to change its value? Or maybe use UPDATE, as you 
suggest below...



Next step can be support of SQL statements
With SQL support you can do

SELECT varname;
UPDATE varname SET value TO xxx;



SELECT * FROM compositevarname;
UPDATE compositevarname SET field TO xxx;


I'm not at ease with the syntax because varname is both a value and a 
relation somehow... But maybe that make sense? Not sure, I'll think about 
it.



Hmmm... Do you mean:

CREATE VARIABLE foo INT DEFAULT 1 SCOPE SESSION;
BEGIN;
  SET @foo = 2;
ROLLBACK;

Then @foo is 2 despite the roolback? Yuk!


This is similar to sequences.


That is not a good reason to do the same. Sequences are special objects 
for which the actual value is expected to be of no importance, only that 
it is different from the previous and the next. I do not think that 
"variables" should behave like that, because their value is important.



If you need transactional content - then you should to use tables.


Why not.

Maybe variables just need be a syntactic convenience around that?

A variable is a table with one row holding one value... In which case 
GRANT/REVOKE makes sense, because a table may be shared and persistent, 
thus is not limited to a session or a transaction.


That allows to set constraints.

CREATE VARIABLE foo INT NOT NULL DEFAULT 1 SCOPE SESSION/SESSION SCOPE;
-> CREATE TEMPORARY TABLE foo(val INT NOT NULL DEFAULT 1) ONE ROW;
-> INSERT INTO foo VALUES();

@foo
-> (SELECT val FROM foo LIMIT 1)

@foo.field
-> (SELECT field FROM foo LIMIT 1)

SET @foo = 2;
-> UPDATE @foo SET val = 2;
SET @foo.field = 3;
-> UPDATE foo SET field = 3;

DROP VARIABLE foo;
-> DROP TABLE foo;

--
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] varlena beyond 1GB and matrix

[Robert Haas]

On Thu, Dec 22, 2016 at 8:44 PM, Kohei KaiGai  wrote:
> 2016-12-23 8:23 GMT+09:00 Robert Haas :
>> On Wed, Dec 7, 2016 at 10:44 PM, Kohei KaiGai  wrote:
 Handling objects >1GB at all seems like the harder part of the
 problem.

>>> I could get your point almost. Does the last line above mention about
>>> amount of the data object >1GB? even if the "super-varlena" format
>>> allows 64bit length?
>>
>> Sorry, I can't understand your question about what I wrote.
>>
> I thought you just pointed out it is always harder part to treat large
> amount of data even if data format allows >1GB or more. Right?

I *think* we agreeing.  But I'm still not 100% sure.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Declarative partitioning vs. sql_inheritance

[Tom Lane]

Robert Haas  writes:
> On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane  wrote:
>> Do you have any particular objection to taking the next step of removing
>> enum InhOption in favor of making inhOpt a bool?

> No, not really.  I don't feel like it's an improvement, but you and
> Alvaro obviously do, so have at it.

OK, will do.

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] Declarative partitioning vs. sql_inheritance

[Robert Haas]

On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Great, committed.  I realize just now that I forgot to credit anyone
>> as a reviewer, but hopefully nobody's going to mind that too much
>> considering this is a purely mechanical patch I wrote in 20 minutes.
>
> Do you have any particular objection to taking the next step of removing
> enum InhOption in favor of making inhOpt a bool?  It seems to me that
> stuff like
>
> -   boolrecurse = interpretInhOption(rv->inhOpt);
> +   boolrecurse = (rv->inhOpt == INH_YES);
>
> just begs the question of why it's not simply
>
> boolrecurse = rv->inh;
>
> Certainly a reader who did not know the history would be confused at
> the useless-looking complexity.

No, not really.  I don't feel like it's an improvement, but you and
Alvaro obviously do, so have at it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Declarative partitioning vs. sql_inheritance

[Tom Lane]

Robert Haas  writes:
> Great, committed.  I realize just now that I forgot to credit anyone
> as a reviewer, but hopefully nobody's going to mind that too much
> considering this is a purely mechanical patch I wrote in 20 minutes.

Do you have any particular objection to taking the next step of removing
enum InhOption in favor of making inhOpt a bool?  It seems to me that
stuff like

-   boolrecurse = interpretInhOption(rv->inhOpt);
+   boolrecurse = (rv->inhOpt == INH_YES);

just begs the question of why it's not simply

boolrecurse = rv->inh;

Certainly a reader who did not know the history would be confused at
the useless-looking complexity.

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] Vacuum: allow usage of more than 1GB of work mem

[Anastasia Lubennikova]

22.12.2016 21:18, Claudio Freire:

On Thu, Dec 22, 2016 at 12:22 PM, Claudio Freire  wrote:

On Thu, Dec 22, 2016 at 12:15 PM, Anastasia Lubennikova
 wrote:

The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:   not tested
Spec compliant:   not tested
Documentation:not tested

Hi,
I haven't read through the thread yet, just tried to apply the patch and run 
tests.
And it seems that the last attached version is outdated now. It doesn't apply 
to the master
and after I've tried to fix merge conflict, it segfaults at initdb.


I'll rebase when I get some time to do it and post an updated version

Attached rebased patches. I called them both v3 to be consistent.

I'm not sure how you ran it, but this works fine for me:

./configure --enable-debug --enable-cassert
make clean
make check

... after a while ...

===
  All 168 tests passed.
===

I reckon the above is equivalent to installcheck, but doesn't require
sudo or actually installing the server, so installcheck should work
assuming the install went ok.


I found the reason. I configure postgres with CFLAGS="-O0" and it causes 
Segfault on initdb.
It works fine and passes tests with default configure flags, but I'm 
pretty sure that we should fix segfault before testing the feature.

If you need it, I'll send a core dump.

--
Anastasia Lubennikova
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] proposal: session server side variables

[Pavel Stehule]

2016-12-23 16:27 GMT+01:00 Fabien COELHO :

>
> Hello Pavel,
>
> The session variables should be:
>>
>
> I have often wished I had such a feature, psql client side :-variables are
> just awful raw text things.
>
> A few comments, mostly about the design:
>
> 1. persistent objects with temporal unshared typed content. The life of
>> content should be limited by session or by transaction. The content is
>> initialized to default (when it is defined) or to NULL when variable is
>> first accessed in variable' time scope (session, transaction).
>>
>> CREATE VARIABLE [schema.]variable type [DEFAULT default_value]
>> [TRANSACTION|SESION SCOPE]
>>
>
> I'm not sure of the order, and from a parser perspective it is nice to
> announce the type before the value.
>

I little bit dislike this style - in my proposal the session variables are
very near to a sequences - and we have not any special symbols for
sequences.
Session secure variables are some different than in MSSQL or MySQL - so I
would not to use same syntax.

I really would to use pg_class as base for metadata of variables -
conflicts are not possible. I can reuse safe GRANT/REVOKE mechanism ..

With different syntax it all lost sense - and I'll to implement it again.


>
> Maybe a SQL-server like @-prefix would be nice, something like:
>
>CREATE VARIABLE @foo TEXT DEFAULT 'hello' SCOPE SESSION;
>
> DROP VARIABLE [schema.]variable
>>
>
> In the long term, What would be the possible scopes?
>
> TRANSACTION, SESSION, PERSISTANT ?
>
> Would some scopes orthogonal (eg SHARED between sessions for a USER in a
> DATABASE, SHARED at the cluster level?).
>

I have a plan to support TRANSACTION and SESSION scope. Persistent or
shared scope needs much more complex rules, and some specialized extensions
will be better.



> How to deal with namespace issues?
>
> 2. accessed with respecting access rights:
>>
>> GRANT SELECT|UPDATE|ALL ON VARIABLE variable TO role
>> REVOKE SELECT|UPDATE|ALL ON VARIABLE variable FROM role
>>
>
> At least for transaction and session scopes it does not make sense that
> they would be accessible outside the session/transaction, so grant/revoke
> do not seem necessary?


It is necessary - and I think so it is fundamental feature - any other
features can be more or less replaced by extensions, but this one cannot or
not simply  - you have to protect content against some users - some
cookies, ids have to be protected. It can be used well with RLS.

 Ada language has packages, package variables. I would not to introduce
packages because are redundant to schemas, but I need some mechanism for
content protecting. I would not to introduce packages, because than I will
have problem with joining ADA packages with Perl, Python.  Instead I
introduce secure granted access. More - I don't need to solve lexical scope
- and I can use a wide used mechanism.


>
>
> 3. accessed/updated with special function "getvar", "setvar":
>>
>> FUNCTION getvar(regclass) RETURNS type
>> FUNCTION setvar(regclass, type) RETURNS void
>>
>
> From an aesthetical point of view, I do not like that much.
>
> If you use CREATE & DROP, then logically you should use ALTER:
>
>   CREATE VARIABLE @name TEXT DEFAULT 'calvin';
> CREATE VARIABLE @name TEXT = 'calvin';
>   ALTER VARIABLE @name SET VALUE TO 'hobbes';
> ALTER VARIABLE @name = 'hoobes';
>   DROP VARIABLE @name;
>
> Maybe "SET" could be an option as well, but it is less logical:
>
>   SET @name = 'susie';
>
> But then "SET @..." would just be a shortcut for ALTER VARIABLE.
>

I would to use a SET statement too. But it is used for another target now.
Using ALTER in this content looks strange to me. It is used for changing
metadata not a value.

Next step can be support of SQL statements

With SQL support you can do

SELECT varname;
SELECT * FROM compositevarname;
UPDATE varname SET value TO xxx;
UPDATE compositevarname SET field TO xxx;


> Also a nicer way to reference them would be great, like SQL server.
>
>   SELECT * FROM SomeTable WHERE name = @name;
>
> A function may be called behind the scene, I'm just arguing about the
> syntax here...
>
> Important question, what nice syntax to assign the result of a query to a
> variable? Maybe it could be:
>
>   SET @name = query-returning-one-row; -- hmmm
>   SET @name FROM query-returning-one-row; -- maybe better
>
> Or:
>
>   ALTER VARIABLE @name WITH one-row-query;
>
> Special variables could allow to get the number of rows modified by the
> last option, like in PL/pgSQL but at the SQL level?
>
> 4. non transactional  - the metadata are transactional, but the content is
>> not.
>>
>
> Hmmm... Do you mean:
>
> CREATE VARIABLE foo INT DEFAULT 1 SCOPE SESSION;
> BEGIN;
>   SET @foo = 2;
> ROLLBACK;
>
> Then @foo is 2 despite the roolback? Yuk!
>
>
This is similar to sequences.

If you need transactional content - then you should to use tables.



> I think that if the implementation is based on some system table for
> storage, then you could get 

Re: [HACKERS] Server Crash while running sqlsmith [TRAP: FailedAssertion("!(keylen < 64)", File: "hashfunc.c", Line: 139) ]

[Tom Lane]

tushar  writes:
> While running sqlsmith against PG v10 , found a crash  . Not sure 
> whether it is reported  earlier or not . Please refer the standalone 
> testcase for the same -

Hmm, so that can be boiled down to

regression=# select has_server_privilege(repeat('x',100),'y');
server closed the connection unexpectedly

which indicates that something is being slothful about identifier
length truncation.  It looks like that's not the only member of
the has_foo_privilege family with that disease, either:

regression=# select has_column_privilege('tenk1',repeat('x',100),'y');
server closed the connection unexpectedly

The majority of those functions truncate putative identifiers before
trying to look them up, and I think these should as well.

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] proposal: session server side variables

[Fabien COELHO]

Hello Pavel,


The session variables should be:


I have often wished I had such a feature, psql client side :-variables are 
just awful raw text things.


A few comments, mostly about the design:


1. persistent objects with temporal unshared typed content. The life of
content should be limited by session or by transaction. The content is
initialized to default (when it is defined) or to NULL when variable is
first accessed in variable' time scope (session, transaction).

CREATE VARIABLE [schema.]variable type [DEFAULT default_value]
[TRANSACTION|SESION SCOPE]


I'm not sure of the order, and from a parser perspective it is nice to 
announce the type before the value.


Maybe a SQL-server like @-prefix would be nice, something like:

   CREATE VARIABLE @foo TEXT DEFAULT 'hello' SCOPE SESSION;


DROP VARIABLE [schema.]variable


In the long term, What would be the possible scopes?

TRANSACTION, SESSION, PERSISTANT ?

Would some scopes orthogonal (eg SHARED between sessions for a USER in a 
DATABASE, SHARED at the cluster level?).


How to deal with namespace issues?


2. accessed with respecting access rights:

GRANT SELECT|UPDATE|ALL ON VARIABLE variable TO role
REVOKE SELECT|UPDATE|ALL ON VARIABLE variable FROM role


At least for transaction and session scopes it does not make sense that 
they would be accessible outside the session/transaction, so grant/revoke 
do not seem necessary?



3. accessed/updated with special function "getvar", "setvar":

FUNCTION getvar(regclass) RETURNS type
FUNCTION setvar(regclass, type) RETURNS void



From an aesthetical point of view, I do not like that much.


If you use CREATE & DROP, then logically you should use ALTER:

  CREATE VARIABLE @name TEXT DEFAULT 'calvin';
CREATE VARIABLE @name TEXT = 'calvin';
  ALTER VARIABLE @name SET VALUE TO 'hobbes';
ALTER VARIABLE @name = 'hoobes';
  DROP VARIABLE @name;

Maybe "SET" could be an option as well, but it is less logical:

  SET @name = 'susie';

But then "SET @..." would just be a shortcut for ALTER VARIABLE.

Also a nicer way to reference them would be great, like SQL server.

  SELECT * FROM SomeTable WHERE name = @name;

A function may be called behind the scene, I'm just arguing about the 
syntax here...


Important question, what nice syntax to assign the result of a query to a 
variable? Maybe it could be:


  SET @name = query-returning-one-row; -- hmmm
  SET @name FROM query-returning-one-row; -- maybe better

Or:

  ALTER VARIABLE @name WITH one-row-query;

Special variables could allow to get the number of rows modified by the 
last option, like in PL/pgSQL but at the SQL level?



4. non transactional  - the metadata are transactional, but the content is
not.


Hmmm... Do you mean:

CREATE VARIABLE foo INT DEFAULT 1 SCOPE SESSION;
BEGIN;
  SET @foo = 2;
ROLLBACK;

Then @foo is 2 despite the roolback? Yuk!

I think that if the implementation is based on some system table for 
storage, then you could get the transaction properties for free, and it 
seems more logical to do so:


CREATE TEMPORARY TABLE pg_session_variables(name TEXT PRIMARY KEY, value TEXT, 
oidtype, ...);

CREATE VARIABLE @foo INTEGER; -- INSERT INTO TABLE ...

SELECT * FROM x WHERE name = @foo;
-- SELECT * FROM x WHERE name = (SELECT value::INT FROM pg_session_variables 
WHERE name='foo')

So maybe some simple syntactic rewriting would be enough? Or some SPI 
function?


--
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] Remove lower limit on checkpoint_timeout?

[Tom Lane]

Andres Freund  writes:
> While it's not a particularly good idea to set it to 1s on a production
> system, I don't see why we need to prevent that. It's not like 30s is
> likely to be a good idea either.

> Hence I'd like to set the lower limit to 1s.

OK, but the documentation for it needs some work if you're going to
do that.  It only warns against making the timeout too large, not
too small.

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] Remove lower limit on checkpoint_timeout?

[Fabien COELHO]

Hello Andres,


for testing - like yesterday's 6ef2eba3f - it's annoying that
checkpoint_timeout has 30s minimum. I've now locally patched that to be
1s a significant number of times.

While it's not a particularly good idea to set it to 1s on a production
system, I don't see why we need to prevent that. It's not like 30s is
likely to be a good idea either.

Hence I'd like to set the lower limit to 1s.


My 0.02€:

I have also resorted to reduce this limit in order to reduce latency and 
avoid postgres freezing on checkpoints, before checkpoint buffers where 
sorted. I think I may have tried down to 0.1s which involved some changes, 
and it worked quite well for the purpose.


So basically I would also be in favor to reduce the lower limit down to 1 
second, maybe with a warning output when set to low values, say under 1 
minute, as you propose.


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

[HACKERS] propose to pushdown qual into EXCEPT

[Armor]

Recently, we find PG fails to generate an effective plan for following SQL:
select * from (select * from table1 execpt select * from table2) as foo 
where foo.a > 0;
Because PG does not pushdown qual to the none of the subquery. And I check 
the source code, find some comments in src/backend/optimizer/path/allpaths.c, 
which says "If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot 
push quals into it, because that could change the results".
However, for this case, I think we can pushdown qual to the  left most 
subquery of EXCEPT, just like other database does. And we can get an more 
effective plan such as:
postgres=# explain select * from (select * from table1 except select * from 
table2) as foo where foo.a > 0;
   QUERY PLAN   


 Subquery Scan on foo  (cost=0.00..118.27 rows=222 width=8)
   ->  HashSetOp Except  (cost=0.00..116.05 rows=222 width=12)
 ->  Append  (cost=0.00..100.98 rows=3013 width=12)
   ->  Subquery Scan on "*SELECT* 1"  (cost=0.00..45.78 rows=753 
width=12)
 ->  Seq Scan on table1  (cost=0.00..38.25 rows=753 width=8)
   Filter: (a > 0)
   ->  Subquery Scan on "*SELECT* 2"  (cost=0.00..55.20 rows=2260 
width=12)
 ->  Seq Scan on table2  (cost=0.00..32.60 rows=2260 
width=8)
(8 rows)



   And the attached patch is a draft, it works for this case.



--
Jerry Yu
https://github.com/scarbrofair

push_qual_to_except.diff
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] Clarifying "server starting" messaging in pg_ctl start without --wait

[Peter Eisentraut]

On 12/20/16 3:43 PM, Peter Eisentraut wrote:
> On 12/20/16 3:31 PM, Ryan Murphy wrote:
>> I'm concerned some new users may not understand this behavior of pg_ctl,
>> so I wanted to suggest that we add some additional messaging after
>> "server starting" - something like:
>>
>> $ pg_ctl -D datadir -l logfile start
>> server starting
>> (to wait for confirmation that server actually started, try pg_ctl again
>> with --wait)
> 
> Maybe the fix is to make --wait the default?

Here is a patch for that.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 400c5445381596c1b5b5083a56d130498ade421b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 23 Dec 2016 12:00:00 -0500
Subject: [PATCH] pg_ctl: Change default to wait for all actions

---
 doc/src/sgml/ref/pg_ctl-ref.sgml | 36 +++-
 src/bin/pg_ctl/pg_ctl.c  | 15 ++-
 2 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
index 5fb6898699..7587355164 100644
--- a/doc/src/sgml/ref/pg_ctl-ref.sgml
+++ b/doc/src/sgml/ref/pg_ctl-ref.sgml
@@ -31,7 +31,7 @@
   
pg_ctl
start
-   -w
+   -W
-t seconds
-s
-D datadir
@@ -60,7 +60,7 @@
   
pg_ctl
restart
-   -w
+   -W
-t seconds
-s
-D datadir
@@ -91,7 +91,7 @@
   
pg_ctl
promote
-   -w
+   -W
-t seconds
-s
-D datadir
@@ -117,7 +117,7 @@
d[emand]
  

-   -w
+   -W
-t seconds
-s
-o options
@@ -391,17 +391,7 @@ Options
 Wait for an operation to complete.  This is supported for the
 modes start, stop,
 restart, promote,
-and register.
-   
-
-   
-Waiting is the default option for shutdowns, but not startups,
-restarts, or promotions.  This is mainly for historical reasons; the
-waiting option is almost always preferable.  If waiting is not
-selected, the requested action is triggered, but there is no feedback
-about its success.  In that case, the server log file or an external
-monitoring system would have to be used to check the progress and
-success of the operation.
+and register, and is the default for those modes.

 

@@ -424,6 +414,18 @@ Options
 Do not wait for an operation to complete.  This is the opposite of the
 option -w.

+
+   
+If waiting is disabled, the requested action is triggered, but there
+is no feedback about its success.  In that case, the server log file
+or an external monitoring system would have to be used to check the
+progress and success of the operation.
+   
+
+   
+In prior releases of PostgreSQL, this was the default except for
+the stop mode.
+   
   
  
 
@@ -593,7 +595,7 @@ Starting the Server
 To start the server, waiting until the server is
 accepting connections:
 
-$ pg_ctl -w start
+$ pg_ctl start
 

 
@@ -637,7 +639,7 @@ Restarting the Server
 To restart the server,
 waiting for it to shut down and restart:
 
-$ pg_ctl -w restart
+$ pg_ctl restart
 

 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 4b476022c0..91d4e0d080 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -71,8 +71,7 @@ typedef enum
 
 #define DEFAULT_WAIT	60
 
-static bool do_wait = false;
-static bool wait_set = false;
+static bool do_wait = true;
 static int	wait_seconds = DEFAULT_WAIT;
 static bool wait_seconds_arg = false;
 static bool silent_mode = false;
@@ -1959,7 +1958,7 @@ do_help(void)
 	printf(_("  -s, --silent   only print errors, no informational messages\n"));
 	printf(_("  -t, --timeout=SECS seconds to wait when using -w option\n"));
 	printf(_("  -V, --version  output version information, then exit\n"));
-	printf(_("  -w, --wait wait until operation completes\n"));
+	printf(_("  -w, --wait wait until operation completes (default)\n"));
 	printf(_("  -W, --no-wait  do not wait until operation completes\n"));
 	printf(_("  -?, --help show this help, then exit\n"));
 	printf(_("(The default is to wait for shutdown, but not for start or restart.)\n\n"));
@@ -2323,11 +2322,9 @@ main(int argc, char **argv)
 	break;
 case 'w':
 	do_wait = true;
-	wait_set = true;
 	break;
 case 'W':
 	do_wait = false;
-	wait_set = true;
 	break;
 case 'c':
 	allow_core_files = true;
@@ -2423,14 +2420,6 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-	if (!wait_set)
-	{
-		if (ctl_command == STOP_COMMAND)
-			do_wait = true;
-		else
-			do_wait = false;
-	}
-
 	if (ctl_command == RELOAD_COMMAND)
 	{
 		sig = SIGHUP;
-- 
2.11.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to 

Re: [HACKERS] Minor correction in alter_table.sgml

[Stephen Frost]

Amit,

* Amit Langote (amitlangot...@gmail.com) wrote:
> On Fri, Dec 23, 2016 at 12:07 AM, Stephen Frost  wrote:
> > * Tom Lane (t...@sss.pgh.pa.us) wrote:
> >> (Of course, maybe the question we ought to be asking here is why
> >> ATTACH/DETACH PARTITION failed to go with the flow and be a
> >> combinable action.)
> >
> > I did wonder that myself but havne't looked at the code.  I'm guessing
> > there's a reason it's that way.
> 
> I thought the possibility of something like the following happening
> should be avoided:
> 
> alter table p attach partition p1 for values in (1, 2, 3), add b int;
> ERROR:  child table is missing column "b"

Sure, but what about something like:

alter table p attach partition p1 for values in (1, 2, 3), alter column
b set default 1; ?

> Although, the same can be said about ALTER TABLE child INHERIT parent, I 
> guess.

Certainly seems like that's an indication that there are use-cases for
allowing it then.  We do tend to avoid arbitrary restrictions and if
there isn't really anything code-level for ATTACH/DETACH partition to be
this way then we change it to be allowed.

Thanks!

Stephen


signature.asc
Description: Digital signature

[HACKERS] Remove lower limit on checkpoint_timeout?

[Andres Freund]

Hi,

for testing - like yesterday's 6ef2eba3f - it's annoying that
checkpoint_timeout has 30s minimum. I've now locally patched that to be
1s a significant number of times.

While it's not a particularly good idea to set it to 1s on a production
system, I don't see why we need to prevent that. It's not like 30s is
likely to be a good idea either.

Hence I'd like to set the lower limit to 1s.

Comments?

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] Parallel Index Scans

[Anastasia Lubennikova]

22.12.2016 07:19, Amit Kapila:

On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova
 wrote:

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Hi, thank you for the patch.
Results are very promising. Do you see any drawbacks of this feature or 
something that requires more testing?


I think you can focus on the handling of array scan keys for testing.
In general, one of my colleagues has shown interest in testing this
patch and I think he has tested as well but never posted his findings.
I will request him to share his findings and what kind of tests he has
done, if any.



Check please code related to buffer locking and pinning once again.
I got the warning. Here are the steps to reproduce it:
Except "autovacuum = off" config is default.

pgbench -i -s 100 test
pgbench -c 10 -T 120 test

SELECT count(aid) FROM pgbench_accounts
WHERE aid > 1000 AND aid < 90 AND bid > 800 AND bid < 900;
WARNING:  buffer refcount leak: [8297] (rel=base/12289/16459, 
blockNum=2469, flags=0x9380, refcount=1 1)

 count
---
 0
(1 row)

postgres=# select 16459::regclass;
   regclass
---
 pgbench_accounts_pkey



2. Don't you mind to rename 'amestimateparallelscan' to let's say 
'amparallelscan_spacerequired'
or something like this?

Sure, I am open to other names, but IMHO, lets keep "estimate" in the
name to keep it consistent with other parallel stuff. Refer
execParallel.c to see how widely this word is used.


As far as I understand there is nothing to estimate, we know this size
for sure. I guess that you've chosen this name because of 
'heap_parallelscan_estimate'.
But now it looks similar to 'amestimate' which refers to indexscan cost for 
optimizer.
That leads to the next question.


Do you mean 'amcostestimate'?  If you want we can rename it
amparallelscanestimate to be consistent with amcostestimate.


I think that 'amparallelscanestimate' seems less ambiguous than 
amestimateparallelscan.
But it's up to you. There are enough comments to understand the purpose 
of this field.



And why do you call it pageStatus? What does it have to do with page?


During scan this tells us whether next page is available for scan.
Another option could be to name it as scanStatus, but not sure if that
is better.  Do you think if we add a comment like "indicates whether
next page is available for scan" for this variable then it will be
clear?


Yes, I think it describes the flag better.

5. Comment for _bt_parallel_seize() says:
"False indicates that we have reached the end of scan for
  current scankeys and for that we return block number as P_NONE."

  What is the reason to check (blkno == P_NONE) after checking (status == false)
  in _bt_first() (see code below)? If comment is correct
  we'll never reach _bt_parallel_done()

+   blkno = _bt_parallel_seize(scan, );
+   if (status == false)
+   {
+   BTScanPosInvalidate(so->currPos);
+   return false;
+   }
+   else if (blkno == P_NONE)
+   {
+   _bt_parallel_done(scan);
+   BTScanPosInvalidate(so->currPos);
+   return false;
+   }


The first time master backend or worker hits last page (calls this
API), it will return P_NONE and after that any worker tries to fetch
next page, it will return status as false.  I think we can expand a
comment to explain it clearly.  Let me know, if you need more
clarification, I can explain it in detail.


Got it,
I think you can add this explanation to the comment for 
_bt_parallel_seize().



6. To avoid code duplication, I would wrap this into the function

+   /* initialize moreLeft/moreRight appropriately for scan direction */
+   if (ScanDirectionIsForward(dir))
+   {
+   so->currPos.moreLeft = false;
+   so->currPos.moreRight = true;
+   }
+   else
+   {
+   so->currPos.moreLeft = true;
+   so->currPos.moreRight = false;
+   }
+   so->numKilled = 0;  /* just paranoia */
+   so->markItemIndex = -1; /* ditto */


Okay, I think we can write a separate function (probably inline
function) for above.


And after that we can also get rid of _bt_parallel_readpage() which only
bring another level of indirection to the code.


See, this function is responsible for multiple actions like
initializing moreLeft/moreRight positions, reading next page, dropping
the lock/pin.  So replicating all these actions in the caller will
make the code in caller less readable as compared to now.  Consider
this point and let me know your view on same.


Thank you for clarification, now I agree with your implementation.
I've 

Re: [HACKERS] pgstattuple documentation clarification

[Andrew Dunstan]

On 12/21/2016 09:04 AM, Andrew Dunstan wrote:





Yes, I agree. In any case, before we change anything can we agree on a 
description of what we currently do?


Here's a second attempt:

   The table_len will always be greater than the sum of the tuple_len,
   dead_tuple_len and free_space. The difference is accounted for by
   fixed page overhead, the per-page table of pointers to tuples, and
   padding to ensure that tuples are correctly aligned.




In the absence of further comment I will proceed along these lines.

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] Logical decoding on standby

[Andrew Dunstan]

On 12/22/2016 01:21 AM, Craig Ringer wrote:

+   my @fields = ('plugin', 'slot_type', 'datoid', 'database',
'active', 'active_pid', 'xmin', 'catalog_xmin', 'restart_lsn');
+   my $result = $self->safe_psql('postgres', 'SELECT ' . join(', ',
@fields) . " FROM pg_catalog.pg_replication_slots WHERE slot_name =
'$slot_name'");
+   $result = undef if $result eq '';
+   # hash slice, see http://stackoverflow.com/a/16755894/398670 .
Couldn't this portion be made more generic? Other queries could
benefit from that by having a routine that accepts as argument an
array of column names for example.

Yeah, probably. I'm not sure where it should live though - TestLib.pm ?

Not sure if there's an idomatic way to  pass a string (in this case
queyr) in Perl with a placeholder for interpolation of values (in this
case columns). in Python you'd pass it with pre-defined
%(placeholders)s for %.






For direct interpolation of an expression, there is this slightly 
baroque gadget:


   my $str = "here it is @{[ arbitrary expression here ]}";

For indirect interpolation using placeholders, there is

   my $str = sprintf("format string",...);

which works much like C except that the string is returned as a result 
instead of being the first argument.


And as we always say, TIMTOWTDI.


cheers

andrew (japh)


--
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] Declarative partitioning vs. sql_inheritance

[Robert Haas]

On Mon, Dec 19, 2016 at 12:25 AM, Amit Langote
 wrote:
>> I agree.  Patch attached, just removing the GUC and a fairly minimal
>> amount of the supporting infrastructure.
>
> +1 to removing the sql_inheritance GUC.  The patch looks good to me.

Great, committed.  I realize just now that I forgot to credit anyone
as a reviewer, but hopefully nobody's going to mind that too much
considering this is a purely mechanical patch I wrote in 20 minutes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

[HACKERS] Server Crash while running sqlsmith [TRAP: FailedAssertion("!(keylen < 64)", File: "hashfunc.c", Line: 139) ]

[tushar]

Hi,

While running sqlsmith against PG v10 , found a crash  . Not sure 
whether it is reported  earlier or not . Please refer the standalone 
testcase for the same -


[centos@tusharcentos7 bin]$ ./psql postgres -p 9000
psql (10devel)
Type "help" for help.

postgres=# select
postgres-#70 as c0,
postgres-#pg_catalog.has_server_privilege(
postgres(# cast(ref_0.indexdef as text),
postgres(# cast(cast(coalesce((select name from 
pg_catalog.pg_settings limit 1 offset 16)

postgres(# ,
postgres(#null) as text) as text)) as c1,
postgres-#pg_catalog.pg_export_snapshot() as c2,
postgres-#ref_0.indexdef as c3,
postgres-#ref_0.indexname as c4
postgres-#  from
postgres-#   pg_catalog.pg_indexes as ref_0
postgres-#  where (ref_0.tablespace = ref_0.tablespace)
postgres-#or (46 = 22)
postgres-#  limit 103;
TRAP: FailedAssertion("!(keylen < 64)", File: "hashfunc.c", Line: 139)
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: 2016-12-23 
17:46:56.405 IST [16809] LOG:  server process (PID 16817) was terminated 
by signal 6: Aborted
2016-12-23 17:46:56.405 IST [16809] DETAIL:  Failed process was running: 
select

   70 as c0,
   pg_catalog.has_server_privilege(
cast(ref_0.indexdef as text),
cast(cast(coalesce((select name from 
pg_catalog.pg_settings limit 1 offset 16)

,
   null) as text) as text)) as c1,
   pg_catalog.pg_export_snapshot() as c2,
   ref_0.indexdef as c3,
   ref_0.indexname as c4
 from
  pg_catalog.pg_indexes as ref_0
 where (ref_0.tablespace = ref_0.tablespace)
   or (46 = 22)
 limit 103;
2016-12-23 17:46:56.405 IST [16809] LOG:  terminating any other active 
server processes
2016-12-23 17:46:56.407 IST [16814] WARNING:  terminating connection 
because of crash of another server process
2016-12-23 17:46:56.407 IST [16814] DETAIL:  The postmaster has 
commanded this server process to roll back the current transaction and 
exit, because another server process exited abnormally and possibly 
corrupted shared memory.
2016-12-23 17:46:56.407 IST [16814] HINT:  In a moment you should be 
able to reconnect to the database and repeat your command.
2016-12-23 17:46:56.407 IST [16818] FATAL:  the database system is in 
recovery mode

Failed.
!> 2016-12-23 17:46:56.408 IST [16809] LOG:  all server processes 
terminated; reinitializing
2016-12-23 17:46:56.442 IST [16819] LOG:  database system was 
interrupted; last known up at 2016-12-23 17:46:46 IST
2016-12-23 17:46:56.614 IST [16819] LOG:  database system was not 
properly shut down; automatic recovery in progress
2016-12-23 17:46:56.616 IST [16819] LOG:  invalid record length at 
0/155E638: wanted 24, got 0

2016-12-23 17:46:56.616 IST [16819] LOG:  redo is not required
2016-12-23 17:46:56.623 IST [16819] LOG:  MultiXact member wraparound 
protections are now enabled
2016-12-23 17:46:56.626 IST [16809] LOG:  database system is ready to 
accept connections

2016-12-23 17:46:56.626 IST [16823] LOG:  autovacuum launcher started

!> exit
-> \q

Please refer the  stack trace  below -

[centos@tusharcentos7 bin]$ gdb -q -c data/core.16817 
/home/centos/PG10_23Dec/postgresql/edbpsql/bin/postgres
Reading symbols from 
/home/centos/PG10_23Dec/postgresql/edbpsql/bin/postgres...done.

[New LWP 16817]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `postgres: centos postgres [local] 
SELECT   '.

Program terminated with signal 6, Aborted.
#0  0x7fe3b88245f7 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install 
glibc-2.17-106.el7_2.6.x86_64 keyutils-libs-1.5.8-3.el7.x86_64 
krb5-libs-1.13.2-12.el7_2.x86_64 libcom_err-1.42.9-7.el7.x86_64 
libselinux-2.2.2-6.el7.x86_64 openssl-libs-1.0.1e-51.el7_2.5.x86_64 
pcre-8.32-15.el7_2.1.x86_64 xz-libs-5.1.2-12alpha.el7.x86_64 
zlib-1.2.7-15.el7.x86_64

(gdb) bt
#0  0x7fe3b88245f7 in raise () from /lib64/libc.so.6
#1  0x7fe3b8825ce8 in abort () from /lib64/libc.so.6
#2  0x00977a61 in ExceptionalCondition (conditionName=0x9f66eb 
"!(keylen < 64)", errorType=0x9f66db "FailedAssertion", 
fileName=0x9f66d0 "hashfunc.c", lineNumber=139)

at assert.c:54
#3  0x004b3882 in hashname (fcinfo=0x7ffdfabd0590) at hashfunc.c:139
#4  0x009815f7 in DirectFunctionCall1Coll (func=0x4b383c 
, collation=0, arg1=33238784) at fmgr.c:1026
#5  0x00958221 in CatalogCacheComputeHashValue (cache=0x1e96750, 
nkeys=1, cur_skey=0x7ffdfabd09e0) at catcache.c:209
#6  0x0095a62b in SearchCatCache 

Re: [HACKERS] Parallel Index Scans

[Rahila Syed]

>> 5. Comment for _bt_parallel_seize() says:
>> "False indicates that we have reached the end of scan for
>>  current scankeys and for that we return block number as P_NONE."
>>
>>  What is the reason to check (blkno == P_NONE) after checking (status ==
false)
>>  in _bt_first() (see code below)? If comment is correct
>>  we'll never reach _bt_parallel_done()
>>
>> +   blkno = _bt_parallel_seize(scan, );
>> +   if (status == false)
>> +   {
>> +   BTScanPosInvalidate(so->currPos);
>> +   return false;
>> +   }
>> +   else if (blkno == P_NONE)
>> +   {
>> +   _bt_parallel_done(scan);
>> +   BTScanPosInvalidate(so->currPos);
>> +   return false;
>> +   }
>>
>The first time master backend or worker hits last page (calls this
>API), it will return P_NONE and after that any worker tries to fetch
>next page, it will return status as false.  I think we can expand a
>comment to explain it clearly.  Let me know, if you need more
>clarification, I can explain it in detail.

Probably this was confusing because we have not mentioned
that P_NONE can be returned even when status = TRUE and
not just when status is false.

I think, the comment above the function can be modified as follows,

+ /*
+ * True indicates that the block number returned is either valid including
P_NONE
+ * and scan is continued or block number is invalid and scan has just
+ * begun.

Thank you,
Rahila Syed


On Thu, Dec 22, 2016 at 9:49 AM, Amit Kapila 
wrote:

> On Wed, Dec 21, 2016 at 8:46 PM, Anastasia Lubennikova
>  wrote:
> > The following review has been posted through the commitfest application:
> > make installcheck-world:  tested, passed
> > Implements feature:   tested, passed
> > Spec compliant:   tested, passed
> > Documentation:tested, passed
> >
> > Hi, thank you for the patch.
> > Results are very promising. Do you see any drawbacks of this feature or
> something that requires more testing?
> >
>
> I think you can focus on the handling of array scan keys for testing.
> In general, one of my colleagues has shown interest in testing this
> patch and I think he has tested as well but never posted his findings.
> I will request him to share his findings and what kind of tests he has
> done, if any.
>
> > I'm willing to oo a review.
>
> Thanks, that will be helpful.
>
>
> > I saw the discussion about parameters in the thread above. And I agree
> that we'd better concentrate
> > on the patch itself and add them later if necessary.
> >
> > 1. Can't we simply use "if (scan->parallel_scan != NULL)" instead of
> xs_temp_snap flag?
> >
> > +   if (scan->xs_temp_snap)
> > +   UnregisterSnapshot(scan->xs_snapshot);
> >
>
> I agree with what Rober has told in his reply.  We do same way for
> heap, refer heap_endscan().
>
> > I must say that I'm quite new with all this parallel stuff. If you give
> me a link,
> > where to read about snapshots for parallel workers, my review will be
> more helpful.
> >
>
> You can read transam/README.parallel.  Refer "State Sharing" portion
> of README to learn more about it.
>
> > Anyway, it would be great to have more comments about it in the code.
> >
>
> We are sharing snapshot to ensure that reads in both master backend
> and worker backend can use the same snapshot.  There is no harm in
> adding comments, but I think it is better to be consistent with
> similar heapam code.  After reading README.parallel, if you still feel
> that we should add more comments in the code, then we can definitely
> do that.
>
> > 2. Don't you mind to rename 'amestimateparallelscan' to let's say
> 'amparallelscan_spacerequired'
> > or something like this?
>
> Sure, I am open to other names, but IMHO, lets keep "estimate" in the
> name to keep it consistent with other parallel stuff. Refer
> execParallel.c to see how widely this word is used.
>
> > As far as I understand there is nothing to estimate, we know this size
> > for sure. I guess that you've chosen this name because of
> 'heap_parallelscan_estimate'.
> > But now it looks similar to 'amestimate' which refers to indexscan cost
> for optimizer.
> > That leads to the next question.
> >
>
> Do you mean 'amcostestimate'?  If you want we can rename it
> amparallelscanestimate to be consistent with amcostestimate.
>
> > 3. Are there any changes in cost estimation?
> >
>
> Yes.
>
> > I didn't find related changes in the patch.
> > Parallel scan is expected to be faster and optimizer definitely should
> know that.
> >
>
> You can find the relavant changes in
> parallel_index_opt_exec_support_v2.patch, refer cost_index().
>
> > 4. +uint8   ps_pageStatus;  /* state of scan, see below */
> > There is no desciption below. I'd make the comment more helpful:
> > /* state of scan. See possible flags values 

Re: [HACKERS] Parallel Index Scans

[tushar]

On 12/23/2016 05:38 PM, Robert Haas wrote:

So why are you reporting it here rather than on a separate thread?
We found it -while testing parallel index scan and later it turned out 
to be crash in general.

Sure- make sense ,will do that.

--
regards,tushar



--
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] Parallel Index Scans

[Robert Haas]

On Fri, Dec 23, 2016 at 1:35 AM, tushar  wrote:
> In addition to that, we  run the sqlsmith against PG v10+PIS (parallel index
> scan) patches and found a crash  but that is coming on plain  PG v10
> (without applying any patches) as well

So why are you reporting it here rather than on a separate thread?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Minor correction in alter_table.sgml

[Amit Langote]

On Fri, Dec 23, 2016 at 12:07 AM, Stephen Frost  wrote:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> (Of course, maybe the question we ought to be asking here is why
>> ATTACH/DETACH PARTITION failed to go with the flow and be a
>> combinable action.)
>
> I did wonder that myself but havne't looked at the code.  I'm guessing
> there's a reason it's that way.

I thought the possibility of something like the following happening
should be avoided:

alter table p attach partition p1 for values in (1, 2, 3), add b int;
ERROR:  child table is missing column "b"

Although, the same can be said about ALTER TABLE child INHERIT parent, I guess.

Thanks,
Amit


-- 
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] Logical decoding on standby

[Craig Ringer]

On 22 December 2016 at 14:21, Craig Ringer  wrote:

changes-in-0001-v2.diff shows the changes to PostgresNode.pm per
Michael's comments, and applies on top of 0001.

I also attach a patch to add a new CREATE_REPLICATION_SLOT option per
Petr's suggestion, so you can request a slot be created
WITHOUT_SNAPSHOT. This replaces the patch series's behaviour of
silently suppressing snapshot export when a slot was created on a
replica. It'll conflict (easily resolved) if applied on top of the
current series.

I have more to do before re-posting the full series, so waiting on
author at this point. The PostgresNode changes likely break later
tests, I'm just posting them so there's some progress here and so I
don't forget over the next few days' distraction.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 28e9f0b..64a4633 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -93,7 +93,6 @@ use RecursiveCopy;
 use Socket;
 use Test::More;
 use TestLib ();
-use pg_lsn qw(parse_lsn);
 use Scalar::Util qw(blessed);
 
 our @EXPORT = qw(
@@ -1325,38 +1324,62 @@ sub run_log
TestLib::run_log(@_);
 }
 
-=pod $node->lsn
+=pod $node->lsn(mode)
 
-Return pg_current_xlog_insert_location() or, on a replica,
-pg_last_xlog_replay_location().
+Look up xlog positions on the server:
+
+* insert position (master only, error on replica)
+* write position (master only, error on replica)
+* flush position
+* receive position (always undef on master)
+* replay position
+
+mode must be specified.
 
 =cut
 
 sub lsn
 {
-   my $self = shift;
-   return $self->safe_psql('postgres', 'select case when 
pg_is_in_recovery() then pg_last_xlog_replay_location() else 
pg_current_xlog_insert_location() end as lsn;');
+   my ($self, $mode) = @_;
+   my %modes = ('insert' => 'pg_current_xlog_insert_location()',
+'flush' => 'pg_current_xlog_flush_location()',
+'write' => 'pg_current_xlog_location()',
+'receive' => 'pg_last_xlog_receive_location()',
+'replay' => 'pg_last_xlog_replay_location()');
+
+   $mode = '' if !defined($mode);
+   die "unknown mode for 'lsn': '$mode', valid modes are " . join(', ', 
keys %modes)
+   if !defined($modes{$mode});
+
+   my $result = $self->safe_psql('postgres', "SELECT $modes{$mode}");
+   chomp($result);
+   if ($result eq '')
+   {
+   return undef;
+   }
+   else
+   {
+   return $result;
+   }
 }
 
 =pod $node->wait_for_catchup(standby_name, mode, target_lsn)
 
 Wait for the node with application_name standby_name (usually from node->name)
-until its replication equals or passes the upstream's xlog insert point at the
-time this function is called. By default the replay_location is waited for,
-but 'mode' may be specified to wait for any of sent|write|flush|replay.
+until its replication position in pg_stat_replication equals or passes the
+upstream's xlog insert point at the time this function is called. By default
+the replay_location is waited for, but 'mode' may be specified to wait for any
+of sent|write|flush|replay.
 
 If there is no active replication connection from this peer, waits until
 poll_query_until timeout.
 
 Requires that the 'postgres' db exists and is accessible.
 
-If pos is passed, use that xlog position instead of the server's current
-xlog insert position.
+target_lsn may be any arbitrary lsn, but is typically 
$master_node->lsn('insert').
 
 This is not a test. It die()s on failure.
 
-Returns the LSN caught up to.
-
 =cut
 
 sub wait_for_catchup
@@ -1364,24 +1387,25 @@ sub wait_for_catchup
my ($self, $standby_name, $mode, $target_lsn) = @_;
$mode = defined($mode) ? $mode : 'replay';
my %valid_modes = ( 'sent' => 1, 'write' => 1, 'flush' => 1, 'replay' 
=> 1 );
-   die "valid modes are " . join(', ', keys(%valid_modes)) unless 
exists($valid_modes{$mode});
-   if ( blessed( $standby_name ) && $standby_name->isa("PostgresNode") ) {
+   die "unknown mode $mode for 'wait_for_catchup', valid modes are " . 
join(', ', keys(%valid_modes)) unless exists($valid_modes{$mode});
+   # Allow passing of a PostgresNode instance as shorthand
+   if ( blessed( $standby_name ) && $standby_name->isa("PostgresNode") )
+   {
$standby_name = $standby_name->name;
}
-   if (!defined($target_lsn)) {
-   $target_lsn = $self->lsn;
-   }
-   $self->poll_query_until('postgres', qq[SELECT '$target_lsn' <= 
${mode}_location FROM pg_catalog.pg_stat_replication WHERE application_name = 
'$standby_name';])
-   or die "timed out waiting for catchup";
-   return $target_lsn;
+   die 

Re: [HACKERS] [PATCH] Transaction traceability - txid_status(bigint)

[Craig Ringer]

On 23 December 2016 at 01:26, Robert Haas  wrote:

> This patch contains unresolved merge conflicts.

Ah, it conflicts with fe0a0b59, the PostmasterRandom changes. I
thought I'd rebased more recently than that.

> Maybe SetPendingTransactionIdLimit could happen in TruncateCLOG rather than
> the caller.

I've spent a while looking at how committs and multixact handle the
race hazard of the gap between clog truncation and shmem state change
- and the related race on standby, where the hazard is the gap between
replay of CLOG_TRUNCATE records and replay of the next checkpoint
containing updated limits.

pg_xact_commit_timestamp() and the underlying
TransactionIdGetCommitTsData are supposed to accept arbitrary xids,
like txid_status(), so they should handle this.

However, as far as I can tell, committs has the same issues presented
by txid_status(). We only update ShmemVariableCache->oldestCommitTsXid
_after_ we truncate the committs SLRU, no lock is held over the
duration, and no other lock-protected var is set before truncation. We
don't record the new limit values in COMMIT_TS_TRUNCATE records so
they only becomes known to the standby at replay of the next
checkpoint.

multixact instead runs a single critical section to write xlog, set
shmem state, and _then_ truncate the SLRUs. It's more complex than
clog or commit_ts since it has two inter-related SLRUs, though.

So: I think we should be setting ShmemVariableCache->oldestXid (under
XidGenLock) from TruncateCLOG, after we write xlog but _before_
truncation. Then the rest of SetTransactionIdLimit(...) proceeds as
before. I don't _think_ we need the critical section since we only
have the one SLRU to worry about. We should also add oldestXid to
CLOG_TRUNCATE xlog records so it is up to date on standbys.

It's a little annoying to take XidGenLock twice - once for oldestXid,
once for the other xid limits - but we can just delay advancing
oldestXid entirely until we've got a clog page to truncate away, in
which case there's a big enough cost that it doesn't matter. We have
to wait to advance the other limits until after we truncate clog to
prevent new (wrapped) xids trying to set values in clog for the
before-wrap xids we're about to truncate away.

On the upside, the need for pendingOldestXid, which always felt hacky,
goes away.

While we're at it, lets do the same thing for commit_ts. Advance its
limit before truncation, not after, and record that limit in its xlog
records for redo. There's no need for any special dancing around
there.



>  Instead of using an LWLock, how about a spin lock?

I wrote an explanation of how that wouldn't work, but then I found the
problem with standby, so it no longer matters.

I'll have to follow up with a patch soon, as it's Toddler o'Clock.

(It's interesting that most of what I'm doing at the moment is
wrestling with resource retention limits and how they're often quite
implicit. All the catalog_xmin stuff for logical decoding on standby
has parallels to this, for example, though not enough to create useful
overlap of functionality.)

-- 
 Craig Ringer   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] BUG: pg_stat_statements query normalization issues with combined queries

[Fabien COELHO]

Yes. I'll try to put together a patch and submit it to the next CF.


Here it is. I'll add this to the next CF.



This patch fixes the handling of compound/combined queries by 
pg_stat_statements (i.e. several queries sent together, eg with psql: 
"SELECT 1 \; SELECT 2;").


This bug was found in passing while investigating a strange performance 
issue I had with compound statements.



Collect query location information in parser:

 * create a ParseNode for statements with location information fields
   (qlocation & qlength).

 * add these fields to all parsed statements (*Stmt), Query and PlannedStmt.

 * statements location is filled in:

   - the lexer keep track of the last ';' encountered.

   - the information is used by the parser to compute the query length,
 which depends on whether the query ended on ';' or on the input end.

   - the query location is propagated to Query and PlannedStmt when built.


Fix pg_stat_statement:

 * the information is used by pg_stat_statement so as to extract the relevant 
part
   of the query string, including some trimming.

 * pg_stat_statement validation is greatly extended so as to test options
   and exercise various cases, including compound statements.


note 1: two non-statements tags (use for alter table commands) have been 
moved so that all statements tags are contiguous and easy to check.


note 2: the impact on the lexer & parser is quite minimal with this 
approach, about 30 LOC, most of which in one function. The largest changes 
are in the node header to add location fields.


note 3: the query length excludes the final separator, so that 
;-terminated and end of input terminated queries show the same.


note 4: the added test suggests that when tracking "all", queries in SQL 
user functions are not tracked, this might be a bug.


--
Fabien.diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 3573c19..3826d38 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -1,21 +1,346 @@
 CREATE EXTENSION pg_stat_statements;
-CREATE TABLE test (a int, b char(20));
--- test the basic functionality of pg_stat_statements
+--
+--
+-- simple and compound statements
+--
+SET pg_stat_statements.track_utility = FALSE;
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --
  
 (1 row)
 
+SELECT 1 AS "int";
+ int 
+-
+   1
+(1 row)
+
+SELECT 'hello'
+  -- multiline
+  AS "text";
+ text  
+---
+ hello
+(1 row)
+
+SELECT 'world' AS "text";
+ text  
+---
+ world
+(1 row)
+
+-- transaction
+BEGIN;
+SELECT 1 AS "int";
+ int 
+-
+   1
+(1 row)
+
+SELECT 'hello' AS "text";
+ text  
+---
+ hello
+(1 row)
+
+COMMIT;
+-- compound transaction
+BEGIN \;
+SELECT 2.0 AS "float" \;
+SELECT 'world' AS "text" \;
+COMMIT;
+-- compound with empty statements and spurious leading spacing
+\;\;   SELECT 3 + 3 \;\;\;   SELECT ' ' || ' !' \;\;   SELECT 1 + 4 \;;
+ ?column? 
+--
+5
+(1 row)
+
+-- non ;-terminated statements
+SELECT 1 + 1 + 1 AS "add" \gset
+SELECT :add + 1 + 1 AS "add" \;
+SELECT :add + 1 + 1 AS "add" \gset
+-- set operator
+SELECT 1 AS i UNION SELECT 2 ORDER BY i;
+ i 
+---
+ 1
+ 2
+(2 rows)
+
+-- cte
+WITH t(f) AS (
+  VALUES (1.0), (2.0)
+)
+  SELECT f FROM t ORDER BY f;
+  f  
+-
+ 1.0
+ 2.0
+(2 rows)
+
+SELECT query, calls, rows FROM pg_stat_statements ORDER BY rows, query;
+  query  | calls | rows 
+-+---+--
+ SELECT ? || ?   | 1 |1
+ SELECT ? AS "float" | 1 |1
+ | 1 |1
+ SELECT ? + ?| 2 |2
+ SELECT ? AS "int"   | 2 |2
+ SELECT ? AS i UNION SELECT ? ORDER BY i | 1 |2
+ WITH t(f) AS ( +| 1 |2
+   VALUES (?), (?)  +|   | 
+ )  +|   | 
+   SELECT f FROM t ORDER BY f|   | 
+ SELECT ? + ? + ? AS "add"   | 3 |3
+ SELECT ?   +| 4 |4
++|   | 
+   AS "text" |   | 
+(9 rows)
+
+--
+--
+-- CRUD: INSERT SELECT UPDATE DELETE on test table
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset 
+--
+ 
+(1 row)
+
+-- utility "create table" must not show
+CREATE TABLE test (a int, b char(20));
 INSERT INTO test VALUES(generate_series(1, 10), 'aaa');
-UPDATE test SET b = 'bbb' WHERE a > 5;
-SELECT query, calls, rows from pg_stat_statements ORDER BY rows;
-   query| calls | rows 
-+---+--
- SELECT 

Re: [HACKERS] [COMMITTERS] pgsql: Simplify LWLock tranche machinery by removing array_base/array_s

[Pavel Stehule]

2016-12-21 14:00 GMT+01:00 Pavel Stehule :

>
>
> 2016-12-21 13:54 GMT+01:00 Robert Haas :
>
>> On Wed, Dec 21, 2016 at 2:14 AM, Pavel Stehule 
>> wrote:
>> > Is there some help for extensions developers, how to fix extensions
>> after
>> > this change?
>> >
>> > Orafce hits this change.
>>
>> array_base and array_stride are gone; don't pass them.  The second
>> argument to LWLockRegisterTranche() is now the tranche name.  Pass
>> that instead of a structure.
>>
>
> Thank you
>

It is working - for record the necessary changes in Orafce
https://github.com/orafce/orafce/commit/cac48984458c24d92243a7d4df549409dec0b826

Regards

Pavel


> Pavel
>
>>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>