diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index badd31a44c..253c690649 100644 --- a/src/backend/partitioning/partprune.c +++ b/src/backend/partitioning/partprune.c @@ -1058,8 +1058,12 @@ gen_partprune_steps_internal(GeneratePruningStepsContext *context, case PARTCLAUSE_MATCH_NULLNESS: if (!clause_is_not_null) { - /* check for conflicting IS NOT NULL */ - if (bms_is_member(i, notnullkeys)) + /* + * check for conflicting IS NOT NULL as well as + * contradicting strict clauses + */ + if (bms_is_member(i, notnullkeys) || + keyclauses[i] != NIL) { context->contradictory = true; return NIL; @@ -1308,34 +1312,18 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, { case PARTITION_STRATEGY_LIST: case PARTITION_STRATEGY_RANGE: - { - PartClauseInfo *last = NULL; + btree_clauses[pc->op_strategy] = + lappend(btree_clauses[pc->op_strategy], pc); - /* - * Add this clause to the list of clauses to be used - * for pruning if this is the first such key for this - * operator strategy or if it is consecutively next to - * the last column for which a clause with this - * operator strategy was matched. - */ - if (btree_clauses[pc->op_strategy] != NIL) - last = llast(btree_clauses[pc->op_strategy]); - - if (last == NULL || - i == last->keyno || i == last->keyno + 1) - btree_clauses[pc->op_strategy] = - lappend(btree_clauses[pc->op_strategy], pc); - - /* - * We can't consider subsequent partition keys if the - * clause for the current key contains a non-inclusive - * operator. - */ - if (pc->op_strategy == BTLessStrategyNumber || - pc->op_strategy == BTGreaterStrategyNumber) - consider_next_key = false; - break; - } + /* + * We can't consider subsequent partition keys if the + * clause for the current key contains a non-inclusive + * operator. + */ + if (pc->op_strategy == BTLessStrategyNumber || + pc->op_strategy == BTGreaterStrategyNumber) + consider_next_key = false; + break; case PARTITION_STRATEGY_HASH: if (pc->op_strategy != HTEqualStrategyNumber) @@ -1374,6 +1362,7 @@ 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; /* @@ -1396,6 +1385,35 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, ListCell *lc1; List *prefix = NIL; List *pc_steps; + bool prefix_valid = true; + + /* + * If this is a clause for the first partition key, + * there are no preceding expressions; generate a + * pruning step without a prefix. + * + * Note that we pass NULL for step_nullkeys, because + * we don't search list/range partition bounds where + * some keys are NULL. + */ + if (pc->keyno == 0) + { + Assert(pc->op_strategy == strat); + pc_steps = get_steps_using_prefix(context, strat, + pc->op_is_ne, + pc->expr, + pc->cmpfn, + 0, + NULL, + NIL); + opsteps = list_concat(opsteps, pc_steps); + continue; + } + + /* (Re-)initialize the pk_has_clauses array */ + Assert(pc->keyno > 0); + for (i = 0; i < pc->keyno; i++) + pk_has_clauses[i] = false; /* * Expressions from = clauses can always be in the @@ -1408,7 +1426,10 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, if (eqpc->keyno == pc->keyno) break; if (eqpc->keyno < pc->keyno) + { prefix = lappend(prefix, eqpc); + pk_has_clauses[eqpc->keyno] = true; + } } /* @@ -1426,7 +1447,10 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, if (lepc->keyno == pc->keyno) break; if (lepc->keyno < pc->keyno) + { prefix = lappend(prefix, lepc); + pk_has_clauses[lepc->keyno] = true; + } } } @@ -1445,11 +1469,33 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, if (gepc->keyno == pc->keyno) break; if (gepc->keyno < pc->keyno) + { prefix = lappend(prefix, gepc); + pk_has_clauses[gepc->keyno] = true; + } } } /* + * Check whether every earlier partition key has at + * least one clause. + */ + for (i = 0; i < pc->keyno; i++) + { + if (!pk_has_clauses[i]) + { + prefix_valid = false; + break; + } + } + + /* + * If prefix_valid, generate PartitionPruneStepOps. + * Otherwise, we would not find clauses for a valid + * subset of the partition keys anymore for the + * strategy; give up on generating partition pruning + * steps further for the strategy. + * * As mentioned above, if 'prefix' contains multiple * expressions for the same key, the following will * generate multiple steps, one for each combination @@ -1459,15 +1505,20 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context, * we don't search list/range partition bounds where * some keys are NULL. */ - Assert(pc->op_strategy == strat); - pc_steps = get_steps_using_prefix(context, strat, - pc->op_is_ne, - pc->expr, - pc->cmpfn, - pc->keyno, - NULL, - prefix); - opsteps = list_concat(opsteps, pc_steps); + if (prefix_valid) + { + Assert(pc->op_strategy == strat); + pc_steps = get_steps_using_prefix(context, strat, + pc->op_is_ne, + pc->expr, + pc->cmpfn, + pc->keyno, + NULL, + prefix); + opsteps = list_concat(opsteps, pc_steps); + } + else + break; } } break; @@ -2182,6 +2233,14 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context, * '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. + * + * 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. */ static List * get_steps_using_prefix(GeneratePruningStepsContext *context, @@ -2193,6 +2252,9 @@ get_steps_using_prefix(GeneratePruningStepsContext *context, Bitmapset *step_nullkeys, List *prefix) { + Assert(step_nullkeys == NULL || + context->rel->part_scheme->strategy == PARTITION_STRATEGY_HASH); + /* Quick exit if there are no values to prefix with. */ if (list_length(prefix) == 0) { @@ -2261,7 +2323,7 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context, ListCell *next_start; /* - * For each clause with cur_keyno, adds its expr and cmpfn to + * For each clause with cur_keyno, add its expr and cmpfn to * step_exprs and step_cmpfns, respectively, and recurse after setting * next_start to the ListCell of the first clause for the next * partition key. @@ -2278,23 +2340,19 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context, for_each_cell(lc, prefix, start) { List *moresteps; + List *step_exprs1, + *step_cmpfns1; pc = lfirst(lc); if (pc->keyno == cur_keyno) { - /* clean up before starting a new recursion cycle. */ - if (cur_keyno == 0) - { - list_free(step_exprs); - list_free(step_cmpfns); - step_exprs = list_make1(pc->expr); - step_cmpfns = list_make1_oid(pc->cmpfn); - } - else - { - step_exprs = lappend(step_exprs, pc->expr); - step_cmpfns = lappend_oid(step_cmpfns, pc->cmpfn); - } + /* Leave the original step_exprs unmodified. */ + step_exprs1 = list_copy(step_exprs); + step_exprs1 = lappend(step_exprs1, pc->expr); + + /* Leave the original step_cmpfns unmodified. */ + step_cmpfns1 = list_copy(step_cmpfns); + step_cmpfns1 = lappend_oid(step_cmpfns1, pc->cmpfn); } else { @@ -2311,9 +2369,12 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context, step_nullkeys, prefix, next_start, - step_exprs, - step_cmpfns); + step_exprs1, + step_cmpfns1); result = list_concat(result, moresteps); + + list_free(step_exprs1); + list_free(step_cmpfns1); } } else @@ -2321,9 +2382,23 @@ get_steps_using_prefix_recurse(GeneratePruningStepsContext *context, /* * End the current recursion cycle and start generating steps, one for * each clause with cur_keyno, which is all clauses from here onward - * till the end of the list. + * till the end of the list. Note that for hash partitioning, + * step_nullkeys is allowed to be non-empty, in which case step_exprs + * would only contain expressions for the earlier partition keys that + * are not specified in step_nullkeys. */ - Assert(list_length(step_exprs) == cur_keyno); + Assert(list_length(step_exprs) == cur_keyno || + !bms_is_empty(step_nullkeys)); + /* + * Note also that for hash partitioning, each partition key should + * have either equality clauses or an IS NULL clause, so if a + * partition key doesn't have an expression, it would be specified + * in step_nullkeys. + */ + Assert(context->rel->part_scheme->strategy + != PARTITION_STRATEGY_HASH || + list_length(step_exprs) + 2 + bms_num_members(step_nullkeys) == + context->rel->part_scheme->partnatts); for_each_cell(lc, prefix, start) { PartClauseInfo *pc = lfirst(lc); diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 4315e8e0a3..687cf8c5f4 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -3671,3 +3671,95 @@ explain (costs off) update listp1 set a = 1 where a = 2; reset constraint_exclusion; reset enable_partition_pruning; drop table listp; +-- +-- Check that gen_prune_steps_from_opexps() works well for various cases of +-- clauses for different partition keys +-- +create table rp_prefix_test1 (a int, b varchar) partition by range(a, b); +create table rp_prefix_test1_p1 partition of rp_prefix_test1 for values from (1, 'a') to (1, 'b'); +create table rp_prefix_test1_p2 partition of rp_prefix_test1 for values from (2, 'a') to (2, 'b'); +-- Don't call get_steps_using_prefix() with the last partition key b plus +-- an empty prefix +explain (costs off) select * from rp_prefix_test1 where a <= 1 and b = 'a'; + QUERY PLAN +-------------------------------------------------- + Seq Scan on rp_prefix_test1_p1 rp_prefix_test1 + Filter: ((a <= 1) AND ((b)::text = 'a'::text)) +(2 rows) + +create table rp_prefix_test2 (a int, b int, c int) partition by range(a, b, c); +create table rp_prefix_test2_p1 partition of rp_prefix_test2 for values from (1, 1, 0) to (1, 1, 10); +create table rp_prefix_test2_p2 partition of rp_prefix_test2 for values from (2, 2, 0) to (2, 2, 10); +-- Don't call get_steps_using_prefix() with the last partition key c plus +-- an invalid prefix (ie, b = 1) +explain (costs off) select * from rp_prefix_test2 where a <= 1 and b = 1 and c >= 0; + QUERY PLAN +------------------------------------------------ + Seq Scan on rp_prefix_test2_p1 rp_prefix_test2 + Filter: ((a <= 1) AND (c >= 0) AND (b = 1)) +(2 rows) + +create table rp_prefix_test3 (a int, b int, c int, d int) partition by range(a, b, c, d); +create table rp_prefix_test3_p1 partition of rp_prefix_test3 for values from (1, 1, 1, 0) to (1, 1, 1, 10); +create table rp_prefix_test3_p2 partition of rp_prefix_test3 for values from (2, 2, 2, 0) to (2, 2, 2, 10); +-- Test that get_steps_using_prefix() handles a prefix that contains multiple +-- 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; + QUERY PLAN +-------------------------------------------------------------------------- + Seq Scan on rp_prefix_test3_p2 rp_prefix_test3 + Filter: ((a >= 1) AND (b >= 1) AND (b >= 2) AND (c >= 2) AND (d >= 0)) +(2 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); +-- 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; + 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)) +(2 rows) + +drop table rp_prefix_test1; +drop table rp_prefix_test2; +drop table rp_prefix_test3; +drop table hp_prefix_test; +-- +-- Check that gen_partprune_steps() detects self-contradiction from clauses +-- regardless of the order of the clauses (Here we use a custom operator to +-- prevent the equivclass.c machinery from reordering the clauses) +-- +create operator === ( + leftarg = int4, + rightarg = int4, + procedure = int4eq, + commutator = ===, + hashes +); +create operator class part_test_int4_ops2 +for type int4 +using hash as +operator 1 ===, +function 2 part_hashint4_noop(int4, int8); +create table hp_contradict_test (a int, b int) partition by hash (a part_test_int4_ops2, b part_test_int4_ops2); +create table hp_contradict_test_p1 partition of hp_contradict_test for values with (modulus 2, remainder 0); +create table hp_contradict_test_p2 partition of hp_contradict_test for values with (modulus 2, remainder 1); +explain (costs off) select * from hp_contradict_test where a is null and a === 1 and b === 1; + QUERY PLAN +-------------------------- + Result + One-Time Filter: false +(2 rows) + +explain (costs off) select * from hp_contradict_test where a === 1 and b === 1 and a is null; + QUERY PLAN +-------------------------- + Result + One-Time Filter: false +(2 rows) + +drop table hp_contradict_test; +drop operator class part_test_int4_ops2 using hash; +drop operator ===(int4, int4); diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 6658455a74..93ef9dc1f3 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -1050,3 +1050,74 @@ reset constraint_exclusion; reset enable_partition_pruning; drop table listp; + +-- +-- Check that gen_prune_steps_from_opexps() works well for various cases of +-- clauses for different partition keys +-- + +create table rp_prefix_test1 (a int, b varchar) partition by range(a, b); +create table rp_prefix_test1_p1 partition of rp_prefix_test1 for values from (1, 'a') to (1, 'b'); +create table rp_prefix_test1_p2 partition of rp_prefix_test1 for values from (2, 'a') to (2, 'b'); + +-- Don't call get_steps_using_prefix() with the last partition key b plus +-- an empty prefix +explain (costs off) select * from rp_prefix_test1 where a <= 1 and b = 'a'; + +create table rp_prefix_test2 (a int, b int, c int) partition by range(a, b, c); +create table rp_prefix_test2_p1 partition of rp_prefix_test2 for values from (1, 1, 0) to (1, 1, 10); +create table rp_prefix_test2_p2 partition of rp_prefix_test2 for values from (2, 2, 0) to (2, 2, 10); + +-- Don't call get_steps_using_prefix() with the last partition key c plus +-- an invalid prefix (ie, b = 1) +explain (costs off) select * from rp_prefix_test2 where a <= 1 and b = 1 and c >= 0; + +create table rp_prefix_test3 (a int, b int, c int, d int) partition by range(a, b, c, d); +create table rp_prefix_test3_p1 partition of rp_prefix_test3 for values from (1, 1, 1, 0) to (1, 1, 1, 10); +create table rp_prefix_test3_p2 partition of rp_prefix_test3 for values from (2, 2, 2, 0) to (2, 2, 2, 10); + +-- Test that get_steps_using_prefix() handles a prefix that contains multiple +-- 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; + +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; +drop table hp_prefix_test; + +-- +-- Check that gen_partprune_steps() detects self-contradiction from clauses +-- regardless of the order of the clauses (Here we use a custom operator to +-- prevent the equivclass.c machinery from reordering the clauses) +-- + +create operator === ( + leftarg = int4, + rightarg = int4, + procedure = int4eq, + commutator = ===, + hashes +); +create operator class part_test_int4_ops2 +for type int4 +using hash as +operator 1 ===, +function 2 part_hashint4_noop(int4, int8); + +create table hp_contradict_test (a int, b int) partition by hash (a part_test_int4_ops2, b part_test_int4_ops2); +create table hp_contradict_test_p1 partition of hp_contradict_test for values with (modulus 2, remainder 0); +create table hp_contradict_test_p2 partition of hp_contradict_test for values with (modulus 2, remainder 1); + +explain (costs off) select * from hp_contradict_test where a is null and a === 1 and b === 1; +explain (costs off) select * from hp_contradict_test where a === 1 and b === 1 and a is null; + +drop table hp_contradict_test; +drop operator class part_test_int4_ops2 using hash; +drop operator ===(int4, int4);