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

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Sergei Glukhov <s(dot)glukhov(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Problem, partition pruning for prepared statement with IS NULL clause.
Date: 2023-10-11 07:50:41
Message-ID: CAApHDvpf2VU4JHEBfXJsHF1Aq=+aKbtfKx3dtsZudOKLF2wu7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)

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.

David

Attachment Content-Type Size
fix_get_steps_using_prefix_recurse.patch text/plain 24.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-10-11 07:53:39 Re: Pre-proposal: unicode normalized text
Previous Message Amit Kapila 2023-10-11 07:43:08 Re: [PoC] pg_upgrade: allow to upgrade publisher node