From 1999fe227df70249dce7c9037147b54c1090b4ad Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 20 Oct 2026 17:32:31 -0400 Subject: [PATCH] Fix OLD/NEW aliases that EXPLAIN prints in RETURNING SubPlans. set_deparse_context_plan() set the aliases for OLD and NEW in RETURNING when EXPLAIN printed a ModifyTable, but that's the wrong place: they should be set in set_deparse_plan() like other plan-node-specific fields. So they were not changed when push_ancestor_plan() moved to the ancestor ModifyTable to print a Param of a SubPlan in its RETURNING list, and the aliases of some previously-printed ModifyTable could be used instead. It seems possible that we'd also fail to print an alias at all, since get_variable() is unfazed by obtaining a NULL refname. That shouldn't happen for RETURNING variables, so complain. Author: Henson Choi Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAAAe_zAUb_XydapJqUk7O8o2crJa7xbxX=KvSi3nY9h98OcN7g@mail.gmail.com Backpatch-through: 18 --- src/backend/utils/adt/ruleutils.c | 30 +++++++----- src/test/regress/expected/returning.out | 64 +++++++++++++++++++++++++ src/test/regress/sql/returning.sql | 24 ++++++++++ 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 7606869abed..908f9056fee 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -3839,13 +3839,6 @@ set_deparse_context_plan(List *dpcontext, Plan *plan, List *ancestors) dpns->ancestors = ancestors; set_deparse_plan(dpns, plan); - /* For ModifyTable, set aliases for OLD and NEW in RETURNING */ - if (IsA(plan, ModifyTable)) - { - dpns->ret_old_alias = ((ModifyTable *) plan)->returningOldAlias; - dpns->ret_new_alias = ((ModifyTable *) plan)->returningNewAlias; - } - return dpcontext; } @@ -5148,10 +5141,10 @@ get_rtable_name(int rtindex, deparse_context *context) * of a given Plan node * * This sets the plan, outer_plan, inner_plan, outer_tlist, inner_tlist, - * and index_tlist fields. Caller must already have adjusted the ancestors - * list if necessary. Note that the rtable, subplans, and ctes fields do - * not need to change when shifting attention to different plan nodes in a - * single plan tree. + * index_tlist, ret_old_alias, and ret_new_alias fields as appropriate. + * Caller must already have adjusted the ancestors list if necessary. + * Note that the rtable, subplans, and ctes fields do not need to change + * when shifting attention to different plan nodes in a single plan tree. */ static void set_deparse_plan(deparse_namespace *dpns, Plan *plan) @@ -5227,6 +5220,15 @@ set_deparse_plan(deparse_namespace *dpns, Plan *plan) dpns->index_tlist = ((CustomScan *) plan)->custom_scan_tlist; else dpns->index_tlist = NIL; + + /* Set up table aliases for OLD and NEW in RETURNING */ + if (IsA(plan, ModifyTable)) + { + dpns->ret_old_alias = ((ModifyTable *) plan)->returningOldAlias; + dpns->ret_new_alias = ((ModifyTable *) plan)->returningNewAlias; + } + else + dpns->ret_old_alias = dpns->ret_new_alias = NULL; } /* @@ -7735,9 +7737,15 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) /* might be returning old/new column value */ if (var->varreturningtype == VAR_RETURNING_OLD) + { refname = dpns->ret_old_alias; + Assert(refname); + } else if (var->varreturningtype == VAR_RETURNING_NEW) + { refname = dpns->ret_new_alias; + Assert(refname); + } else refname = (char *) list_nth(dpns->rtable_names, varno - 1); diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out index 44d4438fb86..17cc4824172 100644 --- a/src/test/regress/expected/returning.out +++ b/src/test/regress/expected/returning.out @@ -1022,6 +1022,70 @@ BEGIN ATOMIC WHERE (foo_1.* = n.*)) AS count; END DROP FUNCTION foo_update; +-- Deparsing must print the OLD/NEW aliases of the ModifyTable that owns a +-- RETURNING list also inside its SubPlans, even when a data-modifying CTE +-- has set different aliases. +CREATE TABLE returning_e1 (k int, c int); +CREATE TABLE returning_e2 (k int, c int); +EXPLAIN (VERBOSE, COSTS OFF) +WITH x AS (UPDATE returning_e2 SET c = c + 1 + RETURNING WITH (OLD AS o, NEW AS n) o.k) +UPDATE returning_e1 SET c = c + 1 + RETURNING WITH (OLD AS p, NEW AS q) p.c, + (SELECT count(*) FROM returning_e2 WHERE returning_e2.c = p.c) AS m, + (SELECT count(*) FROM returning_e2 WHERE returning_e2.c = q.c) AS m2; + QUERY PLAN +----------------------------------------------------------------- + Update on public.returning_e1 + Output: p.c, (SubPlan expr_1), (SubPlan expr_2) + CTE x + -> Update on public.returning_e2 + Output: o.k + -> Seq Scan on public.returning_e2 + Output: (returning_e2.c + 1), returning_e2.ctid + -> Seq Scan on public.returning_e1 + Output: (returning_e1.c + 1), returning_e1.ctid + SubPlan expr_1 + -> Aggregate + Output: count(*) + -> Seq Scan on public.returning_e2 returning_e2_1 + Output: returning_e2_1.k, returning_e2_1.c + Filter: (returning_e2_1.c = p.c) + SubPlan expr_2 + -> Aggregate + Output: count(*) + -> Seq Scan on public.returning_e2 returning_e2_2 + Output: returning_e2_2.k, returning_e2_2.c + Filter: (returning_e2_2.c = q.c) +(21 rows) + +-- likewise when a SubPlan compares values returned by the CTE +EXPLAIN (VERBOSE, COSTS OFF) +WITH x AS (UPDATE returning_e2 SET c = c + 1 + RETURNING WITH (OLD AS o, NEW AS n) n.k, o.c AS oc) +UPDATE returning_e1 SET c = c + 1 + RETURNING WITH (OLD AS p) p.c, + (SELECT count(*) FROM x WHERE x.oc = p.c) AS m; + QUERY PLAN +----------------------------------------------------------------- + Update on public.returning_e1 + Output: p.c, (SubPlan expr_1) + CTE x + -> Update on public.returning_e2 + Output: n.k, o.c + -> Seq Scan on public.returning_e2 + Output: (returning_e2.c + 1), returning_e2.ctid + -> Seq Scan on public.returning_e1 + Output: (returning_e1.c + 1), returning_e1.ctid + SubPlan expr_1 + -> Aggregate + Output: count(*) + -> CTE Scan on x + Output: x.k, x.oc + Filter: (x.oc = p.c) +(15 rows) + +DROP TABLE returning_e1, returning_e2; -- Test that the planner does not fold OLD/NEW IS NULL tests to constants -- based on NOT NULL constraints, since OLD is NULL for INSERT and NEW is -- NULL for DELETE. diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql index a893f47e34d..5ca89607fbe 100644 --- a/src/test/regress/sql/returning.sql +++ b/src/test/regress/sql/returning.sql @@ -425,6 +425,30 @@ END; \sf foo_update DROP FUNCTION foo_update; +-- Deparsing must print the OLD/NEW aliases of the ModifyTable that owns a +-- RETURNING list also inside its SubPlans, even when a data-modifying CTE +-- has set different aliases. +CREATE TABLE returning_e1 (k int, c int); +CREATE TABLE returning_e2 (k int, c int); + +EXPLAIN (VERBOSE, COSTS OFF) +WITH x AS (UPDATE returning_e2 SET c = c + 1 + RETURNING WITH (OLD AS o, NEW AS n) o.k) +UPDATE returning_e1 SET c = c + 1 + RETURNING WITH (OLD AS p, NEW AS q) p.c, + (SELECT count(*) FROM returning_e2 WHERE returning_e2.c = p.c) AS m, + (SELECT count(*) FROM returning_e2 WHERE returning_e2.c = q.c) AS m2; + +-- likewise when a SubPlan compares values returned by the CTE +EXPLAIN (VERBOSE, COSTS OFF) +WITH x AS (UPDATE returning_e2 SET c = c + 1 + RETURNING WITH (OLD AS o, NEW AS n) n.k, o.c AS oc) +UPDATE returning_e1 SET c = c + 1 + RETURNING WITH (OLD AS p) p.c, + (SELECT count(*) FROM x WHERE x.oc = p.c) AS m; + +DROP TABLE returning_e1, returning_e2; + -- Test that the planner does not fold OLD/NEW IS NULL tests to constants -- based on NOT NULL constraints, since OLD is NULL for INSERT and NEW is -- NULL for DELETE.