From be3d90026a3c17c7e6cc23d52430c37df403d869 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 11 Jun 2018 12:08:09 -0400 Subject: [PATCH] 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 --- src/backend/partitioning/partprune.c | 48 ++++++++++++++----- src/test/regress/expected/partition_prune.out | 14 ++++++ src/test/regress/sql/partition_prune.sql | 4 ++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c index fc0388e621..aaad35badb 100644 --- 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 ab32c7d67e..854b553d0a 100644 --- 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 609fe09aeb..ae361b52f9 100644 --- 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