On 9 April 2018 at 13:03, David Rowley <david.row...@2ndquadrant.com> wrote: > On 9 April 2018 at 09:54, Tom Lane <t...@sss.pgh.pa.us> wrote: >> BTW, pademelon just exhibited a different instability in this test: >> >> *** >> /home/bfarm/bf-data/HEAD/pgsql.build/src/test/regress/expected/partition_prune.out >> Sun Apr 8 01:56:04 2018 >> --- >> /home/bfarm/bf-data/HEAD/pgsql.build/src/test/regress/results/partition_prune.out >> Sun Apr 8 17:48:14 2018 >> *************** >> *** 1606,1612 **** >> -> Partial Aggregate (actual rows=1 loops=3) >> -> Parallel Append (actual rows=0 loops=3) >> Subplans Removed: 6 >> ! -> Parallel Seq Scan on ab_a2_b1 (actual rows=0 >> loops=1) >> Filter: ((a >= $1) AND (a <= $2) AND (b < 4)) >> -> Parallel Seq Scan on ab_a2_b2 (actual rows=0 >> loops=1) >> Filter: ((a >= $1) AND (a <= $2) AND (b < 4)) >> --- 1606,1612 ---- >> -> Partial Aggregate (actual rows=1 loops=3) >> -> Parallel Append (actual rows=0 loops=3) >> Subplans Removed: 6 >> ! -> Parallel Seq Scan on ab_a2_b1 (actual rows=0 >> loops=2) >> Filter: ((a >= $1) AND (a <= $2) AND (b < 4)) >> -> Parallel Seq Scan on ab_a2_b2 (actual rows=0 >> loops=1) >> Filter: ((a >= $1) AND (a <= $2) AND (b < 4)) >> > > Reading code it looks like a bug in choose_next_subplan_for_worker(): > > The following needs to be changed for this patch: > > /* Advance to next plan. */ > pstate->pa_next_plan++; > > I'll think a bit harder about the best way to fix and submit a patch > for it later.
Okay, I've written and attached a fix for this. I'm not 100% certain that this is the cause of the problem on pademelon, but the code does look wrong, so needs to be fixed. Hopefully, it'll make pademelon happy, if not I'll think a bit harder about what might be causing that instability. I've also attached a 2nd patch to fix a spelling mistake and a misleading comment in the code. The misleading comment claimed we unset the extern params so we didn't perform pruning again using these. I'd failed to update this comment after I realised that we still need to attempt to prune again with the external params since quals against the partition key may actually contain a mix of exec and external params, which would mean that it's only possible to prune partitions using both types of params. No actual code needs to be updated since the 2nd pass of pruning uses "allparams" anyway. We could actually get away without the bms_free() and set to NULL in the lines below the comment, but I wanted some way to ensure that we never write any code which calls the function twice on the same PartitionPruneState, but maybe I'm just overly paranoid there. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
0001-Fix-incorrect-logic-for-choosing-the-next-Parallel-A.patch
Description: Binary data
0002-Fix-misleading-comment-and-typo.patch
Description: Binary data