Fix run-time partition pruning code to handle NULL values properly.

The previous coding just ignored pruning constraints that compare a
partition key to a null-valued expression.  This is silly, since really
what we can do there is conclude that all partitions are rejected: the
pruning operator is known strict so the comparison must always fail.

This also fixes the logic to not ignore constisnull for a Const comparison
value.  That's probably an unreachable case, since the planner would
normally have simplified away a strict operator with a constant-null input.
But this code has no business assuming that.

David Rowley, per a gripe from me

Discussion: https://postgr.es/m/26279.1528670981@sss.pgh.pa.us

diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index fc0388e621675ee657ca24790a0d716fd87bc287..aaad35badb2b26fce9f3d305966908263ffe82aa 100644 (file)
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -170,7 +170,8 @@ static PruneStepResult *perform_pruning_combine_step(PartitionPruneContext *cont
 static bool match_boolean_partition_clause(Oid partopfamily, Expr *clause,
                                                           Expr *partkey, Expr **outconst);
 static bool partkey_datum_from_expr(PartitionPruneContext *context,
-                                               Expr *expr, int stateidx, Datum *value);
+                                               Expr *expr, int stateidx,
+                                               Datum *value, bool *isnull);
 
 
 /*
@@ -184,8 +185,9 @@ static bool partkey_datum_from_expr(PartitionPruneContext *context,
  * indexes.
  *
  * If no non-Const expressions are being compared to the partition key in any
- * of the 'partitioned_rels', then we return NIL.  In such a case run-time
- * partition pruning would be useless, since the planner did it already.
+ * of the 'partitioned_rels', then we return NIL to indicate no run-time
+ * pruning should be performed.  Run-time pruning would be useless, since the
+ * pruning done during planning will have pruned everything that can be.
  */
 List *
 make_partition_pruneinfo(PlannerInfo *root, List *partition_rels,
@@ -2835,14 +2837,33 @@ perform_pruning_base_step(PartitionPruneContext *context,
                        Expr       *expr;
                        int                     stateidx;
                        Datum           datum;
+                       bool            isnull;
 
                        expr = lfirst(lc1);
                        stateidx = PruneCxtStateIdx(context->partnatts,
                                                                                opstep->step.step_id, keyno);
-                       if (partkey_datum_from_expr(context, expr, stateidx, &datum))
+                       if (partkey_datum_from_expr(context, expr, stateidx,
+                                                                               &datum, &isnull))
                        {
                                Oid                     cmpfn;
 
+                               /*
+                                * Since we only allow strict operators in pruning steps, any
+                                * null-valued comparison value must cause the comparison to
+                                * fail, so that no partitions could match.
+                                */
+                               if (isnull)
+                               {
+                                       PruneStepResult *result;
+
+                                       result = (PruneStepResult *) palloc(sizeof(PruneStepResult));
+                                       result->bound_offsets = NULL;
+                                       result->scan_default = false;
+                                       result->scan_null = false;
+
+                                       return result;
+                               }
+
                                /*
                                 * If we're going to need a different comparison function than
                                 * the one cached in the PartitionKey, we'll need to look up
@@ -3072,8 +3093,8 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
  *             Evaluate expression for potential partition pruning
  *
  * Evaluate 'expr', whose ExprState is stateidx of the context exprstate
- * array; set *value to the resulting Datum.  Return true if evaluation was
- * possible, otherwise false.
+ * array; set *value and *isnull to the resulting Datum and nullflag.
+ * Return true if evaluation was possible, otherwise false.
  *
  * Note that the evaluated result may be in the per-tuple memory context of
  * context->planstate->ps_ExprContext, and we may have leaked other memory
@@ -3082,11 +3103,16 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
  */
 static bool
 partkey_datum_from_expr(PartitionPruneContext *context,
-                                               Expr *expr, int stateidx, Datum *value)
+                                               Expr *expr, int stateidx,
+                                               Datum *value, bool *isnull)
 {
        if (IsA(expr, Const))
        {
-               *value = ((Const *) expr)->constvalue;
+               /* We can always determine the value of a constant */
+               Const      *con = (Const *) expr;
+
+               *value = con->constvalue;
+               *isnull = con->constisnull;
                return true;
        }
        else
@@ -3105,14 +3131,10 @@ partkey_datum_from_expr(PartitionPruneContext *context,
                {
                        ExprState  *exprstate;
                        ExprContext *ectx;
-                       bool            isNull;
 
                        exprstate = context->exprstates[stateidx];
                        ectx = context->planstate->ps_ExprContext;
-                       *value = ExecEvalExprSwitchContext(exprstate, ectx, &isNull);
-                       if (isNull)
-                               return false;
-
+                       *value = ExecEvalExprSwitchContext(exprstate, ectx, isnull);
                        return true;
                }
        }

diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index ab32c7d67eda05901cfdc63a8e5ce7e52b5ffff2..854b553d0aeaa46e190d42407ac61126c0dff815 100644 (file)
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2731,6 +2731,20 @@ explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,1);
          Filter: ((b = ANY (ARRAY[$1, $2])) AND ($3 <> b) AND ($4 <> b))
 (4 rows)
 
+-- Ensure Params that evaluate to NULL properly prune away all partitions
+explain (analyze, costs off, summary off, timing off)
+select * from listp where a = (select null::int);
+                  QUERY PLAN                  
+----------------------------------------------
+ Append (actual rows=0 loops=1)
+   InitPlan 1 (returns $0)
+     ->  Result (actual rows=1 loops=1)
+   ->  Seq Scan on listp_1_1 (never executed)
+         Filter: (a = $0)
+   ->  Seq Scan on listp_2_1 (never executed)
+         Filter: (a = $0)
+(7 rows)
+
 drop table listp;
 -- Ensure runtime pruning works with initplans params with boolean types
 create table boolvalues (value bool not null);

diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 609fe09aeb0325f5cd85fde40301624066cab743..ae361b52f9f07eb59f04bfebfe0c4815e8688d7d 100644 (file)
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -685,6 +685,10 @@ explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,0);
 -- One subplan will remain in this case, but it should not be executed.
 explain (analyze, costs off, summary off, timing off)  execute q1 (1,2,2,1);
 
+-- Ensure Params that evaluate to NULL properly prune away all partitions
+explain (analyze, costs off, summary off, timing off)
+select * from listp where a = (select null::int);
+
 drop table listp;
 
 -- Ensure runtime pruning works with initplans params with boolean types