Re: pgsql: Support partition pruning at execution time

On 9 April 2018 at 13:03, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 9 April 2018 at 09:54, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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