Re: Problem, partition pruning for prepared statement with IS NULL clause.

David Rowley <dgrowleyml(at)gmail(dot)com> 于2023年10月11日周三 15:52写道：

> On Wed, 11 Oct 2023 at 15:49, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > It might have been better if PartClauseInfo could also describe IS
> > NULL quals, but I feel if we do that now then it would require lots of
> > careful surgery in partprune.c to account for that. Probably the fix
> > should be localised to get_steps_using_prefix_recurse() to have it do
> > something like pass the keyno to try and work on rather than trying to
> > get that from the "prefix" list. That way if there's no item in that
> > list for that keyno, we can check in step_nullkeys for the keyno.
> >
> > I'll continue looking.
>
> The fix seems to amount to the attached. The following condition
> assumes that by not recursively processing step_lastkeyno - 1 that
> there will be at least one more PartClauseInfo in the prefix List to
> process. However, that didn't work when that partition key clause was
> covered by an IS NULL clause.
>
> If we adjust the following condition:
>
> if (cur_keyno < step_lastkeyno - 1)
>
> to become:
>
> final_keyno = ((PartClauseInfo *) llast(prefix))->keyno;
> if (cur_keyno < final_keyno)
>

Yeah, aggred.

> then that ensures that the else clause can pick up any clauses for the
> final column mentioned in the 'prefix' list, plus any nullkeys if
> there happens to be any of those too.
>
> For testing, given that 13838740f (from 2020) had a go at fixing this
> already, I'm kinda thinking that it's not overkill to test all
> possible 16 combinations of IS NULL and equality equals on the 4
> partition key column partitioned table that commit added in
> partition_prune.sql.
>
> I added some tests there using \gexec to prevent having to write out
> each of the 16 queries by hand. I tested that pruning worked (i.e 1
> matching partition in EXPLAIN), and that we get the correct results
> (i.e we pruned the correct partition) by running the query and we get
> the expected 1 row after having inserted 16 rows, one for each
> combination of quals to test.
>
> I wanted to come up with some tests that test for multiple quals
> matching the same partition key. This is tricky due to the
> equivalence class code being smart and removing any duplicates or
> marking the rel as dummy when it finds conflicting quals. With hash
> partitioning, we're limited to just equality quals, so maybe something
> could be done with range-partitioned tables instead. I see there are
> some tests just above the ones I modified which try to cover this.
>
> I also tried to outsmart the planner by using Params and prepared
> queries. Namely:
>
> set plan_cache_mode = 'force_generic_plan';
> prepare q1 (int, int, int, int, int, int, int, int) as select
> tableoid::regclass,* from hp_prefix_test where a = $1 and b = $2 and c
> = $3 and d = $4 and a = $5 and b = $6 and c = $7 and d = $8;
> explain (costs off) execute q1 (1,2,3,4,1,2,3,4);
>
> But I was outsmarted again with a gating qual which checked the pairs
> match before doing the scan :-(
>
> Append
> Subplans Removed: 15
> -> Result
> One-Time Filter: (($1 = $5) AND ($2 = $6) AND ($3 = $7) AND ($4 =
> $8))
> -> Seq Scan on hp_prefix_test_p14 hp_prefix_test_1
> Filter: ((a = $5) AND (b = $6) AND (c = $7) AND (d = $8))
>
> I'm aiming to commit these as two separate fixes, so I'm going to go
> look again at the first one and wait to see if anyone wants to comment
> on this patch in the meantime.
>
+1, LGTM

> David
>