For hash partition table, if partition key is IS NULL clause,  the
condition in if  in get_steps_using_prefix_recurse:
if (cur_keyno < step_lastkeyno - 1)
is not enough.
Like the decode crashed case, explain select * from hp where a = 1 and b is
null and c = 1;
prefix list just has a = 1 clause.
I try fix this in attached patch.
David Rowley <dgrowle...@gmail.com> 于2023年10月11日周三 10:50写道:

> On Tue, 10 Oct 2023 at 21:31, Sergei Glukhov <s.gluk...@postgrespro.ru>
> wrote:
> > create table hp (a int, b text, c int, d int)
> >    partition by hash (a part_test_int4_ops, b part_test_text_ops, c
> > part_test_int4_ops);
> > create table hp0 partition of hp for values with (modulus 4, remainder
> 0);
> > create table hp3 partition of hp for values with (modulus 4, remainder
> 3);
> > create table hp1 partition of hp for values with (modulus 4, remainder
> 1);
> > create table hp2 partition of hp for values with (modulus 4, remainder
> 2);
> >
> >
> > Another crash in the different place even with the fix:
> > explain select * from hp where a = 1 and b is null and c = 1;
>
> Ouch.  It looks like 13838740f tried to fix things in this area before
> and even added a regression test for it. Namely:
>
> -- Test that get_steps_using_prefix() handles non-NULL step_nullkeys
> explain (costs off) select * from hp_prefix_test where a = 1 and b is
> null and c = 1 and d = 1;
>
> I guess that one does not crash because of the "d = 1" clause is in
> the "start" ListCell in get_steps_using_prefix_recurse(), whereas,
> with your case start is NULL which is an issue for cur_keyno =
> ((PartClauseInfo *) lfirst(start))->keyno;.
>
> 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.
>
> David
>
>
>
From 1640c0d05269c3368fb41fcffc66e38630ff522d Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.w...@openpie.com>
Date: Wed, 11 Oct 2023 11:32:04 +0800
Subject: [PATCH] Fix null partition key pruning for hash parittion table.

---
 src/backend/partitioning/partprune.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/partitioning/partprune.c 
b/src/backend/partitioning/partprune.c
index 7179b22a05..c2a388d454 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -2438,6 +2438,7 @@ 
get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
        List       *result = NIL;
        ListCell   *lc;
        int                     cur_keyno;
+       int                     prefix_lastkeyno;
 
        /* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
        check_stack_depth();
@@ -2445,7 +2446,11 @@ 
get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
        /* Check if we need to recurse. */
        Assert(start != NULL);
        cur_keyno = ((PartClauseInfo *) lfirst(start))->keyno;
-       if (cur_keyno < step_lastkeyno - 1)
+       /* Note that for hash partitioning, if partition key is IS NULL clause,
+        * that partition key will not present in prefix List.
+        */
+       prefix_lastkeyno = ((PartClauseInfo *) llast(prefix))->keyno;
+       if (cur_keyno < step_lastkeyno - 1 && cur_keyno != prefix_lastkeyno)
        {
                PartClauseInfo *pc;
                ListCell   *next_start;
-- 
2.25.1

Reply via email to