From: Dean Rasheed Date: Thu, 11 Jun 2026 11:08:47 +0000 (+0100) Subject: Fix parsing of parenthesised OLD/NEW in RETURNING list. X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=79c65b9d97fe92ea2792be09479cf4bbea7cefe1;p=thirdparty%2Fpostgresql.git Fix parsing of parenthesised OLD/NEW in RETURNING list. When parsing expressions like (old).colname and (old).* in a RETURNING list, the parser would lose track of the intended varreturningtype, and therefore return incorrect results. The root cause was code using GetNSItemByRangeTablePosn() to find a namespace item from its rtindex and levelsup, without taking into account returningtype, which would return the wrong namespace item. Fix by adding a new function GetNSItemByVar() that does take returningtype into account. Backpatch to v18, where support for RETURNING OLD/NEW was added. Bug: #19516 Reported-by: Marko Grujic Author: Marko Grujic Suggested-by: Dean Rasheed Reviewed-by: Dean Rasheed Discussion: https://postgr.es/m/CAOvwyF2cO_5mAt=w=y-dFnaG5UkZ+3H8nSDoKF_iuWZHsU2ARg@mail.gmail.com Backpatch-through: 18 --- diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 9edace67e1d..3b92ae2a920 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -1035,13 +1035,12 @@ coerce_record_to_complex(ParseState *pstate, Node *node, else if (node && IsA(node, Var) && ((Var *) node)->varattno == InvalidAttrNumber) { - int rtindex = ((Var *) node)->varno; - int sublevels_up = ((Var *) node)->varlevelsup; - int vlocation = ((Var *) node)->location; + Var *var = (Var *) node; ParseNamespaceItem *nsitem; - nsitem = GetNSItemByRangeTablePosn(pstate, rtindex, sublevels_up); - args = expandNSItemVars(pstate, nsitem, sublevels_up, vlocation, NULL); + nsitem = GetNSItemByVar(pstate, var); + args = expandNSItemVars(pstate, nsitem, var->varlevelsup, + var->location, NULL); } else ereport(ERROR, diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index e07e7911f87..2e4cc1de50d 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2056,9 +2056,7 @@ ParseComplexProjection(ParseState *pstate, const char *funcname, Node *first_arg { ParseNamespaceItem *nsitem; - nsitem = GetNSItemByRangeTablePosn(pstate, - ((Var *) first_arg)->varno, - ((Var *) first_arg)->varlevelsup); + nsitem = GetNSItemByVar(pstate, (Var *) first_arg); /* Return a Var if funcname matches a column, else NULL */ return scanNSItemForColumn(pstate, nsitem, ((Var *) first_arg)->varlevelsup, diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 43460e4a5a5..ced210cd206 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -509,6 +509,9 @@ check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem, /* * Given an RT index and nesting depth, find the corresponding * ParseNamespaceItem (there must be one). + * + * NB: Callers starting from a Var should consider using GetNSItemByVar() + * instead, to find the namespace item with matching varreturningtype. */ ParseNamespaceItem * GetNSItemByRangeTablePosn(ParseState *pstate, @@ -533,6 +536,35 @@ GetNSItemByRangeTablePosn(ParseState *pstate, return NULL; /* keep compiler quiet */ } +/* + * Given a Var, find the corresponding ParseNamespaceItem (there must be one). + * + * Like GetNSItemByRangeTablePosn(), but uses the Var's varreturningtype in + * addition to its varno and varlevelsup to find the namespace item. + */ +ParseNamespaceItem * +GetNSItemByVar(ParseState *pstate, Var *var) +{ + int sublevels_up = var->varlevelsup; + ListCell *lc; + + while (sublevels_up-- > 0) + { + pstate = pstate->parentParseState; + Assert(pstate != NULL); + } + foreach(lc, pstate->p_namespace) + { + ParseNamespaceItem *nsitem = (ParseNamespaceItem *) lfirst(lc); + + if (nsitem->p_rtindex == var->varno && + nsitem->p_returning_type == var->varreturningtype) + return nsitem; + } + elog(ERROR, "nsitem not found (internal error)"); + return NULL; /* keep compiler quiet */ +} + /* * Given an RT index and nesting depth, find the corresponding RTE. * (Note that the RTE need not be in the query's namespace.) diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 541fef5f183..6a862d5a4f9 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1451,7 +1451,7 @@ ExpandRowReference(ParseState *pstate, Node *expr, Var *var = (Var *) expr; ParseNamespaceItem *nsitem; - nsitem = GetNSItemByRangeTablePosn(pstate, var->varno, var->varlevelsup); + nsitem = GetNSItemByVar(pstate, var); return ExpandSingleTable(pstate, nsitem, var->varlevelsup, var->location, make_target_entry); } diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 59721a70efb..1bf39922c4c 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -32,6 +32,7 @@ extern void checkNameSpaceConflicts(ParseState *pstate, List *namespace1, extern ParseNamespaceItem *GetNSItemByRangeTablePosn(ParseState *pstate, int varno, int sublevels_up); +extern ParseNamespaceItem *GetNSItemByVar(ParseState *pstate, Var *var); extern RangeTblEntry *GetRTEByRangeTablePosn(ParseState *pstate, int varno, int sublevels_up); diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out index 196829e94fa..ca83d9fcc09 100644 --- a/src/test/regress/expected/returning.out +++ b/src/test/regress/expected/returning.out @@ -542,6 +542,31 @@ DELETE FROM foo WHERE f1 = 5 foo | (0,7) | 5 | ok | 42 | 100 | | | | | | | 5 | ok | 42 | 100 (1 row) +-- Parenthesized OLD and NEW +INSERT INTO foo VALUES (6, 'paren-test', 60, 600) + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; + old | f4 | f1 | f2 | f3 | f4 | new | f4 | f1 | f2 | f3 | f4 +-----+----+----+----+----+----+-----------------------+-----+----+------------+----+----- + | | | | | | (6,paren-test,60,600) | 600 | 6 | paren-test | 60 | 600 +(1 row) + +UPDATE foo SET f4 = 700 WHERE f1 = 6 + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; + old | f4 | f1 | f2 | f3 | f4 | new | f4 | f1 | f2 | f3 | f4 +-----------------------+-----+----+------------+----+-----+-----------------------+-----+----+------------+----+----- + (6,paren-test,60,600) | 600 | 6 | paren-test | 60 | 600 | (6,paren-test,60,700) | 700 | 6 | paren-test | 60 | 700 +(1 row) + +DELETE FROM foo WHERE f1 = 6 + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; + old | f4 | f1 | f2 | f3 | f4 | new | f4 | f1 | f2 | f3 | f4 +-----------------------+-----+----+------------+----+-----+-----+----+----+----+----+---- + (6,paren-test,60,700) | 700 | 6 | paren-test | 60 | 700 | | | | | | +(1 row) + -- RETURNING OLD and NEW from subquery EXPLAIN (verbose, costs off) INSERT INTO foo VALUES (5, 'subquery test') diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql index b3c8c5df550..27fa4a374ed 100644 --- a/src/test/regress/sql/returning.sql +++ b/src/test/regress/sql/returning.sql @@ -243,6 +243,17 @@ DELETE FROM foo WHERE f1 = 5 RETURNING old.tableoid::regclass, old.ctid, old.*, new.tableoid::regclass, new.ctid, new.*, *; +-- Parenthesized OLD and NEW +INSERT INTO foo VALUES (6, 'paren-test', 60, 600) + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; +UPDATE foo SET f4 = 700 WHERE f1 = 6 + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; +DELETE FROM foo WHERE f1 = 6 + RETURNING old, (old).f4, (old).*, + new, (new).f4, (new).*; + -- RETURNING OLD and NEW from subquery EXPLAIN (verbose, costs off) INSERT INTO foo VALUES (5, 'subquery test')