David Rowley <dgrowle...@gmail.com> 于2023年10月11日周三 15:52写道:
> On Wed, 11 Oct 2023 at 15:49, David Rowley <dgrowle...@gmail.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 >