Fix incorrect pruning of NULL partition for boolean IS NOT clauses
authorDavid Rowley <drowley@postgresql.org>
Mon, 19 Feb 2024 23:49:37 +0000 (12:49 +1300)
committerDavid Rowley <drowley@postgresql.org>
Mon, 19 Feb 2024 23:49:37 +0000 (12:49 +1300)
Partition pruning wrongly assumed that, for a table partitioned on a
boolean column, a clause in the form "boolcol IS NOT false" and "boolcol
IS NOT true" could be inverted to correspondingly become "boolcol IS true"
and "boolcol IS false".  These are not equivalent as the NOT version
matches the opposite boolean value *and* NULLs.  This incorrect assumption
meant that partition pruning pruned away partitions that could contain
NULL values.

Here we fix this by correctly not pruning partitions which could store
NULLs.

To be affected by this, the table must be partitioned by a NULLable boolean
column and queries would have to contain "boolcol IS NOT false" or "boolcol
IS NOT true".  This could result in queries filtering out NULL values
with a LIST partitioned table and "ERROR:  invalid strategy number 0"
for RANGE and HASH partitioned tables.

Reported-by: Alexander Lakhin
Bug: #18344
Discussion: https://postgr.es/m/18344-8d3f00bada6d09c6@postgresql.org
Backpatch-through: 12

src/backend/partitioning/partprune.c
src/test/regress/expected/partition_prune.out
src/test/regress/sql/partition_prune.sql

index 6b635e8ad16511efae581afa6482df09499c77b0..ccb9a579b30ef1ceb7c51935c9fac001d26b228e 100644 (file)
@@ -1810,11 +1810,63 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
    {
        PartClauseInfo *partclause;
 
+       /*
+        * For bool tests in the form of partkey IS NOT true and IS NOT false,
+        * we invert these clauses.  Effectively, "partkey IS NOT true"
+        * becomes "partkey IS false OR partkey IS NULL".  We do this by
+        * building an OR BoolExpr and forming a clause just like that and
+        * punt it off to gen_partprune_steps_internal() to generate pruning
+        * steps.
+        */
+       if (noteq)
+       {
+           List       *new_clauses;
+           List       *or_clause;
+           BooleanTest *new_booltest = (BooleanTest *) copyObject(clause);
+           NullTest   *nulltest;
+
+           /* We expect 'noteq' to only be set to true for BooleanTests */
+           Assert(IsA(clause, BooleanTest));
+
+           /* reverse the bool test */
+           if (new_booltest->booltesttype == IS_NOT_TRUE)
+               new_booltest->booltesttype = IS_FALSE;
+           else if (new_booltest->booltesttype == IS_NOT_FALSE)
+               new_booltest->booltesttype = IS_TRUE;
+           else
+           {
+               /*
+                * We only expect match_boolean_partition_clause to match for
+                * IS_NOT_TRUE and IS_NOT_FALSE.  IS_NOT_UNKNOWN is not
+                * supported.
+                */
+               Assert(false);
+           }
+
+           nulltest = makeNode(NullTest);
+           nulltest->arg = copyObject(partkey);
+           nulltest->nulltesttype = IS_NULL;
+           nulltest->argisrow = false;
+           nulltest->location = -1;
+
+           new_clauses = list_make2(new_booltest, nulltest);
+           or_clause = list_make1(makeBoolExpr(OR_EXPR, new_clauses, -1));
+
+           /* Finally, generate steps */
+           *clause_steps = gen_partprune_steps_internal(context, or_clause);
+
+           if (context->contradictory)
+               return PARTCLAUSE_MATCH_CONTRADICT; /* shouldn't happen */
+           else if (*clause_steps == NIL)
+               return PARTCLAUSE_UNSUPPORTED;  /* step generation failed */
+           return PARTCLAUSE_MATCH_STEPS;
+       }
+
        partclause = (PartClauseInfo *) palloc(sizeof(PartClauseInfo));
        partclause->keyno = partkeyidx;
        /* Do pruning with the Boolean equality operator. */
        partclause->opno = BooleanEqualOperator;
-       partclause->op_is_ne = noteq;
+       partclause->op_is_ne = false;
        partclause->expr = expr;
        /* We know that expr is of Boolean type. */
        partclause->cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
@@ -2358,7 +2410,7 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
  * For LIST and RANGE partitioned tables, callers must ensure that
  * step_nullkeys is NULL, and that prefix contains at least one clause for
  * each of the partition keys prior to the key that 'step_lastexpr' and
- * 'step_lastcmpfn'belong to.
+ * 'step_lastcmpfn' belong to.
  *
  * For HASH partitioned tables, callers must ensure that 'prefix' contains at
  * least one clause for each of the partition keys apart from the final key
index 9a4c48c0556fca816b752eaa9d2af6846a8dd72b..b41950d923b298bc2338709838c35927da0bfb2a 100644 (file)
@@ -1169,6 +1169,57 @@ select * from boolpart where a is not unknown;
  t
 (2 rows)
 
+-- try some other permutations with a NULL partition instead of a DEFAULT
+delete from boolpart where a is null;
+create table boolpart_null partition of boolpart for values in (null);
+insert into boolpart values(null);
+explain (costs off) select * from boolpart where a is not true;
+                 QUERY PLAN                 
+--------------------------------------------
+ Append
+   ->  Seq Scan on boolpart_f boolpart_1
+         Filter: (a IS NOT TRUE)
+   ->  Seq Scan on boolpart_null boolpart_2
+         Filter: (a IS NOT TRUE)
+(5 rows)
+
+explain (costs off) select * from boolpart where a is not true and a is not false;
+                    QUERY PLAN                    
+--------------------------------------------------
+ Seq Scan on boolpart_null boolpart
+   Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
+(2 rows)
+
+explain (costs off) select * from boolpart where a is not false;
+                 QUERY PLAN                 
+--------------------------------------------
+ Append
+   ->  Seq Scan on boolpart_t boolpart_1
+         Filter: (a IS NOT FALSE)
+   ->  Seq Scan on boolpart_null boolpart_2
+         Filter: (a IS NOT FALSE)
+(5 rows)
+
+select * from boolpart where a is not true;
+ a 
+---
+ f
+(2 rows)
+
+select * from boolpart where a is not true and a is not false;
+ a 
+---
+(1 row)
+
+select * from boolpart where a is not false;
+ a 
+---
+ t
+(2 rows)
+
 -- inverse boolean partitioning - a seemingly unlikely design, but we've got
 -- code for it, so we'd better test it.
 create table iboolpart (a bool) partition by list ((not a));
@@ -1315,11 +1366,37 @@ select * from iboolpart where a is not unknown;
  f
 (2 rows)
 
+-- Try some other permutations with a NULL partition instead of a DEFAULT
+delete from iboolpart where a is null;
+create table iboolpart_null partition of iboolpart for values in (null);
+insert into iboolpart values(null);
+-- Pruning shouldn't take place for these.  Just check the result is correct
+select * from iboolpart where a is not true;
+ a 
+---
+ f
+(2 rows)
+
+select * from iboolpart where a is not true and a is not false;
+ a 
+---
+(1 row)
+
+select * from iboolpart where a is not false;
+ a 
+---
+ t
+(2 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);
+create table boolrangep_null partition of boolrangep default;
 -- 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                  
@@ -1328,6 +1405,32 @@ explain (costs off)  select * from boolrangep where not a and not b and c = 25;
    Filter: ((NOT a) AND (NOT b) AND (c = 25))
 (2 rows)
 
+-- ensure we prune boolrangep_tf
+explain (costs off)  select * from boolrangep where a is not true and not b and c = 25;
+                         QUERY PLAN                         
+------------------------------------------------------------
+ Append
+   ->  Seq Scan on boolrangep_ff1 boolrangep_1
+         Filter: ((a IS NOT TRUE) AND (NOT b) AND (c = 25))
+   ->  Seq Scan on boolrangep_ff2 boolrangep_2
+         Filter: ((a IS NOT TRUE) AND (NOT b) AND (c = 25))
+   ->  Seq Scan on boolrangep_ft boolrangep_3
+         Filter: ((a IS NOT TRUE) AND (NOT b) AND (c = 25))
+   ->  Seq Scan on boolrangep_null boolrangep_4
+         Filter: ((a IS NOT TRUE) AND (NOT b) AND (c = 25))
+(9 rows)
+
+-- ensure we prune everything apart from boolrangep_tf and boolrangep_null
+explain (costs off)  select * from boolrangep where a is not false and not b and c = 25;
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Append
+   ->  Seq Scan on boolrangep_tf boolrangep_1
+         Filter: ((a IS NOT FALSE) AND (NOT b) AND (c = 25))
+   ->  Seq Scan on boolrangep_null boolrangep_2
+         Filter: ((a IS NOT FALSE) AND (NOT b) AND (c = 25))
+(5 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');
index 7bf3920827ff2fedff87dcb7ec14a4b43bef74f2..7ba6a9ff3701507463dab19548550ab217aa1ed2 100644 (file)
@@ -178,6 +178,19 @@ select * from boolpart where a is not true and a is not false;
 select * from boolpart where a is unknown;
 select * from boolpart where a is not unknown;
 
+-- try some other permutations with a NULL partition instead of a DEFAULT
+delete from boolpart where a is null;
+create table boolpart_null partition of boolpart for values in (null);
+insert into boolpart values(null);
+
+explain (costs off) select * from boolpart where a is not true;
+explain (costs off) select * from boolpart where a is not true and a is not false;
+explain (costs off) select * from boolpart where a is not false;
+
+select * from boolpart where a is not true;
+select * from boolpart where a is not true and a is not false;
+select * from boolpart where a is not false;
+
 -- inverse boolean partitioning - a seemingly unlikely design, but we've got
 -- code for it, so we'd better test it.
 create table iboolpart (a bool) partition by list ((not a));
@@ -204,15 +217,32 @@ select * from iboolpart where a is not true and a is not false;
 select * from iboolpart where a is unknown;
 select * from iboolpart where a is not unknown;
 
+-- Try some other permutations with a NULL partition instead of a DEFAULT
+delete from iboolpart where a is null;
+create table iboolpart_null partition of iboolpart for values in (null);
+insert into iboolpart values(null);
+
+-- Pruning shouldn't take place for these.  Just check the result is correct
+select * from iboolpart where a is not true;
+select * from iboolpart where a is not true and a is not false;
+select * from iboolpart where a is not false;
+
 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);
+create table boolrangep_null partition of boolrangep default;
 
 -- 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;
 
+-- ensure we prune boolrangep_tf
+explain (costs off)  select * from boolrangep where a is not true and not b and c = 25;
+
+-- ensure we prune everything apart from boolrangep_tf and boolrangep_null
+explain (costs off)  select * from boolrangep where a is not false 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');