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 <markoog@gmail.com>
Author: Marko Grujic <markoog@gmail.com>
Suggested-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
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 0b5b81c7f27ee66743dd32859cf2b453466e5537..3900f6edbcf830f9c861c1306fdad3e969559c9c 100644 (file)
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1034,13 +1034,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 583bbbf232f0479eee0641958d0bbbc7f984083e..cd2bd3784b484c46ac5d1addcf469a038d469db8 100644 (file)
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -1930,9 +1930,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 04ecf64b1fc25002c9d620f521461063fff26de5..b59df7dcdf379eba2a1f0e73ae8207f01a1a0e48 100644 (file)
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -513,6 +513,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,
@@ -537,6 +540,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 4aba0d9d4d5ccfda55da06685b2b399e06f3c9fa..336cf66f224b2b3c01c94f3aea11b79fae3e3776 100644 (file)
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1446,7 +1446,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 d59599cf2424eadfe38ebcbe2b0052b7dab6a731..a05f48317b5540f59c62b7aa971f804fb177aa05 100644 (file)
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -31,6 +31,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 341b689f76655c31ed05d5ae85119dda89660e37..d5920fcc652e6686edf5e6fa6178cd25a081516b 100644 (file)
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -540,6 +540,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 cc99cb53f63c4b87e152af838e62da53e29d9e93..133e115851b36c72b8e5dbf820df4e86931e5d0f 100644 (file)
--- 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')