aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorEtsuro Fujita <efujita@postgresql.org>2020-08-07 14:45:04 +0900
committerEtsuro Fujita <efujita@postgresql.org>2020-08-07 14:45:04 +0900
commit1db9c80f835e436ca3c27bccf43135ebf7e3c62b (patch)
treed40fd7d5f63fe91d93331bb93c7f2f534b09c5d5 /src
parent495a9b1fbfe890d5da2aaf52ee06a8fd4f4fe339 (diff)
downloadpostgresql-1db9c80f835e436ca3c27bccf43135ebf7e3c62b.tar.gz
postgresql-1db9c80f835e436ca3c27bccf43135ebf7e3c62b.zip
Fix yet another issue with step generation in partition pruning.
Commit 13838740f fixed some issues with step generation in partition pruning, but there was yet another one: get_steps_using_prefix() assumes that clauses in the passed-in prefix list are sorted in ascending order of their partition key numbers, but the caller failed to ensure this for range partitioning, which led to an assertion failure in debug builds. Adjust the caller function to arrange the clauses in the prefix list in the required order for range partitioning. Back-patch to v11, like the previous commit. Patch by me, reviewed by Amit Langote. Discussion: https://postgr.es/m/CAPmGK16jkXiFG0YqMbU66wte-oJTfW6D1HaNvQf%3D%2B5o9%3Dm55wQ%40mail.gmail.com
Diffstat (limited to 'src')
-rw-r--r--src/backend/partitioning/partprune.c138
-rw-r--r--src/test/regress/expected/partition_prune.out11
-rw-r--r--src/test/regress/sql/partition_prune.sql5
3 files changed, 97 insertions, 57 deletions
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index cd64643246a..d4a126849bd 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1355,7 +1355,6 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
List *eq_clauses = btree_clauses[BTEqualStrategyNumber];
List *le_clauses = btree_clauses[BTLessEqualStrategyNumber];
List *ge_clauses = btree_clauses[BTGreaterEqualStrategyNumber];
- bool pk_has_clauses[PARTITION_MAX_KEYS];
int strat;
/*
@@ -1375,10 +1374,15 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
foreach(lc, btree_clauses[strat])
{
PartClauseInfo *pc = lfirst(lc);
+ ListCell *eq_start;
+ ListCell *le_start;
+ ListCell *ge_start;
ListCell *lc1;
List *prefix = NIL;
List *pc_steps;
bool prefix_valid = true;
+ bool pk_has_clauses;
+ int keyno;
/*
* If this is a clause for the first partition key,
@@ -1403,79 +1407,96 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
continue;
}
- /* (Re-)initialize the pk_has_clauses array */
- Assert(pc->keyno > 0);
- for (i = 0; i < pc->keyno; i++)
- pk_has_clauses[i] = false;
+ eq_start = list_head(eq_clauses);
+ le_start = list_head(le_clauses);
+ ge_start = list_head(ge_clauses);
/*
- * Expressions from = clauses can always be in the
- * prefix, provided they're from an earlier key.
+ * We arrange clauses into prefix in ascending order
+ * of their partition key numbers.
*/
- foreach(lc1, eq_clauses)
+ for (keyno = 0; keyno < pc->keyno; keyno++)
{
- PartClauseInfo *eqpc = lfirst(lc1);
+ pk_has_clauses = false;
- if (eqpc->keyno == pc->keyno)
- break;
- if (eqpc->keyno < pc->keyno)
+ /*
+ * Expressions from = clauses can always be in the
+ * prefix, provided they're from an earlier key.
+ */
+ for_each_cell(lc1, eq_start)
{
- prefix = lappend(prefix, eqpc);
- pk_has_clauses[eqpc->keyno] = true;
- }
- }
+ PartClauseInfo *eqpc = lfirst(lc1);
- /*
- * If we're generating steps for </<= strategy, we can
- * add other <= clauses to the prefix, provided
- * they're from an earlier key.
- */
- if (strat == BTLessStrategyNumber ||
- strat == BTLessEqualStrategyNumber)
- {
- foreach(lc1, le_clauses)
- {
- PartClauseInfo *lepc = lfirst(lc1);
-
- if (lepc->keyno == pc->keyno)
+ if (eqpc->keyno == keyno)
+ {
+ prefix = lappend(prefix, eqpc);
+ pk_has_clauses = true;
+ }
+ else
+ {
+ Assert(eqpc->keyno > keyno);
break;
- if (lepc->keyno < pc->keyno)
+ }
+ }
+ eq_start = lc1;
+
+ /*
+ * If we're generating steps for </<= strategy, we
+ * can add other <= clauses to the prefix,
+ * provided they're from an earlier key.
+ */
+ if (strat == BTLessStrategyNumber ||
+ strat == BTLessEqualStrategyNumber)
+ {
+ for_each_cell(lc1, le_start)
{
- prefix = lappend(prefix, lepc);
- pk_has_clauses[lepc->keyno] = true;
+ PartClauseInfo *lepc = lfirst(lc1);
+
+ if (lepc->keyno == keyno)
+ {
+ prefix = lappend(prefix, lepc);
+ pk_has_clauses = true;
+ }
+ else
+ {
+ Assert(lepc->keyno > keyno);
+ break;
+ }
}
+ le_start = lc1;
}
- }
- /*
- * If we're generating steps for >/>= strategy, we can
- * add other >= clauses to the prefix, provided
- * they're from an earlier key.
- */
- if (strat == BTGreaterStrategyNumber ||
- strat == BTGreaterEqualStrategyNumber)
- {
- foreach(lc1, ge_clauses)
+ /*
+ * If we're generating steps for >/>= strategy, we
+ * can add other >= clauses to the prefix,
+ * provided they're from an earlier key.
+ */
+ if (strat == BTGreaterStrategyNumber ||
+ strat == BTGreaterEqualStrategyNumber)
{
- PartClauseInfo *gepc = lfirst(lc1);
-
- if (gepc->keyno == pc->keyno)
- break;
- if (gepc->keyno < pc->keyno)
+ for_each_cell(lc1, ge_start)
{
- prefix = lappend(prefix, gepc);
- pk_has_clauses[gepc->keyno] = true;
+ PartClauseInfo *gepc = lfirst(lc1);
+
+ if (gepc->keyno == keyno)
+ {
+ prefix = lappend(prefix, gepc);
+ pk_has_clauses = true;
+ }
+ else
+ {
+ Assert(gepc->keyno > keyno);
+ break;
+ }
}
+ ge_start = lc1;
}
- }
- /*
- * Check whether every earlier partition key has at
- * least one clause.
- */
- for (i = 0; i < pc->keyno; i++)
- {
- if (!pk_has_clauses[i])
+ /*
+ * If this key has no clauses, prefix is not valid
+ * anymore.
+ */
+ if (!pk_has_clauses)
{
prefix_valid = false;
break;
@@ -2234,6 +2255,9 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
* 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.
*/
static List *
get_steps_using_prefix(GeneratePruningStepsContext *context,
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 0ee44a92591..3dcc8845b99 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -3747,6 +3747,17 @@ explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b
Filter: ((a >= 1) AND (b >= 1) AND (b >= 2) AND (c >= 2) AND (d >= 0))
(3 rows)
+-- Test that get_steps_using_prefix() handles a prefix that contains multiple
+-- clauses for the partition key b (ie, b >= 1 and b = 2) (This also tests
+-- 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;
+ QUERY PLAN
+------------------------------------------------------------------------------
+ Append
+ -> Seq Scan on rp_prefix_test3_p2
+ Filter: ((a >= 1) AND (b >= 1) AND (d >= 0) AND (b = 2) AND (c = 2))
+(3 rows)
+
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);
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 8d77a9d6a9b..f17240a5e40 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1051,6 +1051,11 @@ create table rp_prefix_test3_p2 partition of rp_prefix_test3 for values from (2,
-- clauses for the partition key b (ie, b >= 1 and b >= 2)
explain (costs off) select * from rp_prefix_test3 where a >= 1 and b >= 1 and b >= 2 and c >= 2 and d >= 0;
+-- Test that get_steps_using_prefix() handles a prefix that contains multiple
+-- clauses for the partition key b (ie, b >= 1 and b = 2) (This also tests
+-- 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);