Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

[Mark Dilger]

> On Apr 8, 2017, at 7:41 PM, Mark Dilger  wrote:
> 
> 
>> On Apr 8, 2017, at 7:35 PM, Tom Lane  wrote:
>> 
>> Mark Dilger  writes:
>>> This is very near where the original crash reported in this thread was 
>>> crashing, probably only
>>> different due to the extra lines of Assert that were added.  Am I missing 
>>> some portion of the
>>> fix that you are testing?  I have only applied the patch that Tom included 
>>> in the previous email.
>> 
>> No, that was the whole patch --- but did you do a full "make clean" and
>> rebuild?  The patch changed NodeTag numbers which would affect the whole
>> tree.
> 
> 

> I'm going to pull completely fresh sources and reapply and retest, though you 
> are welcome
> to review my patch while I do that.

That fixed it.  Thanks.



-- 
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] [sqlsmith] Planner crash on foreign table join

[Mark Dilger]

> On Apr 8, 2017, at 7:35 PM, Tom Lane  wrote:
> 
> Mark Dilger  writes:
>> This is very near where the original crash reported in this thread was 
>> crashing, probably only
>> different due to the extra lines of Assert that were added.  Am I missing 
>> some portion of the
>> fix that you are testing?  I have only applied the patch that Tom included 
>> in the previous email.
> 
> No, that was the whole patch --- but did you do a full "make clean" and
> rebuild?  The patch changed NodeTag numbers which would affect the whole
> tree.

I had trouble applying your patch using

 mark$ patch -p 1 < patch
patching file src/backend/nodes/bitmapset.c
Hunk #1 FAILED at 115.
Hunk #2 FAILED at 146.
Hunk #3 FAILED at 190.
Hunk #4 FAILED at 205.
Hunk #5 FAILED at 229.
Hunk #6 FAILED at 265.
Hunk #7 FAILED at 298.
Hunk #8 FAILED at 324.
Hunk #9 FAILED at 364.
Hunk #10 FAILED at 444.
Hunk #11 FAILED at 463.
Hunk #12 FAILED at 488.
Hunk #13 FAILED at 517.
Hunk #14 FAILED at 554.
Hunk #15 FAILED at 598.
Hunk #16 FAILED at 635.
Hunk #17 FAILED at 665.
Hunk #18 FAILED at 694.
Hunk #19 FAILED at 732.
Hunk #20 FAILED at 770.
Hunk #21 FAILED at 789.
Hunk #22 FAILED at 825.
Hunk #23 FAILED at 853.
Hunk #24 FAILED at 878.
Hunk #25 FAILED at 927.
Hunk #26 FAILED at 981.
Hunk #27 FAILED at 1027.
27 out of 27 hunks FAILED -- saving rejects to file 
src/backend/nodes/bitmapset.c.rej
patching file src/include/nodes/bitmapset.h
Hunk #1 FAILED at 20.
Hunk #2 FAILED at 38.
2 out of 2 hunks FAILED -- saving rejects to file 
src/include/nodes/bitmapset.h.rej
patching file src/include/nodes/nodes.h
Hunk #1 FAILED at 291.
1 out of 1 hunk FAILED -- saving rejects to file src/include/nodes/nodes.h.rej


So I did the patching manually against my copy of the current master,
aba696d1af9a267eee85d69845c3cdeccf788525, and then ran:

make distclean; ./configure --enable-cassert --enable-tap-tests --enable-depend 
&& make -j4 && make check-world

I am attaching my patch, which I admit on closer inspection differs from yours, 
but not
in any way I can tell is relevant:



patch.mark.1
Description: Binary data


I'm going to pull completely fresh sources and reapply and retest, though you 
are welcome
to review my patch while I do that.

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

Re: Fwd: Re: [HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Andrew Dunstan]

On 04/08/2017 02:49 PM, Andrew Dunstan wrote:
>
> On 04/08/2017 12:11 PM, Tom Lane wrote:
>> Andrew Dunstan  writes:
 I think it's partially knowing which target failed, and which
 regression.diffs to display.  If we were able to revamp check-world so
 it outputs a list of targets the regression machinery were able to run
 individually, it'd probably help?
>>> Yes, I don't want just to run check-world.
>> Yup.  The situation with the TAP tests (bin-check step) is already a
>> usability fail: when there's a failure, your first problem is to root
>> through megabytes of poorly-differentiated logs just to figure out
>> what actually failed.  Treating all of check-world as a single buildfarm
>> step would be a disaster.
>>
>>> Instead of just adding targets to check-world, perhaps we need to look
>>> at what we can do so that the buildfarm client can discover what checks
>>> it might run and run them, just as we specify test schedules for pg_regress.
>> +1.  In the meantime, is there any chance of breaking down bin-check into
>> a separate step per src/bin/ subdirectory?
>>
>>  
> Possibly. I will look when I go to do the missing checks, later today or
> tomorrow.
>


OK, crake is running this code now.  See

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

Mark Dilger  writes:
> This is very near where the original crash reported in this thread was 
> crashing, probably only
> different due to the extra lines of Assert that were added.  Am I missing 
> some portion of the
> fix that you are testing?  I have only applied the patch that Tom included in 
> the previous email.

No, that was the whole patch --- but did you do a full "make clean" and
rebuild?  The patch changed NodeTag numbers which would affect the whole
tree.

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] [sqlsmith] Planner crash on foreign table join

[Mark Dilger]

> On Apr 8, 2017, at 6:48 PM, Mark Dilger  wrote:
> 
> 
>> On Apr 8, 2017, at 6:38 PM, Mark Dilger  wrote:
>> 
>> 
>>> On Apr 8, 2017, at 5:13 PM, Tom Lane  wrote:
>>> 
>>> I wrote:
 Robert Haas  writes:
> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
> I think it's pretty dubious to change this, honestly.  Just because it
> would have caught this one bug doesn't make it an especially valuable
> thing in general.  Bytes are still not free.
>>> 
 What I think I might do is write a trial patch that turns Bitmapsets
 into Nodes, and see if it catches any other existing bugs.  If it does
 not, that would be good evidence for your position.
>>> 
>>> I made the attached quick-hack patch, and found that check-world
>>> passes just fine with it. 
>> 
>> Not so for me.  I get a failure almost immediately:
> 
> I recant.  Looks like I didn't get the patch applied quite right.  So sorry 
> for the noise.

The regression tests now fail on a number of tests due to a server crash:

2017-04-08 18:55:19.826 PDT [90779] pg_regress/errors STATEMENT:  select 
infinite_recurse();
TRAP: FailedAssertion("!(const Node*)(a))->type) == T_Bitmapset))", File: 
"bitmapset.c", Line: 601)
2017-04-08 18:55:22.487 PDT [90242] LOG:  server process (PID 90785) was 
terminated by signal 6: Abort trap
2017-04-08 18:55:22.487 PDT [90242] DETAIL:  Failed process was running: 
explain (costs off)
select * from onek2 where unique2 = 11 and stringu1 = 'AT';


This is very near where the original crash reported in this thread was 
crashing, probably only
different due to the extra lines of Assert that were added.  Am I missing some 
portion of the
fix that you are testing?  I have only applied the patch that Tom included in 
the previous email.

mark

-- 
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] [sqlsmith] Planner crash on foreign table join

[Mark Dilger]

> On Apr 8, 2017, at 6:38 PM, Mark Dilger  wrote:
> 
> 
>> On Apr 8, 2017, at 5:13 PM, Tom Lane  wrote:
>> 
>> I wrote:
>>> Robert Haas  writes:
 On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
 I think it's pretty dubious to change this, honestly.  Just because it
 would have caught this one bug doesn't make it an especially valuable
 thing in general.  Bytes are still not free.
>> 
>>> What I think I might do is write a trial patch that turns Bitmapsets
>>> into Nodes, and see if it catches any other existing bugs.  If it does
>>> not, that would be good evidence for your position.
>> 
>> I made the attached quick-hack patch, and found that check-world
>> passes just fine with it. 
> 
> Not so for me.  I get a failure almost immediately:

I recant.  Looks like I didn't get the patch applied quite right.  So sorry for 
the noise.

-- 
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] [sqlsmith] Planner crash on foreign table join

[Mark Dilger]

> On Apr 8, 2017, at 5:13 PM, Tom Lane  wrote:
> 
> I wrote:
>> Robert Haas  writes:
>>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
>>> I think it's pretty dubious to change this, honestly.  Just because it
>>> would have caught this one bug doesn't make it an especially valuable
>>> thing in general.  Bytes are still not free.
> 
>> What I think I might do is write a trial patch that turns Bitmapsets
>> into Nodes, and see if it catches any other existing bugs.  If it does
>> not, that would be good evidence for your position.
> 
> I made the attached quick-hack patch, and found that check-world
> passes just fine with it. 

Not so for me.  I get a failure almost immediately:

Running in no-clean mode.  Mistakes will not be cleaned up.
The files belonging to this database system will be owned by user "mark".
This user must also own the server process.

The database cluster will be initialized with locales
  COLLATE:  en_US.UTF-8
  CTYPE:en_US.UTF-8
  MESSAGES: C
  MONETARY: en_US.UTF-8
  NUMERIC:  en_US.UTF-8
  TIME: en_US.UTF-8
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".

Data page checksums are disabled.

creating directory 
/Users/mark/hydra/postgresql/src/test/regress/./tmp_check/data ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 128MB
selecting dynamic shared memory implementation ... posix
creating configuration files ... ok
running bootstrap script ... TRAP: FailedAssertion("!(const 
Node*)(a))->type) == T_Bitmapset))", File: "bitmapset.c", Line: 731)
child process was terminated by signal 6: Abort trap
initdb: data directory 
"/Users/mark/hydra/postgresql/src/test/regress/./tmp_check/data" not removed at 
user's request



-- 
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] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

I wrote:
> Robert Haas  writes:
>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
>> I think it's pretty dubious to change this, honestly.  Just because it
>> would have caught this one bug doesn't make it an especially valuable
>> thing in general.  Bytes are still not free.

> What I think I might do is write a trial patch that turns Bitmapsets
> into Nodes, and see if it catches any other existing bugs.  If it does
> not, that would be good evidence for your position.

I made the attached quick-hack patch, and found that check-world
passes just fine with it.  That's not complete proof that we have
no other bugs of this ilk, but it definitely supports the idea
that we don't really need to add the overhead.  I'll just put this
in the archives for possible future reference.

(Or perhaps Andreas would like to try bashing on a copy with this
installed.)

regards, tom lane

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index bf8545d..39d7b41 100644
*** a/src/backend/nodes/bitmapset.c
--- b/src/backend/nodes/bitmapset.c
*** bms_copy(const Bitmapset *a)
*** 115,120 
--- 115,121 
  
  	if (a == NULL)
  		return NULL;
+ 	Assert(IsA(a, Bitmapset));
  	size = BITMAPSET_SIZE(a->nwords);
  	result = (Bitmapset *) palloc(size);
  	memcpy(result, a, size);
*** bms_equal(const Bitmapset *a, const Bitm
*** 145,150 
--- 146,153 
  	}
  	else if (b == NULL)
  		return bms_is_empty(a);
+ 	Assert(IsA(a, Bitmapset));
+ 	Assert(IsA(b, Bitmapset));
  	/* Identify shorter and longer input */
  	if (a->nwords <= b->nwords)
  	{
*** bms_make_singleton(int x)
*** 187,192 
--- 190,196 
  	wordnum = WORDNUM(x);
  	bitnum = BITNUM(x);
  	result = (Bitmapset *) palloc0(BITMAPSET_SIZE(wordnum + 1));
+ 	result->type = T_Bitmapset;
  	result->nwords = wordnum + 1;
  	result->words[wordnum] = ((bitmapword) 1 << bitnum);
  	return result;
*** void
*** 201,207 
--- 205,214 
  bms_free(Bitmapset *a)
  {
  	if (a)
+ 	{
+ 		Assert(IsA(a, Bitmapset));
  		pfree(a);
+ 	}
  }
  
  
*** bms_union(const Bitmapset *a, const Bitm
*** 222,227 
--- 229,236 
  	int			otherlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL)
  		return bms_copy(b);
*** bms_intersect(const Bitmapset *a, const 
*** 256,261 
--- 265,272 
  	int			resultlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL || b == NULL)
  		return NULL;
*** bms_difference(const Bitmapset *a, const
*** 287,292 
--- 298,305 
  	int			shortlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL)
  		return NULL;
*** bms_is_subset(const Bitmapset *a, const 
*** 311,316 
--- 324,331 
  	int			longlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL)
  		return true;			/* empty set is a subset of anything */
*** bms_subset_compare(const Bitmapset *a, c
*** 349,354 
--- 364,371 
  	int			longlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL)
  	{
*** bms_is_member(int x, const Bitmapset *a)
*** 427,432 
--- 444,450 
  		elog(ERROR, "negative bitmapset member not allowed");
  	if (a == NULL)
  		return false;
+ 	Assert(IsA(a, Bitmapset));
  	wordnum = WORDNUM(x);
  	bitnum = BITNUM(x);
  	if (wordnum >= a->nwords)
*** bms_overlap(const Bitmapset *a, const Bi
*** 445,450 
--- 463,470 
  	int			shortlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL || b == NULL)
  		return false;
*** bms_overlap_list(const Bitmapset *a, con
*** 468,473 
--- 488,494 
  	int			wordnum,
  bitnum;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
  	if (a == NULL || b == NIL)
  		return false;
  
*** bms_nonempty_difference(const Bitmapset 
*** 496,501 
--- 517,524 
  	int			shortlen;
  	int			i;
  
+ 	Assert(a == NULL || IsA(a, Bitmapset));
+ 	Assert(b == NULL || IsA(b, Bitmapset));
  	/* Handle cases where either input is NULL */
  	if (a == NULL)
  		return false;
*** bms_singleton_member(const Bitmapset *a)
*** 531,536 
--- 554,560 
  
  	if (a == NULL)
  		elog(ERROR, "bitmapset is empty");
+ 	Assert(IsA(a, Bitmapset));
  	nwords = a->nwords;
 

Re: [HACKERS] recent deadlock regression test failures

[Kevin Grittner]

On Sat, Apr 8, 2017 at 12:56 PM, Tom Lane  wrote:
> Thomas Munro  writes:
>> On Sat, Apr 8, 2017 at 4:22 PM, Tom Lane  wrote:

>> Based on the above, here is a version that introduces a simple boolean
>> function pg_waiting_for_safe_snapshot(pid) and adds that the the
>> query.  This was my "option 1" upthread.
>
> Nah, this is not good either.  Yes, it's a fairly precise conversion
> of what Kevin's patch did, but I think that patch is wrong even
> without considering the performance angle.  If you look back at the
> discussions in Feb 2016 that led to what we had, it turned out to be
> important not just to be able to say that process X is blocked, but
> that it is blocked by one of the other isolationtest sessions, and
> not by some random third party (like autovacuum).  I do not know
> whether there is, right now, any way for autovacuum to be the guilty
> party for a SafeSnapshot wait --- but it does not seem like a good
> plan to assume that there never will be.

It would make no sense to run autovacuum at the serializable
transaction isolation level, and only overlapping read-write
serializable transactions can block the attempt to acquire a safe
snapshot.

> So I think what we need is functionality very much like what you had
> in the prior patch, ie identify the specific PIDs that are causing
> process X to wait for a safe snapshot.  I'm just not happy with how
> you packaged it.
>
> Here's a sketch of what I think we ought to do:
>
> 1. Leave pg_blocking_pids() alone; it's a published API now.

Fair enough.

> 2. Add GetSafeSnapshotBlockingPids() more or less as you had it
> in the previous patch (+ Kevin's recommendations).  There might be
> value in providing a SQL-level way to call that, but I'm not sure,
> and it would be independent of fixing isolationtester anyway.

It seems entirely plausible that someone might want to know what is
holding up the start of a backup or large report which uses the READ
ONLY DEFERRABLE option, so I think there is a real use case for a
documented SQL function similar to pg_blocking_pids() to show the
pids of connections currently running transactions which are holding
things up.  Of course, they may not initially know whether it is
being blocked by heavyweight locks or concurrent serializable
read-write transactions, but it should not be a big deal to run two
separate functions.

Given the inability to run isolation tests to cover the deferrable
code, we used a variation on DBT-2 that could cause serialization
anomalies to generate a high concurrency saturation run using
serializable transactions, and started a SERIALIZABLE READ ONLY
DEFERRABLE transaction 1200 times competing with this load, timing
how long it took to start.  To quote the VLDB paper[1], "The median
latency was 1.98 seconds, with 90% of transactions able to obtain a
safe snapshot within 6 seconds, and all within 20 seconds. Given the
intended use (long-running transactions), we believe this delay is
reasonable."  That said, a single long-running serializable
read-write transaction could hold up the attempt indefinitely --
there is no maximum bound.  Hence the benefit of a SQL function to
find out what's happening.

> 3. Invent a SQL function that is dedicated specifically to supporting
> isolationtester and need not be documented at all; this gets us out
> of the problem of whether it's okay to whack its semantics around
> anytime somebody thinks of something else they want to test.
> I'm imagining an API like
>
>   isolation_test_is_waiting_for(int, int[]) returns bool
>
> so that isolationtester's query would reduce to something like
>
> SELECT pg_catalog.isolation_test_is_waiting_for($1, '{...}')
>
> which would take even more cycles out of the parse/plan overhead for it
> (which is basically what's killing the CCA animals).  Internally, this
> function would call pg_blocking_pids and, if necessary,
> GetSafeSnapshotBlockingPids, and would check for matches in its array
> argument directly; it could probably do that significantly faster than the
> general-purpose array && code.  So we'd have to expend a bit of backend C
> code, but we'd have something that would be quite speedy and we could
> customize in future without fear of breaking behavior that users are
> depending on.

Good suggestion.

Thomas, would you like to produce a patch along these lines, or
should I?

--
Kevin Grittner

[1]  Dan R. K. Ports and Kevin Grittner. 2012.
 Serializable Snapshot Isolation in PostgreSQL.
 Proceedings of the VLDB Endowment, Vol. 5, No. 12.
 The 38th International Conference on Very Large Data Bases,
 August 27th - 31st 2012, Istanbul, Turkey.
 http://vldb.org/pvldb/vol5/p1850_danrkports_vldb2012.pdf


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

Re: [pgsql-www] [HACKERS] Small issue in online devel documentation build

[Bruce Momjian]

On Sat, Apr  8, 2017 at 12:50:19PM -0400, Robert Haas wrote:
> On Sat, Apr 8, 2017 at 6:39 AM, Bruce Momjian  wrote:
> > What other problems do we have with pgweb that I can work on?
> 
> Well, the 10devel documentation doesn't believe in orange.  Compare:
> 
> https://www.postgresql.org/docs/devel/static/sql-createtable.html
> https://www.postgresql.org/docs/9.6/static/sql-createtable.html
> 
> I think that needs to be fixed.

The attached CSS patch will fix that specific issue, but I am unclear if
there are more places that need color.

-- 
  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 +
diff --git a/media/css/text.css b/media/css/text.css
new file mode 100644
index 129735c..a910076
*** a/media/css/text.css
--- b/media/css/text.css
***
*** 4,10 
  
  /* Heading Definitions */
  
! h1 {
color: #EC5800;
  }
  
--- 4,10 
  
  /* Heading Definitions */
  
! h1, h2 .refentrytitle {
color: #EC5800;
  }
  

-- 
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] recent deadlock regression test failures

[Tom Lane]

Kevin Grittner  writes:
> On Sat, Apr 8, 2017 at 12:56 PM, Tom Lane  wrote:
>> I'm imagining an API like
>>  isolation_test_is_waiting_for(int, int[]) returns bool

> Good suggestion.

> Thomas, would you like to produce a patch along these lines, or
> should I?

I'll be happy to review/push if someone else does a first draft.

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] recent deadlock regression test failures

[Tom Lane]

... BTW, one other minor coding suggestion for
GetSafeSnapshotBlockingPids(): it might be better to avoid doing so much
palloc work while holding the SerializableXactHashLock.  Even if it's
only held shared, I imagine that it's a contention bottleneck.  You
could avoid that by returning an array rather than a list; the array
could be preallocated of size MaxBackends before ever taking the lock.
That would be a little bit space-wasteful, but since it's only short-lived
storage it doesn't seem like much of a problem.

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] 2017-03 CF Closed

[Josh Berkus]

On 04/08/2017 09:12 AM, Tom Lane wrote:
> Michael Paquier  writes:
>> On Sat, Apr 8, 2017 at 11:27 PM, Andres Freund  wrote:
>>> Thanks for your work on managing the fest!
> 
>> +1. Great work!
> 
> Seconded.  That's a huge amount of generally-underappreciated work.
> 

And only a week over schedule.  Good work.


-- 
Josh Berkus
Containers & Databases Oh My!


-- 
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] [sqlsmith] Planner crash on foreign table join

[Andres Freund]

On 2017-04-08 17:20:28 -0400, Tom Lane wrote:
> Robert Haas  writes:
> > On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
> >> This makes me wonder whether we were being penny-wise and pound-foolish
> >> by not making Bitmapsets be a kind of Node, so that there could be IsA
> >> assertions in the bitmapset.c routines, as there are for Lists.
> 
> > I think it's pretty dubious to change this, honestly.  Just because it
> > would have caught this one bug doesn't make it an especially valuable
> > thing in general.  Bytes are still not free.
> 
> Yeah, true.  OTOH I recall Andres lobbying to change the bitmap word
> size to 64 bits on 64-bit hardware, and it *would* be free in that case
> due to alignment padding.

Hah, yes, I did. A loong time ago ;) I still think it's a good idea, and
probably has become more useful with just about anyone using 64bits
these days.  Also interesting for tidbitmap, which reuses bitmapset's
bitmapword.

> We could also consider installing the nodetag only in Assert-enabled
> builds, although that approach would prevent us from applying followon
> simplifications such as not having to treat bitmapset fields specially
> in copyfuncs.c and like places.

Yea, don't like this much.


- 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] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

Robert Haas  writes:
> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
>> This makes me wonder whether we were being penny-wise and pound-foolish
>> by not making Bitmapsets be a kind of Node, so that there could be IsA
>> assertions in the bitmapset.c routines, as there are for Lists.

> I think it's pretty dubious to change this, honestly.  Just because it
> would have caught this one bug doesn't make it an especially valuable
> thing in general.  Bytes are still not free.

Yeah, true.  OTOH I recall Andres lobbying to change the bitmap word
size to 64 bits on 64-bit hardware, and it *would* be free in that case
due to alignment padding.

What I think I might do is write a trial patch that turns Bitmapsets
into Nodes, and see if it catches any other existing bugs.  If it does
not, that would be good evidence for your position.

We could also consider installing the nodetag only in Assert-enabled
builds, although that approach would prevent us from applying followon
simplifications such as not having to treat bitmapset fields specially
in copyfuncs.c and like places.

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] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

Andrew Gierth  writes:
> "Tom" == Tom Lane  writes:
>  Tom> Experimentation shows that actually, the standard regression tests
>  Tom> provide dozens of opportunities for find_relation_from_clauses to
>  Tom> fail on non-RestrictInfo input.  However, it lacks any IsA check,

> In a discussion with Andres on the hash grouping sets review thread, I
> proposed that we should have something of the form

> #define lfirst_node(_type_, l) (castNode(_type_,lfirst(l)))

That seems like a fairly good idea.  A significant fraction of the
existing castNode() calls are being applied to lfirst(something),
and this would shorten that idiom a bit.

There's another noticeable fraction that are being applied to
linitial(something), but I'm not sure if defining linitial_node()
is worth the trouble.

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] [sqlsmith] Planner crash on foreign table join

[Robert Haas]

On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane  wrote:
> This makes me wonder whether we were being penny-wise and pound-foolish
> by not making Bitmapsets be a kind of Node, so that there could be IsA
> assertions in the bitmapset.c routines, as there are for Lists.  Most
> Bitmapsets in a typical backend probably have only one payload word
> (ie highest member < 32), so right now they occupy 8 bytes.  Adding
> a nodetag would kick them up to the next palloc category, 16 bytes,
> which is probably why I didn't do it like that to begin with.
> Still, that decision is looking unduly byte-miserly in 2017.

I think it's pretty dubious to change this, honestly.  Just because it
would have caught this one bug doesn't make it an especially valuable
thing in general.  Bytes are still not free.

-- 
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] pgbench - allow to store select results into variables

[Fabien COELHO]

As the variable infrastructures are pretty different between psql & pgbench 
(typed vs untyped values, sorted array vs linked list data structure, no hook 
vs 2 hooks, name spaces vs no such thing...), I have chosen the simplest 
option of just copying the name checking function and extending the lexer to 
authorize non-ascii letters, so that psql/pgbench would accept the same 
variable names with the same constraint about encodings.


See patch attached & test script.


Argh, I should be asleep:-(

Wrong patch suffix, wrong from, multiple apology:-(

Here it is again.

--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1eee8dc..502d31b 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -825,6 +825,8 @@ pgbench  options  dbname
  
   Sets variable varname to a value calculated
   from expression.
+  The variable name must consist of letters (including non-Latin letters),
+  digits, and underscores.
   The expression may contain integer constants such as 5432,
   double constants such as 3.14159,
   references to variables :variablename,
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index dc1367b..1862fe4 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -58,9 +58,9 @@ extern void expr_yyset_column(int column_no, yyscan_t yyscanner);
 %option prefix="expr_yy"
 
 /* Character classes */
-alpha			[a-zA-Z_]
+alpha			[a-zA-Z\200-\377_]
 digit			[0-9]
-alnum			[a-zA-Z0-9_]
+alnum			[A-Za-z\200-\377_0-9]
 /* {space} + {nonspace} + {newline} should cover all characters */
 space			[ \t\r\f\v]
 nonspace		[^ \t\r\f\v\n]
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 78f1e6b..85f2edb 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1020,19 +1020,35 @@ makeVariableNumeric(Variable *var)
 	return true;
 }
 
-/* check whether the name consists of alphabets, numerals and underscores. */
+/*
+ * Check whether a variable's name is allowed.
+ *
+ * We allow any non-ASCII character, as well as ASCII letters, digits, and
+ * underscore.  Keep this in sync with the definition of variable_char in
+ * psqlscan.l and psqlscanslash.l.
+ *
+ * This static function is copied from "src/bin/psql/variables.c"
+ */
 static bool
-isLegalVariableName(const char *name)
+valid_variable_name(const char *name)
 {
-	int			i;
+	const unsigned char *ptr = (const unsigned char *) name;
 
-	for (i = 0; name[i] != '\0'; i++)
+	/* Mustn't be zero-length */
+	if (*ptr == '\0')
+		return false;
+
+	while (*ptr)
 	{
-		if (!isalnum((unsigned char) name[i]) && name[i] != '_')
+		if (IS_HIGHBIT_SET(*ptr) ||
+			strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+   "_0123456789", *ptr) != NULL)
+			ptr++;
+		else
 			return false;
 	}
 
-	return (i > 0);/* must be non-empty */
+	return true;
 }
 
 /*
@@ -1054,7 +1070,7 @@ lookupCreateVariable(CState *st, const char *context, char *name)
 		 * Check for the name only when declaring a new variable to avoid
 		 * overhead.
 		 */
-		if (!isLegalVariableName(name))
+		if (!valid_variable_name(name))
 		{
 			fprintf(stderr, "%s: invalid variable name: \"%s\"\n",
 	context, name);


pgbench-utf8-vars.sql
Description: application/sql

-- 
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] [sqlsmith] Planner crash on foreign table join

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 Tom> Experimentation shows that actually, the standard regression tests
 Tom> provide dozens of opportunities for find_relation_from_clauses to
 Tom> fail on non-RestrictInfo input.  However, it lacks any IsA check,

In a discussion with Andres on the hash grouping sets review thread, I
proposed that we should have something of the form

#define lfirst_node(_type_, l) (castNode(_type_,lfirst(l)))

to replace the current idiom of

foreach(l, blah)
{
SomeType *x = (SomeType *) lfirst(l);

(in my code I tend to omit the (SomeType *), which I dislike because it
adds no real protection)

by

foreach(l, blah)
{
SomeType *x = lfirst_node(SomeType, l);

in order to get that IsA check in there in a convenient way.

-- 
Andrew (irc:RhodiumToad)


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

Re: [HACKERS] pgbench - allow to store select results into variables

[Fabien COELHO]

Hello Tatsuo-san,


If I understand correctly, the patch is moved because of the unrelated
issue that variables cannot be utf8 in pgbench, and it is a condition
to consider this patch that existing pgbench variables (set with \set)
can be utf8?


I'm not sure if it is "unrelated" because the new feature relies on
existing pgbench variable infrastructure.


Sure. I meant that the constraint on variable names exists before the patch 
and the patch is not related to variable names, but the patch is about 
variables, obviously.


As "psql" variables can be utf8 and that the same scanner is used, but the 
variables values are not stritcly the same (they are typed in pgbench), I'm 
wondering whether the effort should be do share more code/abstraction between 
psql & pgbench or just adjust/replicate the needed small functions/code 
excerpts.


As the variable infrastructures are pretty different between psql & 
pgbench (typed vs untyped values, sorted array vs linked list data 
structure, no hook vs 2 hooks, name spaces vs no such thing...), I have 
chosen the simplest option of just copying the name checking function and 
extending the lexer to authorize non-ascii letters, so that psql/pgbench 
would accept the same variable names with the same constraint about 
encodings.


See patch attached & test script.

--
Fabien.

pgbench-utf8-vars-1.sql
Description: application/sql


pgbench-utf8-vars.sql
Description: application/sql

-- 
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] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

I wrote:
> Andrew Gierth  writes:
>> Commit ac2b095088 assumes that clauselist_selectivity is being passed a
>> list of RelOptInfo, but postgres_fdw is passing it a list of bare
>> clauses.  One of them is wrong :-)

> It's a bit scary that apparently none of the committed regression tests
> caught that.

Experimentation shows that actually, the standard regression tests
provide dozens of opportunities for find_relation_from_clauses to fail on
non-RestrictInfo input.  However, it lacks any IsA check, and the only
thing that it does with the alleged rinfo is 
if (bms_get_singleton_member(rinfo->clause_relids, ))
As long as there's some kind of object pointer where the clause_relids
field would be, it's highly likely that bms_get_singleton_member will just
return false without crashing, thereby obscuring the fault.  Andreas'
example kills it by causing the argument to be a Param node, whose field
layout doesn't put a pointer there.

This makes me wonder whether we were being penny-wise and pound-foolish
by not making Bitmapsets be a kind of Node, so that there could be IsA
assertions in the bitmapset.c routines, as there are for Lists.  Most
Bitmapsets in a typical backend probably have only one payload word
(ie highest member < 32), so right now they occupy 8 bytes.  Adding
a nodetag would kick them up to the next palloc category, 16 bytes,
which is probably why I didn't do it like that to begin with.
Still, that decision is looking unduly byte-miserly in 2017.

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] Undefined psql variables

[Pavel Stehule]

2017-04-08 12:25 GMT+02:00 Fabien COELHO :

>
> Hello Pavel,
> n
>
>> you proposal disallow client side expressions.
>>
>
> I do agree that some client side expressions are necessary. I do not want
> to disallow them.
>
> I agree so is not possible to mix server side and client side expressions.
>>
>
> My point is that a minimal of cross-support is possible.
>
> But I am sceptic so benefit of server side expression is higher than a
>> lost of client side expressions.
>>
>
> There is a misunderstanding. I am not against client side expression. I do
> want to allow the same server & client side capabilities suggested by Tom,
> I'm just arguing about the syntax, to avoid a prefix oriented approach.
>
> If we disallow server side expressions, then your examples are only two
>> lines longer, but the implementation can be more simpler.
>>
>> SELECT version FROM  app_version
>> \gset
>> \if :version >= 2.0
>> ...
>>
>> Still I don't think so server side expression in \if is good idea.
>>
>
> Ok, so you do not like server-side expression capabities integrated to
> \if. I understood that you were in favor of Tom's proposal.
>

In this moment I see difficult implementation of \if expressions if we
should to separate server side and client side expressions. The prefix
oriented approach is used well in PLpgSQL

FOR IN SELECT, FOR IN ARRAY, FOR IN 1..x

Tom's design should be more simply for implementation and can be simply
extended. There is clean if expr is client side or server side.



>
> From a semantical point of view they are not necessary because the same
> effect can be obtained through \gset, at the price of an intermediate
> variable. So the server-side thing is just a syntax convenience. I think
> that independently of whether they are added, Tom's point is that it should
> be possible to add those features later on, hence the discussion about a
> design.


We are talking about primitive scripting language - that should be simple
how it is possible. One command more, or performance there are not
important - if the performance will not be terrible. The overhead of
intermediate variable is +/- zero against remote expression. But back to
main issue. We should to find syntax if some variable is defined and can be
used or not.

Maybe the solution should not be directly joined with \if command.

what do you think about special \set command, that can be active only when
variable is not defined.

some like

  \setifempty xxx default

Regards

Pavel


>
>
> --
> Fabien.
>

Re: Fwd: Re: [HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Andrew Dunstan]

On 04/08/2017 12:11 PM, Tom Lane wrote:
> Andrew Dunstan  writes:
>>> I think it's partially knowing which target failed, and which
>>> regression.diffs to display.  If we were able to revamp check-world so
>>> it outputs a list of targets the regression machinery were able to run
>>> individually, it'd probably help?
>> Yes, I don't want just to run check-world.
> Yup.  The situation with the TAP tests (bin-check step) is already a
> usability fail: when there's a failure, your first problem is to root
> through megabytes of poorly-differentiated logs just to figure out
> what actually failed.  Treating all of check-world as a single buildfarm
> step would be a disaster.
>
>> Instead of just adding targets to check-world, perhaps we need to look
>> at what we can do so that the buildfarm client can discover what checks
>> it might run and run them, just as we specify test schedules for pg_regress.
> +1.  In the meantime, is there any chance of breaking down bin-check into
> a separate step per src/bin/ subdirectory?
>
>   

Possibly. I will look when I go to do the missing checks, later today or
tomorrow.

cheers

andrew




-- 
Andrew Dunstanhttps://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)

[Pavan Deolasee]

On Wed, Apr 5, 2017 at 11:57 PM, Andres Freund  wrote:

> On 2017-04-05 09:36:47 -0400, Robert Haas wrote:
> > By the way, the "Converting WARM chains back to HOT chains" section of
> > README.WARM seems to be out of date.  Any chance you could update that
> > to reflect the current state and thinking of the patch?
>
> I propose we move this patch to the next CF.  That shouldn't prevent you
> working on it, although focusing on review of patches that still might
> make it wouldn't hurt either.
>
>
Thank you all for the  reviews, feedback, tests, criticism. And apologies
for keep pushing it till the last minute even though it was clear to me
quite some time back the patch is not going to make it. But if I'd given
up, it would have never received whatever little attention it got. The only
thing that disappoints me is that the patch was held back on no strong
technical grounds -  at least none were clear to me. There were concerns
about on-disk changes etc, but most on-disk changes were known for 7 months
now. Reminds me of HOT development, when it would not receive adequate
feedback for quite many months, probably for very similar reasons - complex
patch, changes on-disk format, risky, even though performance gains were
quite substantial. I was much more hopeful this time because we have many
more experts now as compared to then, but we probably have equally more
amount of complex patches to review/commit.

I understand that we would like this patch to go in very early in the
development cycle. So as Alvaro mentioned elsewhere, we will continue to
work on it so that we can get it in as soon as v11 tree open. We shall soon
submit a revised version, with the list of critical things so that we can
discuss them here and get some useful feedback. I hope everyone understands
that the feature of this kind won't happen without on-disk format changes.
So to be able to address any concerns, we will need specific feedback and
workable suggestions, if any.

Finally, my apologies for not spending enough time reviewing other patches.
I know its critical, and I'll try to improve on that. Congratulations to
all whose work got accepted and many thanks to all reviewers/committers/CF
managers. I know how difficult and thankless that work is.

Thanks,
Pavan

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

Re: [HACKERS] recent deadlock regression test failures

[Tom Lane]

Thomas Munro  writes:
> On Sat, Apr 8, 2017 at 4:22 PM, Tom Lane  wrote:
>> It seems an entirely principle-free change in the function's definition.

> You might say that pg_blocking_pid() is about locking only and not
> arbitrary other kinds of waits, but safe snapshots are not completely
> unrelated to locking if you tilt your head at the right angle:
> GetSafeSnapshot waits for all transactions that might hold SIRead
> locks that could affect this transaction's serializability to
> complete.

Well, the problem I'm having is that once we say that pg_blocking_pids()
is not just about heavyweight locks, it's not clear where we stop.
There might be many other cases where, if you dig around in the right data
structures, it'd be possible to say that process X is waiting for process
Y to do something.  How many of those will we expect pg_blocking_pids()
to cover?  And there certainly will be cases where X is waiting for Y but
we don't have any effective way of identifying that.  Is that a bug in
pg_blocking_pids()?

Admittedly, this is a somewhat academic objection; but I dislike taking a
function with a fairly clear, published spec and turning it into something
that does whatever's expedient for a single use-case.

> Based on the above, here is a version that introduces a simple boolean
> function pg_waiting_for_safe_snapshot(pid) and adds that the the
> query.  This was my "option 1" upthread.

Nah, this is not good either.  Yes, it's a fairly precise conversion
of what Kevin's patch did, but I think that patch is wrong even
without considering the performance angle.  If you look back at the
discussions in Feb 2016 that led to what we had, it turned out to be
important not just to be able to say that process X is blocked, but
that it is blocked by one of the other isolationtest sessions, and
not by some random third party (like autovacuum).  I do not know
whether there is, right now, any way for autovacuum to be the guilty
party for a SafeSnapshot wait --- but it does not seem like a good
plan to assume that there never will be.

So I think what we need is functionality very much like what you had
in the prior patch, ie identify the specific PIDs that are causing
process X to wait for a safe snapshot.  I'm just not happy with how
you packaged it.

Here's a sketch of what I think we ought to do:

1. Leave pg_blocking_pids() alone; it's a published API now.

2. Add GetSafeSnapshotBlockingPids() more or less as you had it
in the previous patch (+ Kevin's recommendations).  There might be
value in providing a SQL-level way to call that, but I'm not sure,
and it would be independent of fixing isolationtester anyway.

3. Invent a SQL function that is dedicated specifically to supporting
isolationtester and need not be documented at all; this gets us out
of the problem of whether it's okay to whack its semantics around
anytime somebody thinks of something else they want to test.
I'm imagining an API like

  isolation_test_is_waiting_for(int, int[]) returns bool

so that isolationtester's query would reduce to something like

SELECT pg_catalog.isolation_test_is_waiting_for($1, '{...}')

which would take even more cycles out of the parse/plan overhead for it
(which is basically what's killing the CCA animals).  Internally, this
function would call pg_blocking_pids and, if necessary,
GetSafeSnapshotBlockingPids, and would check for matches in its array
argument directly; it could probably do that significantly faster than the
general-purpose array && code.  So we'd have to expend a bit of backend C
code, but we'd have something that would be quite speedy and we could
customize in future without fear of breaking behavior that users are
depending on.

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: [pgsql-www] [HACKERS] Small issue in online devel documentation build

[Robert Haas]

On Sat, Apr 8, 2017 at 6:39 AM, Bruce Momjian  wrote:
> What other problems do we have with pgweb that I can work on?

Well, the 10devel documentation doesn't believe in orange.  Compare:

https://www.postgresql.org/docs/devel/static/sql-createtable.html
https://www.postgresql.org/docs/9.6/static/sql-createtable.html

I think that needs to be fixed.

-- 
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] [sqlsmith] Planner crash on foreign table join

[Tom Lane]

Andrew Gierth  writes:
> "Andreas" == Andreas Seltenreich  writes:
>  Andreas> testing master at f0e44021df with a loopback postgres_fdw
>  Andreas> installed, I see lots of crashes on queries joining foreign
>  Andreas> tables with various expressions.  Below is a reduced recipe
>  Andreas> for the regression database and a backtrace.

> Commit ac2b095088 assumes that clauselist_selectivity is being passed a
> list of RelOptInfo, but postgres_fdw is passing it a list of bare
> clauses.  One of them is wrong :-)

It's a bit scary that apparently none of the committed regression tests
caught that.

More generally, I think the convention up to now has been that
clauselist_selectivity would work on either RestrictInfos or bare boolean
clauses, caching its results in the former case but succeeding anyway.
If we're to standardize on only one of those behaviors it should certainly
be the former, but I think postgres_fdw is probably not the only code that
will be broken if we remove the option for the latter.

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: Fwd: Re: [HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Heikki Linnakangas]

On 04/08/2017 05:26 PM, Andrew Dunstan wrote:

Yes, I don't want just to run check-world.

I am aware of a few test sets that need to be added, and I'm planning on
doing that this weekend, in fact. Specifically: recovery, subscription,
authentication and SSL. Peter Eisentraut raised this with me about a
week ago.


Thanks, that'd be great!

Beware that src/test/ssl is not safe to run on a multi-user system, 
because it allows connections from localhost with well-known user 
certificates. I've been running it on chipmunk for a while, with the 
attached module script.


- Heikki



TestSSL.pm
Description: Perl program

-- 
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] 2017-03 CF Closed

[Tom Lane]

Michael Paquier  writes:
> On Sat, Apr 8, 2017 at 11:27 PM, Andres Freund  wrote:
>> Thanks for your work on managing the fest!

> +1. Great work!

Seconded.  That's a huge amount of generally-underappreciated work.

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: Fwd: Re: [HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Tom Lane]

Andrew Dunstan  writes:
>> I think it's partially knowing which target failed, and which
>> regression.diffs to display.  If we were able to revamp check-world so
>> it outputs a list of targets the regression machinery were able to run
>> individually, it'd probably help?

> Yes, I don't want just to run check-world.

Yup.  The situation with the TAP tests (bin-check step) is already a
usability fail: when there's a failure, your first problem is to root
through megabytes of poorly-differentiated logs just to figure out
what actually failed.  Treating all of check-world as a single buildfarm
step would be a disaster.

> Instead of just adding targets to check-world, perhaps we need to look
> at what we can do so that the buildfarm client can discover what checks
> it might run and run them, just as we specify test schedules for pg_regress.

+1.  In the meantime, is there any chance of breaking down bin-check into
a separate step per src/bin/ subdirectory?

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] 2017-03 CF Closed

[Michael Paquier]

On Sat, Apr 8, 2017 at 11:27 PM, Andres Freund  wrote:
> Thanks for your work on managing the fest!

+1. Great work!
-- 
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] snapbuild woes

[David Steele]

On 4/8/17 10:29 AM, Erik Rijkers wrote:
> On 2017-04-08 15:56, Andres Freund wrote:
>> On 2017-04-08 09:51:39 -0400, David Steele wrote:
>>> On 3/2/17 7:54 PM, Petr Jelinek wrote:
>>> >
>>> > Yes the copy patch needs rebase as well. But these ones are fine.
>>>
>>> This bug has been moved to CF 2017-07.
>>
>> FWIW, as these are bug-fixes that need to be backpatched, I do plan to
>> work on them soon.
>>
> 
> CF 2017-07 pertains to postgres 11, is that right?

In general, yes, but bugs will always be fixed as needed.  It doesn't
matter what CF they are in.

-- 
-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] snapbuild woes

[Andres Freund]

On 2017-04-08 16:29:10 +0200, Erik Rijkers wrote:
> On 2017-04-08 15:56, Andres Freund wrote:
> > On 2017-04-08 09:51:39 -0400, David Steele wrote:
> > > On 3/2/17 7:54 PM, Petr Jelinek wrote:
> > > >
> > > > Yes the copy patch needs rebase as well. But these ones are fine.
> > > 
> > > This bug has been moved to CF 2017-07.
> > 
> > FWIW, as these are bug-fixes that need to be backpatched, I do plan to
> > work on them soon.
> > 
> 
> CF 2017-07 pertains to postgres 11, is that right?
> 
> But I hope you mean to commit these snapbuild patches before the postgres 10
> release?  As far as I know, logical replication is still very broken without
> them (or at least some of that set of 5 patches - I don't know which ones
> are essential and which may not be).

Yes, these should go into 10 *and* earlier releases, and I don't plan to
wait for 2017-07.

- 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] partitioned tables and contrib/sepgsql

[Stephen Frost]

Joe,

* Joe Conway (m...@joeconway.com) wrote:
> On 04/07/2017 05:36 PM, Robert Haas wrote:
> > On Fri, Apr 7, 2017 at 5:22 PM, Joe Conway  wrote:
> >> 1) commit the 0002 patch now before the feature freeze and follow up
> >>with the regression test patch when ready in a couple of days
> >> 2) hold off on both patches until ready
> >> 3) push both patches to the next commitfest/pg11
> >>
> >> Some argue this is an open issue against the new partitioning feature in
> >> pg10 and therefore should be addressed now, and others do not. I can see
> >> both sides of that argument.
> >>
> >> In any case, thoughts on what to do?
> > 
> > Speaking only for myself, I'm OK with any of those options, provided
> > that that "a couple" means what my dictionary says it means.
> 
> Thanks. I'd prefer not to do #1 actually, and I think we can adhere to
> the dictionary meaning of "a couple" (i.e. by EOD Sunday). Assuming we
> are ready by Sunday I will push both together (#2) or else I will move
> them both together to the next CF (#3).

Sounds good to me.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] snapbuild woes

[Erik Rijkers]

On 2017-04-08 15:56, Andres Freund wrote:

On 2017-04-08 09:51:39 -0400, David Steele wrote:

On 3/2/17 7:54 PM, Petr Jelinek wrote:
>
> Yes the copy patch needs rebase as well. But these ones are fine.

This bug has been moved to CF 2017-07.


FWIW, as these are bug-fixes that need to be backpatched, I do plan to
work on them soon.



CF 2017-07 pertains to postgres 11, is that right?

But I hope you mean to commit these snapbuild patches before the 
postgres 10 release?  As far as I know, logical replication is still 
very broken without them (or at least some of that set of 5 patches - I 
don't know which ones are essential and which may not be).


If it's at all useful I can repeat tests to show how often current 
master still fails (easily 50% or so failure-rate).


This would be the pgbench-over-logical-replication test that I did so 
often earlier on.


thanks,

Erik Rijkers


--
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] 2017-03 CF Closed

[Andres Freund]

Hi David, All,

Thanks for your work on managing the fest!

On 2017-04-08 10:25:06 -0400, David Steele wrote:
> Final stats for the commitfest:
> 
> Committed: 116
> Moved to next CF: 50
> Rejected: 2
> Returned with Feedback: 41
> Total: 209.

Not a bad haul...

- 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: Fwd: Re: [HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Andrew Dunstan]

On 04/08/2017 10:11 AM, Andrew Dunstan wrote:
>
>
>  Forwarded Message 
> Subject:  Re: [HACKERS] Running make check-world in buildfarm (was Re:
> [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM
> authentication.)
> Date: Sat, 8 Apr 2017 07:05:54 -0700
> From: Andres Freund 
> To:   Michael Paquier , Andrew Dunstan
> 
> CC:   Heikki Linnakangas , Tom Lane
> , pgsql-hackers 
>
>
>
> On 2017-04-08 23:01:06 +0900, Michael Paquier wrote:
> > On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas  wrote:
> > > Hmm. It looks like none of the buildfarm members are running the
> > > authentication tests. Nor recovery tests, nor subscription tests. We're
> > > missing a trick here, at least some of the buildfarm members really ought 
> > > to
> > > run "make check-world", we're missing a lot of coverage otherwise.
> > 
> > I recall that Andrew has been favoring as much as possible one folder
> > path per test series in the buildfarm client (perhaps to keep the
> > tests separated and have the logs easier to analyze?). I would not
> > mind much if this is replaced by a pure make check-world, which is
> > what most of the serious hackers do, or at least a make check running
> > from src/test to save us a lot of maintenance pain.
>
> I think it's partially knowing which target failed, and which
> regression.diffs to display.  If we were able to revamp check-world so
> it outputs a list of targets the regression machinery were able to run
> individually, it'd probably help?
>



Yes, I don't want just to run check-world.

I am aware of a few test sets that need to be added, and I'm planning on
doing that this weekend, in fact. Specifically: recovery, subscription,
authentication and SSL. Peter Eisentraut raised this with me about a
week ago.

Instead of just adding targets to check-world, perhaps we need to look
at what we can do so that the buildfarm client can discover what checks
it might run and run them, just as we specify test schedules for pg_regress.

cheers

andrew

-- 

Andrew Dunstanhttps://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

[HACKERS] 2017-03 CF Closed

[David Steele]

The commitfest has officially ended. I have pushed the remaining patches
to the next commitfest or returned then with feedback as appropriate:

Features moved to next CF:
- new plpgsql extra_checks
- Better estimate merging for duplicate vars in clausesel.c
- initdb configurable wal_segment_size
- Vacuum: allow usage of more than 1GB of work mem
- Bug in to_timestamp() [not exactly bug, should be renamed?]
- Push down more UPDATEs/DELETEs in postgres_fdw
- postgres_fdw: support parameterized foreign joins

BUGS moved to next CF:
- snapshot builder fixes
- Fix the optimization to skip WAL-logging on table created in same
transaction
- replace broken GetExistingLocalJoinPath with CreateLocalJoinPath
- Failure at replay for corrupted 2PC files + reduce window between
end-of-recovery record and history file write

Returned with Feedback:
- Indexes with Included Columns

Final stats for the commitfest:

Committed: 116
Moved to next CF: 50
Rejected: 2
Returned with Feedback: 41
Total: 209.

Thank you to all the authors, reviewers, and committers who participated!

-- 
-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] Remaining 2017-03 CF entries

[Andres Freund]

Hi,

On 2017-04-08 13:09:13 +0900, Masahiko Sawada wrote:
> Could you consider the item 2PC on FDW as well? It is marked as "Move
> to Next CF" early yesterday but I'm not sure that reason..

I've not moved it, but given that it was moved just before the feature
freeze, it doesn't seem wrong to me - it's too large a patchset to just
have been merged yesterday afternoon.  I also suspect that some more
design discussion will be needed next cycle...

Greetings,

Andres Freund


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

Re: [HACKERS] postgres_fdw: support parameterized foreign joins

[David Steele]

On 4/7/17 9:54 AM, Arthur Zakirov wrote:
> 
> Marked the patch as "Ready for Commiter". But the patch should be
> commited only after the patch [1].

This submission has been moved to CF 2017-07.

-- 
-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] Push down more UPDATEs/DELETEs in postgres_fdw

[David Steele]

On 3/22/17 6:20 AM, Etsuro Fujita wrote:
> On 2017/02/22 19:57, Rushabh Lathia wrote:
>> Marked this as Ready for Committer.
> 
> I noticed that this item in the CF app was incorrectly marked as
> Committed.  This patch isn't committed, so I returned it to the previous
> status.  I also rebased the patch.  Attached is a new version of the patch.

This submission has been moved to CF 2017-07.

-- 
-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] Bug in to_timestamp().

[David Steele]

On 2/27/17 4:19 AM, Artur Zakirov wrote:
> On 15.02.2017 15:26, Amul Sul wrote:
>>
>> The new status of this patch is: Ready for Committer
>>
> 
> Thank you for your review!

This submission has been moved to CF 2017-07.

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

[David Steele]

On 4/7/17 10:19 PM, Claudio Freire wrote:
> 
> I rebased the early free patch (patch 3) to apply on top of the v9
> patch 2 (it needed some changes). I recognize the early free patch
> didn't get nearly as much scrutiny, so I'm fine with commiting only 2
> if that one's ready to go but 3 isn't.
> 
> If it's decided to go for fixed 128M segments and a binary search of
> segments, I don't think I can get that ready and tested before the
> commitfest ends.

This submission has been moved to CF 2017-07.

-- 
-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] increasing the default WAL segment size

[David Steele]

On 4/7/17 2:59 AM, Beena Emerson wrote:
> I ran tests and following are the details:
> 
> Machine details:
> Architecture:  ppc64le
> Byte Order:Little Endian
> CPU(s):192
> On-line CPU(s) list:   0-191
> Thread(s) per core:8
> Core(s) per socket:1
> Socket(s): 24
> NUMA node(s):  4
> Model: IBM,8286-42A
> 
> clients>  16  32   64  
>  128
> size
> 16MB  18895.63486 28799.48759 37855.39521 27968.88309
> 32MB  18313.1461  29201.44954 40733.80051 32458.74147
> 64 MB18055.73141 30875.28687 42713.54447 38009.60542
> 128MB   18234.31424 33208.65419 48604.5593  45498.27689
> 256MB19524.36498 35740.19032 54686.16898 54060.11168
> 512MB 20351.90719 37426.72174 55045.60719 56194.99349
> 1024MB   19667.67062 35696.19194 53666.60373 54353.0614
> 
> I did not get any degradation, in fact, higher values showed performance
> improvement for higher client count.

This submission has been moved to CF 2017-07.

-- 
-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] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Andres Freund]

On 2017-04-08 23:01:06 +0900, Michael Paquier wrote:
> On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas  wrote:
> > Hmm. It looks like none of the buildfarm members are running the
> > authentication tests. Nor recovery tests, nor subscription tests. We're
> > missing a trick here, at least some of the buildfarm members really ought to
> > run "make check-world", we're missing a lot of coverage otherwise.
> 
> I recall that Andrew has been favoring as much as possible one folder
> path per test series in the buildfarm client (perhaps to keep the
> tests separated and have the logs easier to analyze?). I would not
> mind much if this is replaced by a pure make check-world, which is
> what most of the serious hackers do, or at least a make check running
> from src/test to save us a lot of maintenance pain.

I think it's partially knowing which target failed, and which
regression.diffs to display.  If we were able to revamp check-world so
it outputs a list of targets the regression machinery were able to run
individually, it'd probably help?

- 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] Crash on promotion when recovery.conf is renamed

[David Steele]

On 3/28/17 1:21 AM, Tsunakawa, Takayuki wrote:
> From: Michael Paquier [mailto:michael.paqu...@gmail.com]
>> Okay. I got the message, and I agree with what you say here. You are right
>> by the way, the error messages just use "two-phase file" and not "two-phase
>> STATE file", missed that previously.
>> --
> Thanks, marked as ready for committer, as the code is good and Alexander 
> reported the test success.

This bug has been moved to CF 2017-07.

-- 
-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] WIP: Covering + unique indexes.

[David Steele]

On 4/4/17 2:47 PM, Peter Geoghegan wrote:
> 
> At the very least, you should change comments to note the issue. I
> think it's highly unlikely that this could ever result in a failure to
> find a split point, which there are many defenses against already, but
> I think I would find that difficult to prove. The intent of the code
> is almost as important as the code, at least in my opinion.

This submission as been Returned with Feedback.  Please feel free to
resubmit to a future commitfest.

-- 
-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] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Michael Paquier]

On Sat, Apr 8, 2017 at 7:33 PM, Heikki Linnakangas  wrote:
> Hmm. It looks like none of the buildfarm members are running the
> authentication tests. Nor recovery tests, nor subscription tests. We're
> missing a trick here, at least some of the buildfarm members really ought to
> run "make check-world", we're missing a lot of coverage otherwise.

I recall that Andrew has been favoring as much as possible one folder
path per test series in the buildfarm client (perhaps to keep the
tests separated and have the logs easier to analyze?). I would not
mind much if this is replaced by a pure make check-world, which is
what most of the serious hackers do, or at least a make check running
from src/test to save us a lot of maintenance pain.
-- 
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] postgres_fdw bug in 9.6

[David Steele]

On 4/7/17 3:24 PM, Robert Haas wrote:
> 
> There's probably more to think about here, but those are my question
> on an initial read-through.

This bug has been moved to CF 2017-07.

-- 
-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] snapbuild woes

[Andres Freund]

On 2017-04-08 09:51:39 -0400, David Steele wrote:
> On 3/2/17 7:54 PM, Petr Jelinek wrote:
> > 
> > Yes the copy patch needs rebase as well. But these ones are fine.
> 
> This bug has been moved to CF 2017-07.

FWIW, as these are bug-fixes that need to be backpatched, I do plan to
work on them soon.

- 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] snapbuild woes

[David Steele]

On 3/2/17 7:54 PM, Petr Jelinek wrote:
> 
> Yes the copy patch needs rebase as well. But these ones are fine.

This bug has been moved to CF 2017-07.

-- 
-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] Making clausesel.c Smarter

[David Steele]

On 4/7/17 5:33 PM, Claudio Freire wrote:
> 
> Also, can you add a test case? Cost values could make the test
> fragile, so if that gives you trouble I'll understand, but it'd be
> best to try and test this if possible.

This submission has been moved to CF 2017-07.

-- 
-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] plpgsql - additional extra checks

[David Steele]

> On 1/13/17 6:55 AM, Marko Tiikkaja wrote:
>> On Fri, Jan 13, 2017 at 2:46 AM, Jim Nasby > > wrote:
>>
>> On 1/11/17 5:54 AM, Pavel Stehule wrote:
>>
>> +too_many_rows
>> +
>> + 
>> +  When result is assigned to a variable by
>> INTO clause,
>> +  checks if query returns more than one row. In this case
>> the assignment
>> +  is not deterministic usually - and it can be signal some
>> issues in design.
>>
>>
>> Shouldn't this also apply to
>>
>> var := blah FROM some_table WHERE ...;
>>
>> ?
>>
>> AIUI that's one of the beefs the plpgsql2 project has.
>>
>>
>> No, not at all.  That syntax is undocumented and only works because
>> PL/PgSQL is a hack internally.  We don't use it, and frankly I don't
>> think anyone should.

This submission has been moved to CF 2017-07.

-- 
-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] recent deadlock regression test failures

[Thomas Munro]

On Sat, Apr 8, 2017 at 4:22 PM, Tom Lane  wrote:
> Thomas Munro  writes:
>> Here is an attempt at option 2 from the menu I posted above.  Questions:
>
>> 1.  Does anyone object to this extension of pg_blocking_pids()'s
>> remit?  If so, I could make it a separate function (that was option
>> 3).
>
> It seems an entirely principle-free change in the function's definition.

Well... other backends can block a SERIALIZABLE DEFERRABLE
transaction, so it doesn't seem that unreasonable to expect that a
function named pg_blocking_pids(blocked_pid) described as "get array
of PIDs of sessions blocking specified backend PID" should be able to
tell you who they are.

You might say that pg_blocking_pid() is about locking only and not
arbitrary other kinds of waits, but safe snapshots are not completely
unrelated to locking if you tilt your head at the right angle:
GetSafeSnapshot waits for all transactions that might hold SIRead
locks that could affect this transaction's serializability to
complete.

But I can see that it's an odd case.  Minimal separate function
version attached.

> I'm not actually clear on why Kevin wanted this change in
> isolationtester's wait behavior anyway, so maybe some clarification
> on that would be a good idea.

I can't speak for Kevin but here's my argument as patch author:  One
of the purposes of the isolation tester is to test our transaction
isolation.  SERIALIZABLE DEFERRABLE is a special case of one of our
levels and should be tested.  Statement s3r in the new spec
read-only-anomaly-3.spec runs at that level and causes the backend to
wait for another backend.  Without any change to isolationtester it
would hang on that statement until timeout failure.  Therefore I
proposed that isolationtester should recognise this kind of waiting
and proceed to later steps that can unblock it, like so:

step s3r: SELECT id, balance FROM bank_account WHERE id IN ('X', 'Y')
ORDER BY id; 
step s2wx: UPDATE bank_account SET balance = -11 WHERE id = 'X';
step s2c: COMMIT;
step s3r: <... completed>

> But if we need it, I think probably
> a dedicated function would be a good thing.  We want the wait-checking
> query to be as trivial as possible at the SQL level, so whatever
> semantic oddities it needs to have should be pushed into C code.

Based on the above, here is a version that introduces a simple boolean
function pg_waiting_for_safe_snapshot(pid) and adds that the the
query.  This was my "option 1" upthread.

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


boolean-function.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: [pgsql-www] [HACKERS] Small issue in online devel documentation build

[Bruce Momjian]

On Sat, Apr  8, 2017 at 07:33:15PM +0900, Fabien COELHO wrote:
> 
> >The fix, as Fabien identified, is to conditionally inject additional CSS
> >to be _more_ specific than the first CSS and set the font-size to a
> >simple '1em' so the first CSS is not called twice.  I don't think
> >'important!' is necessary but it would be good to test this.
> 
> I did not wrote the fix, a JS-guru colleague of mine did, and he argued that
> it was useful to fix priorities between competing and contradicting CSS
> rules. Now maybe in this particular case it might work without it.

It would be needed for his version because he wasn't matching the exact
tags of the CSS that the JavaScript generated.

> >Attached is a patch that can be applied to pgweb which should fix all of
> >this.
> 
> Great, the devel doc looks ok now.

Great.

What other problems do we have with pgweb that I can work on?

-- 
  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

[HACKERS] Running make check-world in buildfarm (was Re: [COMMITTERS] pgsql: Use SASLprep to normalize passwords for SCRAM authentication.)

[Heikki Linnakangas]

On 04/08/2017 04:23 AM, Tom Lane wrote:

Heikki Linnakangas  writes:

Use SASLprep to normalize passwords for SCRAM authentication.


The test script that this adds appears to fail unless the environment
selects a UTF8-based locale.  On my RHEL6 machine, I see:

LANG=C make check   fail
LANG=en_US.iso88591 make check  fail
LANG=en_US.utf8 make check  ok


Fixed, thanks!


I'm surprised that more of the buildfarm hasn't fallen over.


Hmm. It looks like none of the buildfarm members are running the 
authentication tests. Nor recovery tests, nor subscription tests. We're 
missing a trick here, at least some of the buildfarm members really 
ought to run "make check-world", we're missing a lot of coverage otherwise.


- Heikki



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

Re: [pgsql-www] [HACKERS] Small issue in online devel documentation build

[Fabien COELHO]

The fix, as Fabien identified, is to conditionally inject additional CSS
to be _more_ specific than the first CSS and set the font-size to a
simple '1em' so the first CSS is not called twice.  I don't think
'important!' is necessary but it would be good to test this.


I did not wrote the fix, a JS-guru colleague of mine did, and he argued 
that it was useful to fix priorities between competing and contradicting 
CSS rules. Now maybe in this particular case it might work without it.



Attached is a patch that can be applied to pgweb which should fix all of
this.


Great, the devel doc looks ok now.

--
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] Undefined psql variables

[Fabien COELHO]

Hello Pavel,
n

you proposal disallow client side expressions.


I do agree that some client side expressions are necessary. I do not want 
to disallow them.


I agree so is not possible to mix server side and client side 
expressions.


My point is that a minimal of cross-support is possible.

But I am sceptic so benefit of server side expression is higher than a 
lost of client side expressions.


There is a misunderstanding. I am not against client side expression. I do 
want to allow the same server & client side capabilities suggested by Tom, 
I'm just arguing about the syntax, to avoid a prefix oriented approach.



If we disallow server side expressions, then your examples are only two
lines longer, but the implementation can be more simpler.

SELECT version FROM  app_version
\gset
\if :version >= 2.0
...

Still I don't think so server side expression in \if is good idea.


Ok, so you do not like server-side expression capabities integrated to 
\if. I understood that you were in favor of Tom's proposal.


From a semantical point of view they are not necessary because the same 
effect can be obtained through \gset, at the price of an intermediate 
variable. So the server-side thing is just a syntax convenience. I think 
that independently of whether they are added, Tom's point is that it 
should be possible to add those features later on, hence the discussion 
about a design.


--
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] [PATCH] Add GUCs for predicate lock promotion thresholds

[Dagfinn Ilmari Mannsåker]

Kevin Grittner  writes:

> Unfortunately, I was unable to get the follow-on patch to allow
> setting by relation into a shape I liked.  Let's see what we can do
> for the next release.

Okay, I'll try and crete a more comprehensive version of it for the next
commitfest.

> The first patch was applied with only very minor tweaks.

Thanks!

> I realize that nothing would break if individual users could set their
> granularity thresholds on individual connections, but as someone who
> has filled the role of DBA, the thought kinda made my skin crawl.  I
> left it as PGC_SIGHUP for now; we can talk about loosening that up
> later, but I think we should have one or more use-cases that outweigh
> the opportunities for confusion and bad choices by individual
> programmers to justify that.

I agree.  The committed version is fine for my current use case.

 - ilmari

-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen


-- 
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] [sqlsmith] Planner crash on foreign table join

[Andrew Gierth]

> "Andreas" == Andreas Seltenreich  writes:

 Andreas> Hi,
 
 Andreas> testing master at f0e44021df with a loopback postgres_fdw
 Andreas> installed, I see lots of crashes on queries joining foreign
 Andreas> tables with various expressions.  Below is a reduced recipe
 Andreas> for the regression database and a backtrace.

Commit ac2b095088 assumes that clauselist_selectivity is being passed a
list of RelOptInfo, but postgres_fdw is passing it a list of bare
clauses.  One of them is wrong :-)

-- 
Andrew (irc:RhodiumToad)


-- 
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] PoC plpgsql - possibility to force custom or generic plan

[Pavel Stehule]

2017-04-08 2:30 GMT+02:00 Peter Eisentraut :

> On 4/6/17 14:32, Pavel Stehule wrote:
> > I like to see any proposals about syntax or implementation.
> >
> > Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada
> > language. The PRAGMA syntax can be used for PRAGMA autonomous with well
> > known syntax. It scales well  - it supports function, block or command
> > level.
>
> I had pragmas implemented in the original autonomous transactions patch
> (https://www.postgresql.org/message-id/659a2fce-b6ee-06de-
> 05c0-c8ed6a019...@2ndquadrant.com).
>  However, the difference there is that the behavior is lexical, specific
> to plpgsql, whereas here you are really just selecting run time
> behavior.  So a GUC, and also something that could apply to other
> places, should be considered.
>

I'll look there - we coordinate work on that.

Regards

Pavel


>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

[HACKERS] [sqlsmith] Planner crash on foreign table join

[Andreas Seltenreich]

Hi,

testing master at f0e44021df with a loopback postgres_fdw installed, I
see lots of crashes on queries joining foreign tables with various
expressions.  Below is a reduced recipe for the regression database and
a backtrace.

regards,
Andreas

--8<---cut here---start->8---
create extension postgres_fdw;
create server myself foreign data wrapper postgres_fdw;
create schema fdw_postgres;
create user fdw login;
grant all on schema public to fdw;
grant all on all tables in schema public to fdw;
create user mapping for public server myself options (user 'fdw');
import foreign schema public from server myself into fdw_postgres;

explain select from
  fdw_postgres.hslot
left join fdw_postgres.num_exp_div
on ((exists (values (1))) and (values (1)) is null);
--8<---cut here---end--->8---

Program terminated with signal SIGSEGV, Segmentation fault.
#0  bms_get_singleton_member (a=0x10, member=member@entry=0x7fffb577cafc) at 
bitmapset.c:577
#1  0x56425107b531 in find_relation_from_clauses (clauses=0x564251a68570, 
root=0x564251a273d8) at clausesel.c:445
#2  clauselist_selectivity (root=root@entry=0x564251a273d8, 
clauses=0x564251a68570, varRelid=varRelid@entry=0, jointype=JOIN_LEFT, 
sjinfo=0x564251a661c0) at clausesel.c:128
#3  0x7f61d3d9f22f in postgresGetForeignJoinPaths (root=, 
joinrel=0x564251a66ba8, outerrel=, innerrel=, 
jointype=, extra=0x7fffb577cc50) at postgres_fdw.c:4466
#4  0x56425108a238 in add_paths_to_joinrel (root=root@entry=0x564251a273d8, 
joinrel=joinrel@entry=0x564251a66ba8, outerrel=outerrel@entry=0x564251a65378, 
innerrel=innerrel@entry=0x564251a65f30, jointype=jointype@entry=JOIN_LEFT, 
sjinfo=sjinfo@entry=0x564251a661c0, restrictlist=0x564251a681c8) at 
joinpath.c:278
#5  0x56425108bff2 in populate_joinrel_with_paths (restrictlist=, sjinfo=0x564251a661c0, joinrel=0x564251a66ba8, rel2=0x564251a65f30, 
rel1=0x564251a65378, root=0x564251a273d8) at joinrels.c:795
#6  make_join_rel (root=root@entry=0x564251a273d8, 
rel1=rel1@entry=0x564251a65378, rel2=rel2@entry=0x564251a65f30) at 
joinrels.c:731
#7  0x56425108c7ef in make_rels_by_clause_joins (other_rels=, old_rel=, root=) at joinrels.c:277
#8  join_search_one_level (root=root@entry=0x564251a273d8, level=level@entry=2) 
at joinrels.c:99
#9  0x564251079bdb in standard_join_search (root=0x564251a273d8, 
levels_needed=2, initial_rels=) at allpaths.c:2385
#10 0x56425107ac7b in make_one_rel (root=root@entry=0x564251a273d8, 
joinlist=joinlist@entry=0x564251a65998) at allpaths.c:184
#11 0x564251099ef4 in query_planner (root=root@entry=0x564251a273d8, 
tlist=tlist@entry=0x0, qp_callback=qp_callback@entry=0x56425109aeb0 
, qp_extra=qp_extra@entry=0x7fffb577cff0) at 
planmain.c:253
#12 0x56425109dbc2 in grouping_planner (root=root@entry=0x564251a273d8, 
inheritance_update=inheritance_update@entry=0 '\000', tuple_fraction=, tuple_fraction@entry=0) at planner.c:1684
#13 0x5642510a0133 in subquery_planner (glob=glob@entry=0x564251a2a6d0, 
parse=parse@entry=0x5642519aac60, parent_root=parent_root@entry=0x0, 
hasRecursion=hasRecursion@entry=0 '\000', 
tuple_fraction=tuple_fraction@entry=0) at planner.c:833
#14 0x5642510a0f71 in standard_planner (parse=0x5642519aac60, 
cursorOptions=256, boundParams=0x0) at planner.c:333
#15 0x5642511458cd in pg_plan_query 
(querytree=querytree@entry=0x5642519aac60, cursorOptions=256, 
boundParams=boundParams@entry=0x0) at postgres.c:802
#16 0x564250fa9a40 in ExplainOneQuery (query=0x5642519aac60, 
cursorOptions=, into=0x0, es=0x564251a513a0, 
queryString=0x564251a09590 "explain select from\n  fdw_postgres.hslot\nleft 
join fdw_postgres.num_exp_div\non ((exists (values (1))) and (values (1)) 
is null);", params=0x0, queryEnv=0x0) at explain.c:367
#17 0x564250faa005 in ExplainQuery (pstate=pstate@entry=0x564251a511f0, 
stmt=stmt@entry=0x564251a0aa58, queryString=queryString@entry=0x564251a09590 
"explain select from\n  fdw_postgres.hslot\nleft join 
fdw_postgres.num_exp_div\non ((exists (values (1))) and (values (1)) is 
null);", params=params@entry=0x0, queryEnv=queryEnv@entry=0x0, 
dest=dest@entry=0x564251a51308) at explain.c:256
#18 0x56425114b9cb in standard_ProcessUtility (pstmt=0x564251a0b2e0, 
queryString=0x564251a09590 "explain select from\n  fdw_postgres.hslot\nleft 
join fdw_postgres.num_exp_div\non ((exists (values (1))) and (values (1)) 
is null);", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, 
dest=0x564251a51308, completionTag=0x7fffb577d390 "") at utility.c:680
#19 0x5642511487b4 in PortalRunUtility (portal=0x564251a07580, 
pstmt=0x564251a0b2e0, isTopLevel=, setHoldSnapshot=, dest=, completionTag=0x7fffb577d390 "") at pquery.c:1179
#20 0x564251149633 in FillPortalStore (portal=portal@entry=0x564251a07580, 
isTopLevel=isTopLevel@entry=1 '\001') at pquery.c:1039
#21 0x56425114a21d in PortalRun