From cefdbc8b938c02ffa84d8d8e99c5e6a5f90f89c7 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Thu, 3 Sep 2026 12:21:20 -0700 Subject: [PATCH v1] Don't evaluate the FOR PORTION OF target under EXPLAIN ExecInitModifyTable() evaluated ForPortionOfExpr.targetRange unconditionally at executor start-up, with no EXEC_FLAG_EXPLAIN_ONLY guard. This caused three problems: - EXPLAIN (GENERIC_PLAN) failed with "no value found for parameter 1". - Plain EXPLAIN with FOR PORTION OF (null) raised "FOR PORTION OF target must not be null". - Plain EXPLAIN ran user functions appearing in the target a second time, beyond the one evaluation the planner already does for selectivity estimation. Now we skip the evaluation and the accompanying null check in explain-only mode. Reported-by: Noah Misch Author: Paul A. Jungwirth Backpatch-through: 19 --- src/backend/executor/nodeModifyTable.c | 33 +++++--- src/test/regress/expected/for_portion_of.out | 89 ++++++++++++++++++++ src/test/regress/sql/for_portion_of.sql | 44 ++++++++++ 3 files changed, 152 insertions(+), 14 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 5681505d31c..3c144241e0a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -5632,22 +5632,27 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) forPortionOf = (ForPortionOfExpr *) node->forPortionOf; /* Eval the FOR PORTION OF target */ - if (mtstate->ps.ps_ExprContext == NULL) - ExecAssignExprContext(estate, &mtstate->ps); - econtext = mtstate->ps.ps_ExprContext; + if (!(eflags & EXEC_FLAG_EXPLAIN_ONLY)) + { + if (mtstate->ps.ps_ExprContext == NULL) + ExecAssignExprContext(estate, &mtstate->ps); + econtext = mtstate->ps.ps_ExprContext; - exprState = ExecPrepareExpr((Expr *) forPortionOf->targetRange, estate); - targetRange = ExecEvalExpr(exprState, econtext, &isNull); + exprState = ExecPrepareExpr((Expr *) forPortionOf->targetRange, estate); + targetRange = ExecEvalExpr(exprState, econtext, &isNull); - /* - * FOR PORTION OF ... TO ... FROM should never give us a NULL target, - * but FOR PORTION OF (...) could. - */ - if (isNull) - ereport(ERROR, - (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), - errmsg("FOR PORTION OF target must not be null"), - executor_errposition(estate, forPortionOf->targetLocation))); + /* + * FOR PORTION OF ... TO ... FROM should never give us a NULL + * target, but FOR PORTION OF (...) could. + */ + if (isNull) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("FOR PORTION OF target must not be null"), + executor_errposition(estate, forPortionOf->targetLocation))); + } + else + targetRange = (Datum) 0; /* Create state for FOR PORTION OF operation */ diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index 64789d1777b..84adfdd24fa 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -2793,4 +2793,93 @@ SELECT * FROM fpo_rls ORDER BY valid_at; DROP TABLE fpo_rls; DROP ROLE regress_fpo_rls; +-- +-- EXPLAIN without ANALYZE must only plan the statement, so it must not +-- evaluate the FOR PORTION OF target. +-- +CREATE TABLE fpo_explain ( + id int4range, + valid_at daterange, + name text +); +INSERT INTO fpo_explain (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); +-- GENERIC_PLAN exists precisely so that a statement containing parameter +-- placeholders can be planned without supplying any parameter values. The +-- hand-written equivalent qual is the control case. +EXPLAIN (COSTS OFF, GENERIC_PLAN) + UPDATE fpo_explain SET name = 'q' WHERE valid_at && $1::daterange; + QUERY PLAN +---------------------------------- + Update on fpo_explain + -> Seq Scan on fpo_explain + Filter: (valid_at && $1) +(3 rows) + +EXPLAIN (COSTS OFF, GENERIC_PLAN) + UPDATE fpo_explain FOR PORTION OF valid_at ($1::daterange) SET name = 'q'; + QUERY PLAN +---------------------------------- + Update on fpo_explain + -> Seq Scan on fpo_explain + Filter: (valid_at && $1) +(3 rows) + +EXPLAIN (COSTS OFF, GENERIC_PLAN) + DELETE FROM fpo_explain FOR PORTION OF valid_at ($1::daterange); + QUERY PLAN +---------------------------------- + Delete on fpo_explain + -> Seq Scan on fpo_explain + Filter: (valid_at && $1) +(3 rows) + +-- A null target is a run-time error, so plain EXPLAIN should still report +-- the plan. +EXPLAIN (COSTS OFF) + UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; + QUERY PLAN +--------------------------------------- + Update on fpo_explain + -> Result + Replaces: Scan on fpo_explain + One-Time Filter: false +(4 rows) + +EXPLAIN (COSTS OFF) + DELETE FROM fpo_explain FOR PORTION OF valid_at (NULL::daterange); + QUERY PLAN +--------------------------------------- + Delete on fpo_explain + -> Result + Replaces: Scan on fpo_explain + One-Time Filter: false +(4 rows) + +-- Actually running it is still an error. +UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; +ERROR: FOR PORTION OF target must not be null +LINE 1: UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange)... + ^ +DELETE FROM fpo_explain FOR PORTION OF valid_at (NULL::daterange); +ERROR: FOR PORTION OF target must not be null +LINE 1: DELETE FROM fpo_explain FOR PORTION OF valid_at (NULL::dater... + ^ +-- EXPLAIN ANALYZE does execute, so the null check must still fire there. +EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF) + UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; +ERROR: FOR PORTION OF target must not be null +LINE 2: UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterang... + ^ +UPDATE fpo_explain FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01' + SET name = 'q'; +SELECT * FROM fpo_explain ORDER BY valid_at; + id | valid_at | name +-------+-------------------------+------ + [1,2) | [2000-01-01,2002-01-01) | one + [1,2) | [2002-01-01,2003-01-01) | q + [1,2) | [2003-01-01,2010-01-01) | one +(3 rows) + +DROP TABLE fpo_explain; RESET datestyle; diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql index b61fe10478e..bfc601be816 100644 --- a/src/test/regress/sql/for_portion_of.sql +++ b/src/test/regress/sql/for_portion_of.sql @@ -1849,4 +1849,48 @@ SELECT * FROM fpo_rls ORDER BY valid_at; DROP TABLE fpo_rls; DROP ROLE regress_fpo_rls; +-- +-- EXPLAIN without ANALYZE must only plan the statement, so it must not +-- evaluate the FOR PORTION OF target. +-- + +CREATE TABLE fpo_explain ( + id int4range, + valid_at daterange, + name text +); +INSERT INTO fpo_explain (id, valid_at, name) VALUES + ('[1,2)', daterange('2000-01-01', '2010-01-01'), 'one'); + +-- GENERIC_PLAN exists precisely so that a statement containing parameter +-- placeholders can be planned without supplying any parameter values. The +-- hand-written equivalent qual is the control case. +EXPLAIN (COSTS OFF, GENERIC_PLAN) + UPDATE fpo_explain SET name = 'q' WHERE valid_at && $1::daterange; +EXPLAIN (COSTS OFF, GENERIC_PLAN) + UPDATE fpo_explain FOR PORTION OF valid_at ($1::daterange) SET name = 'q'; +EXPLAIN (COSTS OFF, GENERIC_PLAN) + DELETE FROM fpo_explain FOR PORTION OF valid_at ($1::daterange); + +-- A null target is a run-time error, so plain EXPLAIN should still report +-- the plan. +EXPLAIN (COSTS OFF) + UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; +EXPLAIN (COSTS OFF) + DELETE FROM fpo_explain FOR PORTION OF valid_at (NULL::daterange); + +-- Actually running it is still an error. +UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; +DELETE FROM fpo_explain FOR PORTION OF valid_at (NULL::daterange); + +-- EXPLAIN ANALYZE does execute, so the null check must still fire there. +EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF) + UPDATE fpo_explain FOR PORTION OF valid_at (NULL::daterange) SET name = 'q'; + +UPDATE fpo_explain FOR PORTION OF valid_at FROM '2002-01-01' TO '2003-01-01' + SET name = 'q'; +SELECT * FROM fpo_explain ORDER BY valid_at; + +DROP TABLE fpo_explain; + RESET datestyle; -- 2.47.3