From c33d16f911b8e76b8b9a1eb6da756b69ba45bb48 Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 10 May 2019 13:52:59 +0900 Subject: [PATCH v3 2/3] Fix bugs in pruning with composite range partition keys * Force using equality (= operator) semantics when comparing clause expressions against partition bounds if the expression for the last matched key cannot be evaluated in a given pruning context; for example, if the expression is stable and pruning is invoked by the planner * Ignore keys after a given key for which we detected a clause containing a non-inclusive operator --- src/backend/partitioning/partprune.c | 88 +++++++++++++-------------- src/test/regress/expected/partition_prune.out | 18 +++++- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index 59f34f5cac..92806ab9e1 100644 --- a/src/backend/partitioning/partprune.c +++ b/src/backend/partitioning/partprune.c @@ -1178,17 +1178,24 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme, List *opsteps = NIL; List *btree_clauses[BTMaxStrategyNumber + 1], *hash_clauses[HTMaxStrategyNumber + 1]; - bool need_next_less, - need_next_eq, - need_next_greater; + bool consider_next_key; int i; + consider_next_key = true; memset(btree_clauses, 0, sizeof(btree_clauses)); memset(hash_clauses, 0, sizeof(hash_clauses)); for (i = 0; i < part_scheme->partnatts; i++) { List *clauselist = keyclauses[i]; - bool consider_next_key = true; + + /* + * If the previous key was found to be compared using such an operator + * that comparisons of subsequent key columns against partition bounds + * that occurs during partition pruning would be redundant, then don't + * bother generating pruning steps for them. + */ + if (!consider_next_key) + break; /* * To be useful for pruning, we must have clauses for a prefix of @@ -1208,7 +1215,6 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme, clauselist == NIL && !bms_is_member(i, nullkeys)) return NULL; - need_next_eq = need_next_less = need_next_greater = true; foreach(lc, clauselist) { PartClauseInfo *pc = (PartClauseInfo *) lfirst(lc); @@ -1224,13 +1230,13 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme, &lefttype, &righttype); + switch (part_scheme->strategy) { case PARTITION_STRATEGY_LIST: case PARTITION_STRATEGY_RANGE: { PartClauseInfo *last = NULL; - bool inclusive = false; /* * Add this clause to the list of clauses to be used @@ -1248,35 +1254,14 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme, lappend(btree_clauses[pc->op_strategy], pc); /* - * We may not need the next clause if they're of - * certain strategy. + * We won't need the clause for the next key if the + * clause for the current key contains a non-inclusive + * operator. */ - switch (pc->op_strategy) - { - case BTLessEqualStrategyNumber: - inclusive = true; - /* fall through */ - case BTLessStrategyNumber: - if (!inclusive) - need_next_eq = need_next_less = false; - break; - case BTEqualStrategyNumber: - /* always accept clauses for the next key. */ - break; - case BTGreaterEqualStrategyNumber: - inclusive = true; - /* fall through */ - case BTGreaterStrategyNumber: - if (!inclusive) - need_next_eq = need_next_greater = false; - break; - } + if (pc->op_strategy == BTLessStrategyNumber || + pc->op_strategy == BTGreaterStrategyNumber) + consider_next_key = false; - /* We may want to change our mind. */ - if (consider_next_key) - consider_next_key = (need_next_eq || - need_next_less || - need_next_greater); break; } @@ -1293,13 +1278,6 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme, break; } } - - /* - * If we've decided that clauses for subsequent partition keys - * wouldn't be useful for pruning, don't search any further. - */ - if (!consider_next_key) - break; } /* @@ -2763,7 +2741,7 @@ get_matching_range_bounds(PartitionPruneContext *context, /* * Look for the greatest bound that is < or <= lookup value and - * set minoff to its offset. + * set maxoff to its offset. */ off = partition_range_datum_bsearch(partsupfunc, partcollation, @@ -3133,11 +3111,29 @@ perform_pruning_base_step(PartitionPruneContext *context, opstep->nullkeys); case PARTITION_STRATEGY_RANGE: - return get_matching_range_bounds(context, - opstep->opstrategy, - values, nvalues, - partsupfunc, - opstep->nullkeys); + { + int opstrategy = opstep->opstrategy; + + /* + * If we got values for only a subset (prefix) of the + * expressions in opstep->exprs, adjust the strategy to + * be used for pruning. Originally, it would be the strategy + * of the operator comparing the *last* expression against the + * corresponding partition key, but it would be wrong to use + * the same strategy now that we'll be comparing the values of + * only a prefix of expressions against the partition bounds. + * Use equality strategy, because all expressions in the + * prefix must have been extracted from clauses containing + * equality (=) or inclusive operators (<=, >=) anyway. + */ + if (nvalues < list_length(opstep->exprs)) + opstrategy = BTEqualStrategyNumber; + return get_matching_range_bounds(context, + opstrategy, + values, nvalues, + partsupfunc, + opstep->nullkeys); + } default: elog(ERROR, "unexpected partition strategy: %d", diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 80dc700323..3edbbc6dbb 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -933,9 +933,11 @@ explain (analyze, costs off, summary off, timing off) select * from mc3p where a -> Result (actual rows=1 loops=1) -> Seq Scan on mc3p0 (actual rows=0 loops=1) Filter: ((a = 1) AND (abs(b) < $0)) + -> Seq Scan on mc3p1 (actual rows=0 loops=1) + Filter: ((a = 1) AND (abs(b) < $0)) -> Seq Scan on mc3p_default (actual rows=0 loops=1) Filter: ((a = 1) AND (abs(b) < $0)) -(7 rows) +(9 rows) explain (analyze, costs off, summary off, timing off) select * from mc3p where a < (select 1) and abs(b) = 1; QUERY PLAN @@ -947,9 +949,21 @@ explain (analyze, costs off, summary off, timing off) select * from mc3p where a Filter: ((a < $0) AND (abs(b) = 1)) -> Seq Scan on mc3p1 (never executed) Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p2 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p3 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p4 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p5 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p6 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) + -> Seq Scan on mc3p7 (never executed) + Filter: ((a < $0) AND (abs(b) = 1)) -> Seq Scan on mc3p_default (actual rows=0 loops=1) Filter: ((a < $0) AND (abs(b) = 1)) -(9 rows) +(21 rows) -- a simpler multi-column keys case create table mc2p (a int, b int) partition by range (a, b); -- 2.11.0