Fix bogus logic for combining range-partitioned columns during pruning.

gen_prune_steps_from_opexps's notion of how to do this was overly
complicated and underly correct.

Per discussion of a report from Alan Jackson (though this fixes only one
aspect of that problem).  Back-patch to v11 where this code came in.

Amit Langote

Discussion: https://postgr.es/m/FAD28A83-AC73-489E-A058-2681FA31D648@tvsquared.com

diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index c12ca703e7be35bde9d899c5814c4284c3865c81..ff3caf14c0c81fc23cf46eeb7f8fb77d3469d8c4 100644 (file)
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -1209,9 +1209,6 @@ 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;
        int                     i;
 
        memset(btree_clauses, 0, sizeof(btree_clauses));
@@ -1222,9 +1219,8 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme,
                bool            consider_next_key = true;
 
                /*
-                * To be useful for pruning, we must have clauses for a prefix of
-                * partition keys in the case of range partitioning.  So, ignore
-                * clauses for keys after this one.
+                * For range partitioning, if we have no clauses for the current key,
+                * we can't consider any later keys either, so we can stop here.
                 */
                if (part_scheme->strategy == PARTITION_STRATEGY_RANGE &&
                        clauselist == NIL)
@@ -1239,7 +1235,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);
@@ -1261,7 +1256,6 @@ gen_prune_steps_from_opexps(PartitionScheme part_scheme,
                                case PARTITION_STRATEGY_RANGE:
                                        {
                                                PartClauseInfo *last = NULL;
-                                               bool            inclusive = false;
 
                                                /*
                                                 * Add this clause to the list of clauses to be used
@@ -1279,35 +1273,13 @@ 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 can't consider subsequent partition keys 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;
-                                               }
-
-                                               /* We may want to change our mind. */
-                                               if (consider_next_key)
-                                                       consider_next_key = (need_next_eq ||
-                                                                                                need_next_less ||
-                                                                                                need_next_greater);
+                                               if (pc->op_strategy == BTLessStrategyNumber ||
+                                                       pc->op_strategy == BTGreaterStrategyNumber)
+                                                       consider_next_key = false;
                                                break;
                                        }
 
@@ -2847,7 +2819,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,

diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index f1e192c68c90fb05a26b0491efed217368d6106f..723fc70f732d9f44262e8fdce520f78803196d2f 100644 (file)
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -3123,6 +3123,33 @@ select * from stable_qual_pruning
 (4 rows)
 
 drop table stable_qual_pruning;
+--
+-- Check that pruning with composite range partitioning works correctly when
+-- it must ignore clauses for trailing keys once it has seen a clause with
+-- non-inclusive operator for an earlier key
+--
+create table mc3p (a int, b int, c int) partition by range (a, abs(b), c);
+create table mc3p0 partition of mc3p
+  for values from (0, 0, 0) to (0, maxvalue, maxvalue);
+create table mc3p1 partition of mc3p
+  for values from (1, 1, 1) to (2, minvalue, minvalue);
+create table mc3p2 partition of mc3p
+  for values from (2, minvalue, minvalue) to (3, maxvalue, maxvalue);
+insert into mc3p values (0, 1, 1), (1, 1, 1), (2, 1, 1);
+explain (analyze, costs off, summary off, timing off)
+select * from mc3p where a < 3 and abs(b) = 1;
+                   QUERY PLAN                    
+-------------------------------------------------
+ Append (actual rows=3 loops=1)
+   ->  Seq Scan on mc3p0 (actual rows=1 loops=1)
+         Filter: ((a < 3) AND (abs(b) = 1))
+   ->  Seq Scan on mc3p1 (actual rows=1 loops=1)
+         Filter: ((a < 3) AND (abs(b) = 1))
+   ->  Seq Scan on mc3p2 (actual rows=1 loops=1)
+         Filter: ((a < 3) AND (abs(b) = 1))
+(7 rows)
+
+drop table mc3p;
 -- Ensure runtime pruning works with initplans params with boolean types
 create table boolvalues (value bool not null);
 insert into boolvalues values('t'),('f');

diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index ada9e6ae0ef5c3064e8765cab846dd292c93e140..2373bd8781d541de5385f7ec3456703995185603 100644 (file)
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -792,6 +792,25 @@ select * from stable_qual_pruning
 
 drop table stable_qual_pruning;
 
+--
+-- Check that pruning with composite range partitioning works correctly when
+-- it must ignore clauses for trailing keys once it has seen a clause with
+-- non-inclusive operator for an earlier key
+--
+create table mc3p (a int, b int, c int) partition by range (a, abs(b), c);
+create table mc3p0 partition of mc3p
+  for values from (0, 0, 0) to (0, maxvalue, maxvalue);
+create table mc3p1 partition of mc3p
+  for values from (1, 1, 1) to (2, minvalue, minvalue);
+create table mc3p2 partition of mc3p
+  for values from (2, minvalue, minvalue) to (3, maxvalue, maxvalue);
+insert into mc3p values (0, 1, 1), (1, 1, 1), (2, 1, 1);
+
+explain (analyze, costs off, summary off, timing off)
+select * from mc3p where a < 3 and abs(b) = 1;
+
+drop table mc3p;
+
 -- Ensure runtime pruning works with initplans params with boolean types
 create table boolvalues (value bool not null);
 insert into boolvalues values('t'),('f');