Re: [HACKERS] Runtime Partition Pruning

On 21 February 2018 at 13:44, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 20 February 2018 at 23:46, Rajkumar Raghuwanshi
> <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com> wrote:
>> I have applied v10 patch on Amit's v27 over head ad7dbee36. I got "ERROR:
>> partition missing from Append subplans" with the patch. on head and only
>> with Amit's patches query is working fine, Please find test case below.
>
> Thanks for the test case. I can recreate locally. This is down to the
> fact that make_partition_pruneinfo() only makes sub-PartitionPruneInfo
> for actual subpaths found in the Append. Your test case happens to
> match both the part_p1 and part_p2 partitions on the first level of
> iteration, but since no child of part_p2 is found in
> make_partition_pruneinfo, that element in the subpartindex never gets
> set.
>
> The fix might be to just remove the error and silently ignore those
> cases, but I was hoping to keep that around as it might catch other
> bugs. I'm just not sure yet how to do both.
>
> I'll rebase this on Amit's latest patch and have a think about it
> while doing that.

I ended up fixing this another way. The patch currently will build a
PartitionPruneInfo and include that in the Append node for any Append
node which belongs to a partitioned table which has a non-empty set of
clauses to attempt to use for pruning. Currently, at no point in
planning does the patch verify those quals to see if they'd be any use
for partition pruning during execution. Really to be any use they
would need to contain at least one Param which is in a clause which
matches a partitioned key of that partition or some subpartition
thereof.

At the moment these clauses are only verified during the first call of
set_valid_runtime_subplans() which is called the first time the
choose_next_subplan() function is called in nodeAppend.c. It would
be possible to determine this during planning, but it would mean doing
an extra call of generate_partition_clauses(), of which most of the
work would be thrown away, as we'd only really want to know if any
Params match the partition key. I thought it was best to do this
during execution as then we can actually make full use of the work
done in that function and cache the result for reuse each time we need
to redetermine the newly matching subplans when any param matching a
partition key changes. I don't entirely think this is perfect, but
since we can't send the PartitionClauseInfo over to the executor from
the planner it seems like the best option.

The way I fixed the reported error was to cache all the subplans for
the partition at the current hierarchy level and if the quals don't
contain any Params matching the partition key, then we can safely
assume that all of the subplans for that partition must match. The
planner should have pruned any unneeded partitions for this
partitioned table since there are no Params, which would only be known
at run-time. Doing it this way allowed me to keep the sanity check
that alerted you to find this bug in the first place.

Other things I don't particularly like about the current patch are how
I had to construct the partition key expressions in
set_valid_runtime_subplans_recurse(). This pretty much uses code
borrowed from set_baserel_partition_key_exprs(). One way around that
would be to change the partprune.c code to deal with the
partkey->partattrs and consume an expression from the list on attnum =
0. I didn't do that as I want to minimise how much I touch Amit's
patch before it gets committed as doing so would likely just cause
more headaches for me keeping this merged with his work. Another
option to resolve this would be to put the partition key expressions
into the PartitionPruneInfo.

I've attached v11 of the patch.

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