From: David Rowley Date: Fri, 12 Jul 2019 07:12:38 +0000 (+1200) Subject: Fix RANGE partition pruning with multiple boolean partition keys X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cfde23493938b543d281c509527f0f5b16fca9ff;p=postgresql Fix RANGE partition pruning with multiple boolean partition keys match_clause_to_partition_key incorrectly would return PARTCLAUSE_UNSUPPORTED if a bool qual could not be matched to the current partition key. This was a problem, as it causes the calling function to discard the qual and not try to match it to any other partition key. If there was another partition key which did match this qual, then the qual would not be checked again and we could fail to prune some partitions. The worst this could do was to cause partitions not to be pruned when they could have been, so there was no danger of incorrect query results here. Fix this by changing match_boolean_partition_clause to have it return a PartClauseMatchStatus rather than a boolean value. This allows it to communicate if the qual is unsupported or if it just does not match this particular partition key, previously these two cases were treated the same. Now, if match_clause_to_partition_key is unable to match the qual to any other qual type then we can simply return the value from the match_boolean_partition_clause call so that the calling function properly treats the qual as either unmatched or unsupported. Reported-by: Rares Salcudean Reviewed-by: Amit Langote Backpatch-through: 11 where partition pruning was introduced Discussion: https://postgr.es/m/CAHp_FN2xwEznH6oyS0hNTuUUZKp5PvegcVv=Co6nBXJ+mC7Y5w@mail.gmail.com --- diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index 5982af4de1..e71a21c0a7 100644 --- a/src/backend/partitioning/partprune.c +++ b/src/backend/partitioning/partprune.c @@ -194,8 +194,10 @@ static PruneStepResult *perform_pruning_base_step(PartitionPruneContext *context static PruneStepResult *perform_pruning_combine_step(PartitionPruneContext *context, PartitionPruneStepCombine *cstep, PruneStepResult **step_results); -static bool match_boolean_partition_clause(Oid partopfamily, Expr *clause, - Expr *partkey, Expr **outconst); +static PartClauseMatchStatus match_boolean_partition_clause(Oid partopfamily, + Expr *clause, + Expr *partkey, + Expr **outconst); static void partkey_datum_from_expr(PartitionPruneContext *context, Expr *expr, int stateidx, Datum *value, bool *isnull); @@ -1623,6 +1625,7 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context, bool *clause_is_not_null, PartClauseInfo **pc, List **clause_steps) { + PartClauseMatchStatus boolmatchstatus; PartitionScheme part_scheme = context->rel->part_scheme; Oid partopfamily = part_scheme->partopfamily[partkeyidx], partcoll = part_scheme->partcollation[partkeyidx]; @@ -1631,7 +1634,10 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context, /* * Recognize specially shaped clauses that match a Boolean partition key. */ - if (match_boolean_partition_clause(partopfamily, clause, partkey, &expr)) + boolmatchstatus = match_boolean_partition_clause(partopfamily, clause, + partkey, &expr); + + if (boolmatchstatus == PARTCLAUSE_MATCH_CLAUSE) { PartClauseInfo *partclause; @@ -2147,7 +2153,21 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context, return PARTCLAUSE_MATCH_NULLNESS; } - return PARTCLAUSE_UNSUPPORTED; + /* + * If we get here then the return value depends on the result of the + * match_boolean_partition_clause call above. If the call returned + * PARTCLAUSE_UNSUPPORTED then we're either not dealing with a bool qual + * or the bool qual is not suitable for pruning. Since the qual didn't + * match up to any of the other qual types supported here, then trying to + * match it against any other partition key is a waste of time, so just + * return PARTCLAUSE_UNSUPPORTED. If the qual just couldn't be matched to + * this partition key, then it may match another, so return + * PARTCLAUSE_NOMATCH. The only other value that + * match_boolean_partition_clause can return is PARTCLAUSE_MATCH_CLAUSE, + * and since that value was already dealt with above, then we can just + * return boolmatchstatus. + */ + return boolmatchstatus; } /* @@ -3395,11 +3415,15 @@ perform_pruning_combine_step(PartitionPruneContext *context, /* * match_boolean_partition_clause * - * Sets *outconst to a Const containing true or false value and returns true if - * we're able to match the clause to the partition key as specially-shaped - * Boolean clause. Returns false otherwise with *outconst set to NULL. + * If we're able to match the clause to the partition key as specially-shaped + * boolean clause, set *outconst to a Const containing a true or false value + * and return PARTCLAUSE_MATCH_CLAUSE. Returns PARTCLAUSE_UNSUPPORTED if the + * clause is not a boolean clause or if the boolean clause is unsuitable for + * partition pruning. Returns PARTCLAUSE_NOMATCH if it's a bool quals but + * just does not match this partition key. *outconst is set to NULL in the + * latter two cases. */ -static bool +static PartClauseMatchStatus match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey, Expr **outconst) { @@ -3408,7 +3432,7 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey, *outconst = NULL; if (!IsBooleanOpfamily(partopfamily)) - return false; + return PARTCLAUSE_UNSUPPORTED; if (IsA(clause, BooleanTest)) { @@ -3417,7 +3441,7 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey, /* Only IS [NOT] TRUE/FALSE are any good to us */ if (btest->booltesttype == IS_UNKNOWN || btest->booltesttype == IS_NOT_UNKNOWN) - return false; + return PARTCLAUSE_UNSUPPORTED; leftop = btest->arg; if (IsA(leftop, RelabelType)) @@ -3430,7 +3454,7 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey, : (Expr *) makeBoolConst(false, false); if (*outconst) - return true; + return PARTCLAUSE_MATCH_CLAUSE; } else { @@ -3450,10 +3474,10 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey, *outconst = (Expr *) makeBoolConst(false, false); if (*outconst) - return true; + return PARTCLAUSE_MATCH_CLAUSE; } - return false; + return PARTCLAUSE_NOMATCH; } /* diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 841bd8bc67..2eecb1744b 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -1086,6 +1086,19 @@ explain (costs off) select * from boolpart where a is not unknown; Filter: (a IS NOT UNKNOWN) (7 rows) +create table boolrangep (a bool, b bool, c int) partition by range (a,b,c); +create table boolrangep_tf partition of boolrangep for values from ('true', 'false', 0) to ('true', 'false', 100); +create table boolrangep_ft partition of boolrangep for values from ('false', 'true', 0) to ('false', 'true', 100); +create table boolrangep_ff1 partition of boolrangep for values from ('false', 'false', 0) to ('false', 'false', 50); +create table boolrangep_ff2 partition of boolrangep for values from ('false', 'false', 50) to ('false', 'false', 100); +-- try a more complex case that's been known to trip up pruning in the past +explain (costs off) select * from boolrangep where not a and not b and c = 25; + QUERY PLAN +---------------------------------------------- + Seq Scan on boolrangep_ff1 + Filter: ((NOT a) AND (NOT b) AND (c = 25)) +(2 rows) + -- test scalar-to-array operators create table coercepart (a varchar) partition by list (a); create table coercepart_ab partition of coercepart for values in ('ab'); @@ -1420,7 +1433,7 @@ explain (costs off) select * from rparted_by_int2 where a > 100000000000000; Filter: (a > '100000000000000'::bigint) (2 rows) -drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2; +drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2; -- -- Test Partition pruning for HASH partitioning -- diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 071e28dce8..7bb4e2fffc 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -159,6 +159,15 @@ explain (costs off) select * from boolpart where a is not true and a is not fals explain (costs off) select * from boolpart where a is unknown; explain (costs off) select * from boolpart where a is not unknown; +create table boolrangep (a bool, b bool, c int) partition by range (a,b,c); +create table boolrangep_tf partition of boolrangep for values from ('true', 'false', 0) to ('true', 'false', 100); +create table boolrangep_ft partition of boolrangep for values from ('false', 'true', 0) to ('false', 'true', 100); +create table boolrangep_ff1 partition of boolrangep for values from ('false', 'false', 0) to ('false', 'false', 50); +create table boolrangep_ff2 partition of boolrangep for values from ('false', 'false', 50) to ('false', 'false', 100); + +-- try a more complex case that's been known to trip up pruning in the past +explain (costs off) select * from boolrangep where not a and not b and c = 25; + -- test scalar-to-array operators create table coercepart (a varchar) partition by list (a); create table coercepart_ab partition of coercepart for values in ('ab'); @@ -264,7 +273,7 @@ create table rparted_by_int2_maxvalue partition of rparted_by_int2 for values fr -- all partitions but rparted_by_int2_maxvalue pruned explain (costs off) select * from rparted_by_int2 where a > 100000000000000; -drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2; +drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2; -- -- Test Partition pruning for HASH partitioning