Problem, partition pruning for prepared statement with IS NULL clause.
Hello postgres hackers,
I noticed that combination of prepared statement with generic plan and
'IS NULL' clause could lead partition pruning to crash.
Affected versions start from 12 it seems.
'How to repeat' below and an attempt to fix it is in attachment.
Data set:
------
create function part_hashint4_noop(value int4, seed int8)
returns int8 as $$
select value + seed;
$$ language sql strict immutable parallel safe;
create operator class part_test_int4_ops for type int4 using hash as
operator 1 =,
function 2 part_hashint4_noop(int4, int8);
create function part_hashtext_length(value text, seed int8)
returns int8 as $$
select length(coalesce(value, ''))::int8
$$ language sql strict immutable parallel safe;
create operator class part_test_text_ops for type text using hash as
operator 1 =,
function 2 part_hashtext_length(text, int8);
create table hp (a int, b text, c int)
partition by hash (a part_test_int4_ops, b part_test_text_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);
insert into hp values (null, null, 0);
insert into hp values (1, null, 1);
insert into hp values (1, 'xxx', 2);
insert into hp values (null, 'xxx', 3);
insert into hp values (2, 'xxx', 4);
insert into hp values (1, 'abcde', 5);
------
Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');
------
Regargs,
Gluh
Attachments:
fix-partition-pruning-with-isnull.patchtext/x-patch; charset=UTF-8; name=fix-partition-pruning-with-isnull.patchDownload
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 7179b22a05..0fbc2dba33 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -3383,10 +3383,21 @@ perform_pruning_base_step(PartitionPruneContext *context,
bool isnull;
Oid cmpfn;
+ /*
+ * IS NULL clause is not added into array of ExprStates
+ * and thus 'stateidx' based on 'keyno' cannot be used to get
+ * ExprStates array element. Use the index based on 'nvalues'
+ * in order to obtain proper datum from the array.
+ */
+ int exprstateidx =
+ PruneCxtStateIdx(context->partnatts, opstep->step.step_id,
+ nvalues);
+
expr = lfirst(lc1);
stateidx = PruneCxtStateIdx(context->partnatts,
opstep->step.step_id, keyno);
- partkey_datum_from_expr(context, expr, stateidx,
+
+ partkey_datum_from_expr(context, expr, exprstateidx,
&datum, &isnull);
/*
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index bb1223e2b1..a16e08a405 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1921,6 +1921,18 @@ explain (costs off) select * from hp where (a = 1 and b = 'abcde') or (a = 2 and
Filter: (((a = 1) AND (b = 'abcde'::text)) OR ((a = 2) AND (b = 'xxx'::text)) OR ((a IS NULL) AND (b IS NULL)))
(7 rows)
+-- test partition pruning with prepared statement and IS NULL clause.
+prepare stmt AS select * from hp where a is null and b = $1;
+explain (costs off) execute stmt('xxx');
+ QUERY PLAN
+--------------------------------------------
+ Append
+ Subplans Removed: 3
+ -> Seq Scan on hp2 hp_1
+ Filter: ((a IS NULL) AND (b = $1))
+(4 rows)
+
+deallocate stmt;
-- test pruning when not all the partitions exist
drop table hp1;
drop table hp3;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 83fed54b8c..9cafbdd538 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -374,6 +374,11 @@ explain (costs off) select * from hp where a = 2 and b = 'xxx';
explain (costs off) select * from hp where a = 1 and b = 'abcde';
explain (costs off) select * from hp where (a = 1 and b = 'abcde') or (a = 2 and b = 'xxx') or (a is null and b is null);
+-- test partition pruning with prepared statement and IS NULL clause.
+prepare stmt AS select * from hp where a is null and b = $1;
+explain (costs off) execute stmt('xxx');
+deallocate stmt;
+
-- test pruning when not all the partitions exist
drop table hp1;
drop table hp3;
On Fri, Oct 6, 2023 at 06:09:45PM +0400, Sergei Glukhov wrote:
Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');
------
I can confirm the crash in git master.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Fri, Oct 6, 2023 at 05:00:54PM -0400, Bruce Momjian wrote:
On Fri, Oct 6, 2023 at 06:09:45PM +0400, Sergei Glukhov wrote:
Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');
------I can confirm the crash in git master.
There were some UTF8 non-space whitespace characters in the email so
attached is a clean reproducable SQL crash file.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
Attachments:
On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
I noticed that combination of prepared statement with generic plan and
'IS NULL' clause could lead partition pruning to crash.
Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');
Thanks for the detailed report and proposed patch.
I think your proposed fix isn't quite correct. I think the problem
lies in InitPartitionPruneContext() where we assume that the list
positions of step->exprs are in sync with the keyno. If you look at
perform_pruning_base_step() the code there makes a special effort to
skip over any keyno when a bit is set in opstep->nullkeys.
It seems that your patch is adjusting the keyno that's given to the
PruneCxtStateIdx() and it looks like (for your test case) it'll end up
passing keyno==0 when it should be passing keyno==1. keyno is the
index of the partition key, so you can't pass 0 when it's for key
index 1.
I wonder if it's worth expanding the tests further to cover more of
the pruning cases to cover run-time pruning too.
David
Attachments:
fix_runtime_pruning_keyno_idx.patchapplication/octet-stream; name=fix_runtime_pruning_keyno_idx.patchDownload
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index eb8a87fd63..eec84115a7 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -2108,7 +2108,8 @@ InitPartitionPruneContext(PartitionPruneContext *context,
foreach(lc, pruning_steps)
{
PartitionPruneStepOp *step = (PartitionPruneStepOp *) lfirst(lc);
- ListCell *lc2;
+ ListCell *lc2 = list_head(step->exprs);
+
int keyno;
/* not needed for other step kinds */
@@ -2117,34 +2118,39 @@ InitPartitionPruneContext(PartitionPruneContext *context,
Assert(list_length(step->exprs) <= partnatts);
- keyno = 0;
- foreach(lc2, step->exprs)
+ for (keyno = 0; keyno < partnatts; keyno++)
{
- Expr *expr = (Expr *) lfirst(lc2);
+ if (bms_is_member(keyno, step->nullkeys))
+ continue;
/* not needed for Consts */
- if (!IsA(expr, Const))
+ if (lc2 != NULL)
{
- int stateidx = PruneCxtStateIdx(partnatts,
- step->step.step_id,
- keyno);
+ Expr *expr = lfirst(lc2);
- /*
- * When planstate is NULL, pruning_steps is known not to
- * contain any expressions that depend on the parent plan.
- * Information of any available EXTERN parameters must be
- * passed explicitly in that case, which the caller must have
- * made available via econtext.
- */
- if (planstate == NULL)
- context->exprstates[stateidx] =
- ExecInitExprWithParams(expr,
- econtext->ecxt_param_list_info);
- else
- context->exprstates[stateidx] =
- ExecInitExpr(expr, context->planstate);
+ if (!IsA(expr, Const))
+ {
+ int stateidx = PruneCxtStateIdx(partnatts,
+ step->step.step_id,
+ keyno);
+
+ /*
+ * When planstate is NULL, pruning_steps is known not to
+ * contain any expressions that depend on the parent plan.
+ * Information of any available EXTERN parameters must be
+ * passed explicitly in that case, which the caller must have
+ * made available via econtext.
+ */
+ if (planstate == NULL)
+ context->exprstates[stateidx] =
+ ExecInitExprWithParams(expr,
+ econtext->ecxt_param_list_info);
+ else
+ context->exprstates[stateidx] =
+ ExecInitExpr(expr, context->planstate);
+ }
+ lc2 = lnext(step->exprs, lc2);
}
- keyno++;
}
}
}
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index bb1223e2b1..36791293ee 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -1921,6 +1921,19 @@ explain (costs off) select * from hp where (a = 1 and b = 'abcde') or (a = 2 and
Filter: (((a = 1) AND (b = 'abcde'::text)) OR ((a = 2) AND (b = 'xxx'::text)) OR ((a IS NULL) AND (b IS NULL)))
(7 rows)
+-- test run-time pruning
+set plan_cache_mode = 'force_generic_plan';
+prepare stmt (text) AS select * from hp where a is null and b = $1;
+explain (costs off) execute stmt('xxx');
+ QUERY PLAN
+--------------------------------------------
+ Append
+ Subplans Removed: 3
+ -> Seq Scan on hp2 hp_1
+ Filter: ((a IS NULL) AND (b = $1))
+(4 rows)
+
+deallocate stmt;
-- test pruning when not all the partitions exist
drop table hp1;
drop table hp3;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 83fed54b8c..d23133fe43 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -374,6 +374,12 @@ explain (costs off) select * from hp where a = 2 and b = 'xxx';
explain (costs off) select * from hp where a = 1 and b = 'abcde';
explain (costs off) select * from hp where (a = 1 and b = 'abcde') or (a = 2 and b = 'xxx') or (a is null and b is null);
+-- test run-time pruning
+set plan_cache_mode = 'force_generic_plan';
+prepare stmt (text) AS select * from hp where a is null and b = $1;
+explain (costs off) execute stmt('xxx');
+deallocate stmt;
+
-- test pruning when not all the partitions exist
drop table hp1;
drop table hp3;
The comment /* not needed for Consts */ may be more better close to if
(!IsA(expr, Const)).
Others look good to me.
David Rowley <dgrowleyml@gmail.com> 于2023年10月9日周一 07:28写道：
On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.glukhov@postgrespro.ru>
wrote:I noticed that combination of prepared statement with generic plan and
'IS NULL' clause could lead partition pruning to crash.Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');Thanks for the detailed report and proposed patch.
I think your proposed fix isn't quite correct. I think the problem
lies in InitPartitionPruneContext() where we assume that the list
positions of step->exprs are in sync with the keyno. If you look at
perform_pruning_base_step() the code there makes a special effort to
skip over any keyno when a bit is set in opstep->nullkeys.It seems that your patch is adjusting the keyno that's given to the
PruneCxtStateIdx() and it looks like (for your test case) it'll end up
passing keyno==0 when it should be passing keyno==1. keyno is the
index of the partition key, so you can't pass 0 when it's for key
index 1.I wonder if it's worth expanding the tests further to cover more of
the pruning cases to cover run-time pruning too.
I think it's worth doing that.
David
Show quoted text
Hi David,
On 10/9/23 03:26, David Rowley wrote:
On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
I noticed that combination of prepared statement with generic plan and
'IS NULL' clause could lead partition pruning to crash.
Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');Thanks for the detailed report and proposed patch.
I think your proposed fix isn't quite correct. I think the problem
lies in InitPartitionPruneContext() where we assume that the list
positions of step->exprs are in sync with the keyno. If you look at
perform_pruning_base_step() the code there makes a special effort to
skip over any keyno when a bit is set in opstep->nullkeys.It seems that your patch is adjusting the keyno that's given to the
PruneCxtStateIdx() and it looks like (for your test case) it'll end up
passing keyno==0 when it should be passing keyno==1. keyno is the
index of the partition key, so you can't pass 0 when it's for key
index 1.I wonder if it's worth expanding the tests further to cover more of
the pruning cases to cover run-time pruning too.
Thanks for the explanation. I thought by some reason that 'exprstates '
array doesn't
contain elements related to 'IS NULL' clause. Now I see that they are
there and
just empty and untouched.
I verified the patch and it fixes the problem.
Regarding test case,
besides the current test case and the test for dynamic partition
pruning, say,
select a, (select b from hp where a is null and b = a.b) AS b from hp a
where a = 1 and b = 'xxx';
I would like to suggest to slightly refactor 'Test Partition pruning for
HASH partitioning' test
from 'partition_prune.sql' and add one more key field. The reason is
that two-element
key is not enough for thorough testing since it tests mostly corner
cases. Let me know
if it's worth doing.
Example:
------
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);
insert into hp values (null, null, null, 0);
insert into hp values (1, null, 1, 1);
insert into hp values (1, 'xxx', 1, 2);
insert into hp values (null, 'xxx', null, 3);
insert into hp values (2, 'xxx', 2, 4);
insert into hp values (1, 'abcde', 1, 5);
------
Another crash in the different place even with the fix:
------
explain select * from hp where a = 1 and b is null and c = 1;
------
Regards,
Gluh
On Tue, 10 Oct 2023 at 21:31, Sergei Glukhov <s.glukhov@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
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 <dgrowleyml@gmail.com> 于2023年10月11日周三 10:50写道：
Show quoted text
On Tue, 10 Oct 2023 at 21:31, Sergei Glukhov <s.glukhov@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, remainder0);
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
Attachments:
0001-Fix-null-partition-key-pruning-for-hash-parittion-ta.patchtext/plain; charset=US-ASCII; name=0001-Fix-null-partition-key-pruning-for-hash-parittion-ta.patchDownload
From 1640c0d05269c3368fb41fcffc66e38630ff522d Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@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
On Wed, 11 Oct 2023 at 15:49, David Rowley <dgrowleyml@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)
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
Attachments:
fix_get_steps_using_prefix_recurse.patchtext/plain; charset=US-ASCII; name=fix_get_steps_using_prefix_recurse.patchDownload
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 7179b22a05..5df1e59e66 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -2355,9 +2355,10 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
*
* To generate steps, step_lastexpr and step_lastcmpfn are appended to
* expressions and cmpfns, respectively, extracted from the clauses in
- * 'prefix'. Actually, since 'prefix' may contain multiple clauses for the
- * same partition key column, we must generate steps for various combinations
- * of the clauses of different keys.
+ * 'prefix'. Since 'prefix' may contain multiple clauses for each partition
+ * key, and since each step can only contain a single clause for each
+ * partition key, when there are multiple clauses for any given key, must
+ * generate steps for all combinations of the clauses.
*
* For list/range partitioning, callers must ensure that step_nullkeys is
* NULL, and that prefix contains at least one clause for each of the
@@ -2397,7 +2398,7 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
return list_make1(step);
}
- /* Recurse to generate steps for various combinations. */
+ /* Recurse to generate steps for every combination of clauses. */
return get_steps_using_prefix_recurse(context,
step_opstrategy,
step_op_is_ne,
@@ -2438,14 +2439,17 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
List *result = NIL;
ListCell *lc;
int cur_keyno;
+ int final_keyno;
/* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
check_stack_depth();
- /* Check if we need to recurse. */
Assert(start != NULL);
cur_keyno = ((PartClauseInfo *) lfirst(start))->keyno;
- if (cur_keyno < step_lastkeyno - 1)
+ final_keyno = ((PartClauseInfo *) llast(prefix))->keyno;
+
+ /* Check if we need to recurse. */
+ if (cur_keyno < final_keyno)
{
PartClauseInfo *pc;
ListCell *next_start;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 36791293ee..1bfdf37657 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4024,20 +4024,327 @@ explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b
Filter: ((a >= 1) AND (b >= 1) AND (d >= 0) AND (b = 2) AND (c = 2))
(2 rows)
+drop table rp_prefix_test1;
+drop table rp_prefix_test2;
+drop table rp_prefix_test3;
create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
-create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 2, remainder 0);
-create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 2, remainder 1);
--- 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;
+-- create 16 partitions
+select 'create table hp_prefix_test_p' || x::text || ' partition of hp_prefix_test for values with (modulus 16, remainder ' || x::text || ');'
+from generate_Series(0,15) x;
+ ?column?
+---------------------------------------------------------------------------------------------------------
+ create table hp_prefix_test_p0 partition of hp_prefix_test for values with (modulus 16, remainder 0);
+ create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 16, remainder 1);
+ create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 16, remainder 2);
+ create table hp_prefix_test_p3 partition of hp_prefix_test for values with (modulus 16, remainder 3);
+ create table hp_prefix_test_p4 partition of hp_prefix_test for values with (modulus 16, remainder 4);
+ create table hp_prefix_test_p5 partition of hp_prefix_test for values with (modulus 16, remainder 5);
+ create table hp_prefix_test_p6 partition of hp_prefix_test for values with (modulus 16, remainder 6);
+ create table hp_prefix_test_p7 partition of hp_prefix_test for values with (modulus 16, remainder 7);
+ create table hp_prefix_test_p8 partition of hp_prefix_test for values with (modulus 16, remainder 8);
+ create table hp_prefix_test_p9 partition of hp_prefix_test for values with (modulus 16, remainder 9);
+ create table hp_prefix_test_p10 partition of hp_prefix_test for values with (modulus 16, remainder 10);
+ create table hp_prefix_test_p11 partition of hp_prefix_test for values with (modulus 16, remainder 11);
+ create table hp_prefix_test_p12 partition of hp_prefix_test for values with (modulus 16, remainder 12);
+ create table hp_prefix_test_p13 partition of hp_prefix_test for values with (modulus 16, remainder 13);
+ create table hp_prefix_test_p14 partition of hp_prefix_test for values with (modulus 16, remainder 14);
+ create table hp_prefix_test_p15 partition of hp_prefix_test for values with (modulus 16, remainder 15);
+(16 rows)
+
+\gexec
+create table hp_prefix_test_p0 partition of hp_prefix_test for values with (modulus 16, remainder 0);
+create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 16, remainder 1);
+create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 16, remainder 2);
+create table hp_prefix_test_p3 partition of hp_prefix_test for values with (modulus 16, remainder 3);
+create table hp_prefix_test_p4 partition of hp_prefix_test for values with (modulus 16, remainder 4);
+create table hp_prefix_test_p5 partition of hp_prefix_test for values with (modulus 16, remainder 5);
+create table hp_prefix_test_p6 partition of hp_prefix_test for values with (modulus 16, remainder 6);
+create table hp_prefix_test_p7 partition of hp_prefix_test for values with (modulus 16, remainder 7);
+create table hp_prefix_test_p8 partition of hp_prefix_test for values with (modulus 16, remainder 8);
+create table hp_prefix_test_p9 partition of hp_prefix_test for values with (modulus 16, remainder 9);
+create table hp_prefix_test_p10 partition of hp_prefix_test for values with (modulus 16, remainder 10);
+create table hp_prefix_test_p11 partition of hp_prefix_test for values with (modulus 16, remainder 11);
+create table hp_prefix_test_p12 partition of hp_prefix_test for values with (modulus 16, remainder 12);
+create table hp_prefix_test_p13 partition of hp_prefix_test for values with (modulus 16, remainder 13);
+create table hp_prefix_test_p14 partition of hp_prefix_test for values with (modulus 16, remainder 14);
+create table hp_prefix_test_p15 partition of hp_prefix_test for values with (modulus 16, remainder 15);
+-- insert one row for each test to perform.
+insert into hp_prefix_test
+select
+ case a when 0 then null else 1 end,
+ case b when 0 then null else 2 end,
+ case c when 0 then null else 3 end,
+ case d when 0 then null else 4 end
+from
+ generate_series(0,1) a,
+ generate_series(0,1) b,
+ generate_Series(0,1) c,
+ generate_Series(0,1) d;
+-- Ensure partition pruning works correctly for each combination of IS NULL
+-- and equality quals.
+select
+ 'explain (costs off) select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+ ?column?
+-------------------------------------------------------------------------------------------------------------------------------
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+(16 rows)
+
+\gexec
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ QUERY PLAN
+-------------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p0 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c IS NULL) AND (d IS NULL))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p1 hp_prefix_test
+ Filter: ((b IS NULL) AND (c IS NULL) AND (d IS NULL) AND (a = 1))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p2 hp_prefix_test
+ Filter: ((a IS NULL) AND (c IS NULL) AND (d IS NULL) AND (b = 2))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((c IS NULL) AND (d IS NULL) AND (a = 1) AND (b = 2))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p3 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (d IS NULL) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p15 hp_prefix_test
+ Filter: ((b IS NULL) AND (d IS NULL) AND (a = 1) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((a IS NULL) AND (d IS NULL) AND (b = 2) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
QUERY PLAN
-------------------------------------------------------------
- Seq Scan on hp_prefix_test_p1 hp_prefix_test
- Filter: ((b IS NULL) AND (a = 1) AND (c = 1) AND (d = 1))
+ Seq Scan on hp_prefix_test_p5 hp_prefix_test
+ Filter: ((d IS NULL) AND (a = 1) AND (b = 2) AND (c = 3))
(2 rows)
-drop table rp_prefix_test1;
-drop table rp_prefix_test2;
-drop table rp_prefix_test3;
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p4 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c IS NULL) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p14 hp_prefix_test
+ Filter: ((b IS NULL) AND (c IS NULL) AND (a = 1) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p13 hp_prefix_test
+ Filter: ((a IS NULL) AND (c IS NULL) AND (b = 2) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p6 hp_prefix_test
+ Filter: ((c IS NULL) AND (a = 1) AND (b = 2) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p5 hp_prefix_test
+ Filter: ((b IS NULL) AND (a = 1) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p6 hp_prefix_test
+ Filter: ((a IS NULL) AND (b = 2) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+ QUERY PLAN
+---------------------------------------------------------
+ Seq Scan on hp_prefix_test_p4 hp_prefix_test
+ Filter: ((a = 1) AND (b = 2) AND (c = 3) AND (d = 4))
+(2 rows)
+
+-- And ensure we get exactly 1 row from each.
+select
+ 'select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+ ?column?
+-----------------------------------------------------------------------------------------------------------
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+(16 rows)
+
+\gexec
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p0 | | | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p1 | 1 | | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p2 | | 2 | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | 1 | 2 | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p3 | | | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p15 | 1 | | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | | 2 | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p5 | 1 | 2 | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p4 | | | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p14 | 1 | | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p13 | | 2 | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p6 | 1 | 2 | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | | | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p5 | 1 | | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p6 | | 2 | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p4 | 1 | 2 | 3 | 4
+(1 row)
+
drop table hp_prefix_test;
--
-- Check that gen_partprune_steps() detects self-contradiction from clauses
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index d23133fe43..6b4039179f 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1188,16 +1188,49 @@ explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b
-- that the caller arranges clauses in that prefix in the required order)
explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b = 2 and c = 2 and d >= 0;
-create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
-create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 2, remainder 0);
-create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 2, remainder 1);
-
--- 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;
-
drop table rp_prefix_test1;
drop table rp_prefix_test2;
drop table rp_prefix_test3;
+
+create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
+
+-- create 16 partitions
+select 'create table hp_prefix_test_p' || x::text || ' partition of hp_prefix_test for values with (modulus 16, remainder ' || x::text || ');'
+from generate_Series(0,15) x;
+\gexec
+
+-- insert one row for each test to perform.
+insert into hp_prefix_test
+select
+ case a when 0 then null else 1 end,
+ case b when 0 then null else 2 end,
+ case c when 0 then null else 3 end,
+ case d when 0 then null else 4 end
+from
+ generate_series(0,1) a,
+ generate_series(0,1) b,
+ generate_Series(0,1) c,
+ generate_Series(0,1) d;
+
+-- Ensure partition pruning works correctly for each combination of IS NULL
+-- and equality quals.
+select
+ 'explain (costs off) select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+\gexec
+
+-- And ensure we get exactly 1 row from each.
+select
+ 'select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+\gexec
+
drop table hp_prefix_test;
--
David Rowley <dgrowleyml@gmail.com> 于2023年10月11日周三 15:52写道：
On Wed, 11 Oct 2023 at 15:49, David Rowley <dgrowleyml@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
Show quoted text
David
Hi,
Thanks for fixing this!
I verified that issues are fixed.
On 10/11/23 11:50, David Rowley wrote:
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.
Regarding test case for the first patch,
the line 'set plan_cache_mode = 'force_generic_plan';' is not necessary
since cache mode is set at the top of the test. On the other hand test
scenario can silently be loosed if someone set another cache mode
somewhere upper. As you mentioned earlier it's worth maybe adding
the test for run-time partition pruning.
Regards,
Gluh
On Wed, 11 Oct 2023 at 22:09, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
Thanks for fixing this!
I verified that issues are fixed.
Thanks for having a look.
Unfortunately, I'd not long sent the last email and realised that the
step_lastkeyno parameter is now unused and can just be removed from
both get_steps_using_prefix() and get_steps_using_prefix_recurse().
This requires some comment rewriting so I've attempted to do that too
in the attached updated version.
David
Attachments:
fix_get_steps_using_prefix_recurse_v2.patchtext/plain; charset=US-ASCII; name=fix_get_steps_using_prefix_recurse_v2.patchDownload
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 7179b22a05..20b5e01c9e 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -167,7 +167,6 @@ static List *get_steps_using_prefix(GeneratePruningStepsContext *context,
bool step_op_is_ne,
Expr *step_lastexpr,
Oid step_lastcmpfn,
- int step_lastkeyno,
Bitmapset *step_nullkeys,
List *prefix);
static List *get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
@@ -175,7 +174,6 @@ static List *get_steps_using_prefix_recurse(GeneratePruningStepsContext *context
bool step_op_is_ne,
Expr *step_lastexpr,
Oid step_lastcmpfn,
- int step_lastkeyno,
Bitmapset *step_nullkeys,
List *prefix,
ListCell *start,
@@ -1531,7 +1529,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
pc->op_is_ne,
pc->expr,
pc->cmpfn,
- 0,
NULL,
NIL);
opsteps = list_concat(opsteps, pc_steps);
@@ -1657,7 +1654,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
pc->op_is_ne,
pc->expr,
pc->cmpfn,
- pc->keyno,
NULL,
prefix);
opsteps = list_concat(opsteps, pc_steps);
@@ -1731,7 +1727,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
false,
pc->expr,
pc->cmpfn,
- pc->keyno,
nullkeys,
prefix);
opsteps = list_concat(opsteps, pc_steps);
@@ -2350,25 +2345,27 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
/*
* get_steps_using_prefix
- * Generate list of PartitionPruneStepOp steps each consisting of given
+ * Generate a list of PartitionPruneStepOp steps each consisting of given
* opstrategy
*
* To generate steps, step_lastexpr and step_lastcmpfn are appended to
* expressions and cmpfns, respectively, extracted from the clauses in
- * 'prefix'. Actually, since 'prefix' may contain multiple clauses for the
- * same partition key column, we must generate steps for various combinations
- * of the clauses of different keys.
+ * 'prefix'. Since 'prefix' may contain multiple clauses for each partition
+ * key, and since each step can only contain a single clause for each
+ * partition key, when there are multiple clauses for any given key, we must
+ * generate steps for all combinations of the clauses.
*
* For list/range partitioning, callers must ensure that step_nullkeys is
* NULL, and that prefix contains at least one clause for each of the
- * partition keys earlier than one specified in step_lastkeyno if it's
- * greater than zero. For hash partitioning, step_nullkeys is allowed to be
- * non-NULL, but they must ensure that prefix contains at least one clause
- * for each of the partition keys other than those specified in step_nullkeys
- * and step_lastkeyno.
- *
- * For both cases, callers must also ensure that clauses in prefix are sorted
- * in ascending order of their partition key numbers.
+ * partition keys prior to the key that 'step_lastexpr' belongs to.
+ *
+ * For hash partitioning, callers must ensure that 'prefix' contains at least
+ * one clause for each of the partition keys apart from the final key. A bit
+ * set in step_nullkeys can substitute clauses in the 'prefix' list for any
+ * given key. Both may not be specified.
+ *
+ * For each of the above cases, callers must ensure that PartClauseInfos in
+ * 'prefix' are sorted in ascending order of keyno.
*/
static List *
get_steps_using_prefix(GeneratePruningStepsContext *context,
@@ -2376,7 +2373,6 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
bool step_op_is_ne,
Expr *step_lastexpr,
Oid step_lastcmpfn,
- int step_lastkeyno,
Bitmapset *step_nullkeys,
List *prefix)
{
@@ -2397,13 +2393,12 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
return list_make1(step);
}
- /* Recurse to generate steps for various combinations. */
+ /* Recurse to generate steps for every combination of clauses. */
return get_steps_using_prefix_recurse(context,
step_opstrategy,
step_op_is_ne,
step_lastexpr,
step_lastcmpfn,
- step_lastkeyno,
step_nullkeys,
prefix,
list_head(prefix),
@@ -2413,9 +2408,8 @@ get_steps_using_prefix(GeneratePruningStepsContext *context,
/*
* get_steps_using_prefix_recurse
* Recursively generate combinations of clauses for different partition
- * keys and start generating steps upon reaching clauses for the greatest
- * column that is less than the one for which we're currently generating
- * steps (that is, step_lastkeyno)
+ * keys and generate PartitionPruneSteps for each combination of
+ * PartClauseInfos in the 'prefix' list.
*
* 'prefix' is the list of PartClauseInfos.
* 'start' is where we should start iterating for the current invocation.
@@ -2428,7 +2422,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
bool step_op_is_ne,
Expr *step_lastexpr,
Oid step_lastcmpfn,
- int step_lastkeyno,
Bitmapset *step_nullkeys,
List *prefix,
ListCell *start,
@@ -2438,14 +2431,17 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
List *result = NIL;
ListCell *lc;
int cur_keyno;
+ int final_keyno;
/* Actually, recursion would be limited by PARTITION_MAX_KEYS. */
check_stack_depth();
- /* Check if we need to recurse. */
Assert(start != NULL);
cur_keyno = ((PartClauseInfo *) lfirst(start))->keyno;
- if (cur_keyno < step_lastkeyno - 1)
+ final_keyno = ((PartClauseInfo *) llast(prefix))->keyno;
+
+ /* Check if we need to recurse. */
+ if (cur_keyno < final_keyno)
{
PartClauseInfo *pc;
ListCell *next_start;
@@ -2493,7 +2489,6 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context,
step_op_is_ne,
step_lastexpr,
step_lastcmpfn,
- step_lastkeyno,
step_nullkeys,
prefix,
next_start,
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 36791293ee..1bfdf37657 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4024,20 +4024,327 @@ explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b
Filter: ((a >= 1) AND (b >= 1) AND (d >= 0) AND (b = 2) AND (c = 2))
(2 rows)
+drop table rp_prefix_test1;
+drop table rp_prefix_test2;
+drop table rp_prefix_test3;
create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
-create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 2, remainder 0);
-create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 2, remainder 1);
--- 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;
+-- create 16 partitions
+select 'create table hp_prefix_test_p' || x::text || ' partition of hp_prefix_test for values with (modulus 16, remainder ' || x::text || ');'
+from generate_Series(0,15) x;
+ ?column?
+---------------------------------------------------------------------------------------------------------
+ create table hp_prefix_test_p0 partition of hp_prefix_test for values with (modulus 16, remainder 0);
+ create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 16, remainder 1);
+ create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 16, remainder 2);
+ create table hp_prefix_test_p3 partition of hp_prefix_test for values with (modulus 16, remainder 3);
+ create table hp_prefix_test_p4 partition of hp_prefix_test for values with (modulus 16, remainder 4);
+ create table hp_prefix_test_p5 partition of hp_prefix_test for values with (modulus 16, remainder 5);
+ create table hp_prefix_test_p6 partition of hp_prefix_test for values with (modulus 16, remainder 6);
+ create table hp_prefix_test_p7 partition of hp_prefix_test for values with (modulus 16, remainder 7);
+ create table hp_prefix_test_p8 partition of hp_prefix_test for values with (modulus 16, remainder 8);
+ create table hp_prefix_test_p9 partition of hp_prefix_test for values with (modulus 16, remainder 9);
+ create table hp_prefix_test_p10 partition of hp_prefix_test for values with (modulus 16, remainder 10);
+ create table hp_prefix_test_p11 partition of hp_prefix_test for values with (modulus 16, remainder 11);
+ create table hp_prefix_test_p12 partition of hp_prefix_test for values with (modulus 16, remainder 12);
+ create table hp_prefix_test_p13 partition of hp_prefix_test for values with (modulus 16, remainder 13);
+ create table hp_prefix_test_p14 partition of hp_prefix_test for values with (modulus 16, remainder 14);
+ create table hp_prefix_test_p15 partition of hp_prefix_test for values with (modulus 16, remainder 15);
+(16 rows)
+
+\gexec
+create table hp_prefix_test_p0 partition of hp_prefix_test for values with (modulus 16, remainder 0);
+create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 16, remainder 1);
+create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 16, remainder 2);
+create table hp_prefix_test_p3 partition of hp_prefix_test for values with (modulus 16, remainder 3);
+create table hp_prefix_test_p4 partition of hp_prefix_test for values with (modulus 16, remainder 4);
+create table hp_prefix_test_p5 partition of hp_prefix_test for values with (modulus 16, remainder 5);
+create table hp_prefix_test_p6 partition of hp_prefix_test for values with (modulus 16, remainder 6);
+create table hp_prefix_test_p7 partition of hp_prefix_test for values with (modulus 16, remainder 7);
+create table hp_prefix_test_p8 partition of hp_prefix_test for values with (modulus 16, remainder 8);
+create table hp_prefix_test_p9 partition of hp_prefix_test for values with (modulus 16, remainder 9);
+create table hp_prefix_test_p10 partition of hp_prefix_test for values with (modulus 16, remainder 10);
+create table hp_prefix_test_p11 partition of hp_prefix_test for values with (modulus 16, remainder 11);
+create table hp_prefix_test_p12 partition of hp_prefix_test for values with (modulus 16, remainder 12);
+create table hp_prefix_test_p13 partition of hp_prefix_test for values with (modulus 16, remainder 13);
+create table hp_prefix_test_p14 partition of hp_prefix_test for values with (modulus 16, remainder 14);
+create table hp_prefix_test_p15 partition of hp_prefix_test for values with (modulus 16, remainder 15);
+-- insert one row for each test to perform.
+insert into hp_prefix_test
+select
+ case a when 0 then null else 1 end,
+ case b when 0 then null else 2 end,
+ case c when 0 then null else 3 end,
+ case d when 0 then null else 4 end
+from
+ generate_series(0,1) a,
+ generate_series(0,1) b,
+ generate_Series(0,1) c,
+ generate_Series(0,1) d;
+-- Ensure partition pruning works correctly for each combination of IS NULL
+-- and equality quals.
+select
+ 'explain (costs off) select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+ ?column?
+-------------------------------------------------------------------------------------------------------------------------------
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+(16 rows)
+
+\gexec
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ QUERY PLAN
+-------------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p0 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c IS NULL) AND (d IS NULL))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p1 hp_prefix_test
+ Filter: ((b IS NULL) AND (c IS NULL) AND (d IS NULL) AND (a = 1))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p2 hp_prefix_test
+ Filter: ((a IS NULL) AND (c IS NULL) AND (d IS NULL) AND (b = 2))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((c IS NULL) AND (d IS NULL) AND (a = 1) AND (b = 2))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p3 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (d IS NULL) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p15 hp_prefix_test
+ Filter: ((b IS NULL) AND (d IS NULL) AND (a = 1) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((a IS NULL) AND (d IS NULL) AND (b = 2) AND (c = 3))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
QUERY PLAN
-------------------------------------------------------------
- Seq Scan on hp_prefix_test_p1 hp_prefix_test
- Filter: ((b IS NULL) AND (a = 1) AND (c = 1) AND (d = 1))
+ Seq Scan on hp_prefix_test_p5 hp_prefix_test
+ Filter: ((d IS NULL) AND (a = 1) AND (b = 2) AND (c = 3))
(2 rows)
-drop table rp_prefix_test1;
-drop table rp_prefix_test2;
-drop table rp_prefix_test3;
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ QUERY PLAN
+---------------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p4 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c IS NULL) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p14 hp_prefix_test
+ Filter: ((b IS NULL) AND (c IS NULL) AND (a = 1) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p13 hp_prefix_test
+ Filter: ((a IS NULL) AND (c IS NULL) AND (b = 2) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p6 hp_prefix_test
+ Filter: ((c IS NULL) AND (a = 1) AND (b = 2) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ QUERY PLAN
+-----------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p12 hp_prefix_test
+ Filter: ((a IS NULL) AND (b IS NULL) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p5 hp_prefix_test
+ Filter: ((b IS NULL) AND (a = 1) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ QUERY PLAN
+-------------------------------------------------------------
+ Seq Scan on hp_prefix_test_p6 hp_prefix_test
+ Filter: ((a IS NULL) AND (b = 2) AND (c = 3) AND (d = 4))
+(2 rows)
+
+explain (costs off) select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+ QUERY PLAN
+---------------------------------------------------------
+ Seq Scan on hp_prefix_test_p4 hp_prefix_test
+ Filter: ((a = 1) AND (b = 2) AND (c = 3) AND (d = 4))
+(2 rows)
+
+-- And ensure we get exactly 1 row from each.
+select
+ 'select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+ ?column?
+-----------------------------------------------------------------------------------------------------------
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+(16 rows)
+
+\gexec
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p0 | | | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p1 | 1 | | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p2 | | 2 | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | 1 | 2 | |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p3 | | | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p15 | 1 | | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d is null
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | | 2 | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d is null
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p5 | 1 | 2 | 3 |
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c is null and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p4 | | | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c is null and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p14 | 1 | | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c is null and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p13 | | 2 | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c is null and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p6 | 1 | 2 | | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b is null and c = 3 and d = 4
+ tableoid | a | b | c | d
+--------------------+---+---+---+---
+ hp_prefix_test_p12 | | | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b is null and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p5 | 1 | | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a is null and b = 2 and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p6 | | 2 | 3 | 4
+(1 row)
+
+select tableoid::regclass,* from hp_prefix_test where a = 1 and b = 2 and c = 3 and d = 4
+ tableoid | a | b | c | d
+-------------------+---+---+---+---
+ hp_prefix_test_p4 | 1 | 2 | 3 | 4
+(1 row)
+
drop table hp_prefix_test;
--
-- Check that gen_partprune_steps() detects self-contradiction from clauses
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index d23133fe43..6b4039179f 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1188,16 +1188,49 @@ explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b
-- that the caller arranges clauses in that prefix in the required order)
explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b = 2 and c = 2 and d >= 0;
-create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
-create table hp_prefix_test_p1 partition of hp_prefix_test for values with (modulus 2, remainder 0);
-create table hp_prefix_test_p2 partition of hp_prefix_test for values with (modulus 2, remainder 1);
-
--- 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;
-
drop table rp_prefix_test1;
drop table rp_prefix_test2;
drop table rp_prefix_test3;
+
+create table hp_prefix_test (a int, b int, c int, d int) partition by hash (a part_test_int4_ops, b part_test_int4_ops, c part_test_int4_ops, d part_test_int4_ops);
+
+-- create 16 partitions
+select 'create table hp_prefix_test_p' || x::text || ' partition of hp_prefix_test for values with (modulus 16, remainder ' || x::text || ');'
+from generate_Series(0,15) x;
+\gexec
+
+-- insert one row for each test to perform.
+insert into hp_prefix_test
+select
+ case a when 0 then null else 1 end,
+ case b when 0 then null else 2 end,
+ case c when 0 then null else 3 end,
+ case d when 0 then null else 4 end
+from
+ generate_series(0,1) a,
+ generate_series(0,1) b,
+ generate_Series(0,1) c,
+ generate_Series(0,1) d;
+
+-- Ensure partition pruning works correctly for each combination of IS NULL
+-- and equality quals.
+select
+ 'explain (costs off) select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+\gexec
+
+-- And ensure we get exactly 1 row from each.
+select
+ 'select tableoid::regclass,* from hp_prefix_test where ' ||
+ string_agg(c.colname || case when g.s & (1 << c.colpos) = 0 then ' is null' else ' = ' || (colpos+1)::text end, ' and ' order by c.colpos)
+from (values('a',0),('b',1),('c',2),('d',3)) c(colname, colpos), generate_Series(0,15) g(s)
+group by g.s
+order by g.s;
+\gexec
+
drop table hp_prefix_test;
--
On 10/11/23 13:19, David Rowley wrote:
Thanks for having a look.
Unfortunately, I'd not long sent the last email and realised that the
step_lastkeyno parameter is now unused and can just be removed from
both get_steps_using_prefix() and get_steps_using_prefix_recurse().
This requires some comment rewriting so I've attempted to do that too
in the attached updated version.
Thanks, verified again and everything is fine!
Regards,
Gluh
On Wed, 11 Oct 2023 at 22:59, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
Unfortunately, I'd not long sent the last email and realised that the
step_lastkeyno parameter is now unused and can just be removed from
both get_steps_using_prefix() and get_steps_using_prefix_recurse().
This requires some comment rewriting so I've attempted to do that too
in the attached updated version.Thanks, verified again and everything is fine!
Thanks for looking. I spent quite a bit more time on this again today
and fiddled lots more with the comments and tests.
I also did more testing after finding a way to easily duplicate the
quals to cause multiple quals per partition key. The equivalence
class code will only make ECs for mergejoin-able clauses, so if we
just find a type that's not mergejoin-able but hashable, we can
duplicate the quals with a hash partitioned table
-- find a suitable non-mergejoin-able type.
select oprleft::regtype from pg_operator where oprcanmerge=false and
oprcanhash=true;
oprleft
---------
xid
cid
aclitem
create table hash_xid(a xid, b xid, c xid) partition by hash(a,b,c);
create table hash_xid1 partition of hash_xid for values with (modulus
2, remainder 0);
create table hash_xid2 partition of hash_xid for values with (modulus
2, remainder 1);
I tried out various combinations of the following query. Each
equality clause is duplicated 6 times. When I enable all 6 for each
of the 3 columns, I see 216 pruning steps. That's 6*6*6, just what I
expected.
The IS NULL quals are not duplicated since we can only set a bit once
in the nullkeys.
explain select * from hash_xid where
a = '123'::xid and a = '123'::xid and a = '123'::xid and a =
'123'::xid and a = '123'::xid and a = '123'::xid and
--a is null and a is null and a is null and a is null and a is null
and a is null and
b = '123'::xid and b = '123'::xid and b = '123'::xid and b =
'123'::xid and b = '123'::xid and b = '123'::xid and
--b is null and b is null and b is null and b is null and b is null
and b is null and
c = '123'::xid and c = '123'::xid and c = '123'::xid and c =
'123'::xid and c = '123'::xid and c = '123'::xid;
--c is null and c is null and c is null and c is null and c is null
and c is null;
putting a breakpoint at the final line of
gen_prune_steps_from_opexps() yields 216 steps.
I didn't include anything of the above as part of the additional
tests. Perhaps something like that is worthwhile in a reduced form.
However, someone might make xid mergejoinable some time, which would
break the test.
Thanks for reviewing the previous version of this patch.
Onto the other run-time one now...
David
On Mon, 9 Oct 2023 at 12:26, David Rowley <dgrowleyml@gmail.com> wrote:
On Sat, 7 Oct 2023 at 03:11, Sergei Glukhov <s.glukhov@postgrespro.ru> wrote:
I noticed that combination of prepared statement with generic plan and
'IS NULL' clause could lead partition pruning to crash.Test case:
------
set plan_cache_mode to force_generic_plan;
prepare stmt AS select * from hp where a is null and b = $1;
explain execute stmt('xxx');Thanks for the detailed report and proposed patch.
I think your proposed fix isn't quite correct. I think the problem
lies in InitPartitionPruneContext() where we assume that the list
positions of step->exprs are in sync with the keyno. If you look at
perform_pruning_base_step() the code there makes a special effort to
skip over any keyno when a bit is set in opstep->nullkeys.
I've now also pushed the fix for the incorrect logic for nullkeys in
ExecInitPruningContext().
I didn't quite find a test to make this work for v11. I tried calling
execute 5 times as we used to have to before the plan_cache_mode GUC
was added in v12, but the test case kept picking the custom plan. So I
ended up pushing v11 without any test. This goes out of support in ~1
month, so I'm not too concerned about the lack of test. I did do a
manual test to ensure it works with:
create table hp (a int, b text, c int) partition by hash (a, b);
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);
prepare hp_q1 (text) as select * from hp where a is null and b = $1;
(set breakpoint in choose_custom_plan() and have it return false when
we hit it.)
explain (costs off) execute hp_q1('xxx');
David