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 9edace67e1d0a7c45883a0bb97fc467e89f9da30..3b92ae2a920d398509fdac6e928c20e628970005 100644 (file)
--- 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 e07e7911f8799c1818124b72887866b4edf8359b..2e4cc1de50de2f7cba0d9e42fb3d146032b6fa50 100644 (file)
--- 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 43460e4a5a5a1eac5664fa38151e208e92de5da7..ced210cd206a612eb69fce8caefec1bb87490133 100644 (file)
--- 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 541fef5f18385977dfe55a8d30bacaf869c417a0..6a862d5a4f9e63856777e15a9a7d207509d56ac3 100644 (file)
--- 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 59721a70efbdd53fd25cb33ab32f9e630bbab14a..1bf39922c4cad1de30a38280461ca0cda700d251 100644 (file)
--- 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 196829e94fa4b47a91ed9fbdcca72bfdb4c64320..ca83d9fcc09134799b009759884e57cc309a230e 100644 (file)
--- 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 b3c8c5df550c8648c259076fa955f34415c2b032..27fa4a374ede019708bb594c6a86bac383f07151 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')