Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE - Mailing list pgsql-bugs

I wrote:
> Double ugh.  I guess we could get preprocess_function_rtes to
> insert the appropriate relid ...

OK, that was less painful than I feared.  makeWholeRowVar has
several different special cases for RTE_FUNCTION, but most of them
don't bear on this problem, because we wouldn't have applied inlining
when they did.  It seems sufficient to fetch pg_type.typrelid for
the function's nominal return type and store that if it's not 0.

Patches for HEAD and v15 attached.  I haven't checked other branches
but I expect the deltas won't be big.

            regards, tom lane

diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index dbbc2f1e30d..71632512380 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -161,6 +161,34 @@ makeWholeRowVar(RangeTblEntry *rte,
                              varlevelsup);
             break;

+        case RTE_SUBQUERY:
+
+            /*
+             * For a standard subquery, the Var should be of RECORD type.
+             * However, if we're looking at a subquery that was expanded from
+             * a view or SRF (only possible during planning), we must use the
+             * appropriate rowtype, so that the resulting Var has the same
+             * type that we would have produced from the original RTE.
+             */
+            if (OidIsValid(rte->relid))
+            {
+                toid = get_rel_type_id(rte->relid);
+                if (!OidIsValid(toid))
+                    ereport(ERROR,
+                            (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                             errmsg("relation \"%s\" does not have a composite type",
+                                    get_rel_name(rte->relid))));
+            }
+            else
+                toid = RECORDOID;
+            result = makeVar(varno,
+                             InvalidAttrNumber,
+                             toid,
+                             -1,
+                             InvalidOid,
+                             varlevelsup);
+            break;
+
         case RTE_FUNCTION:

             /*
@@ -217,8 +245,8 @@ makeWholeRowVar(RangeTblEntry *rte,
         default:

             /*
-             * RTE is a join, subselect, tablefunc, or VALUES.  We represent
-             * this as a whole-row Var of RECORD type. (Note that in most
+             * RTE is a join, tablefunc, VALUES, CTE, etc.  We represent these
+             * cases as a whole-row Var of RECORD type.  (Note that in most
              * cases the Var will be expanded to a RowExpr during planning,
              * but that is not our concern here.)
              */
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index bcc40dd5a84..477ff3fa76d 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -43,6 +43,7 @@
 #include "parser/parsetree.h"
 #include "rewrite/rewriteHandler.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/lsyscache.h"
 #include "utils/rel.h"


@@ -911,9 +912,28 @@ preprocess_function_rtes(PlannerInfo *root)
             if (funcquery)
             {
                 /* Successful expansion, convert the RTE to a subquery */
+                Node       *fexpr;
+                Oid            toid;
+
                 rte->rtekind = RTE_SUBQUERY;
                 rte->subquery = funcquery;
                 rte->security_barrier = false;
+
+                /*
+                 * If the SRF returned a named composite type (not RECORD), we
+                 * must also set rte->relid so that makeWholeRowVar can still
+                 * produce the same output for the RTE as it did before.  This
+                 * code relies on the fact that inline_set_returning_function
+                 * won't have succeeded unless there is exactly one entry in
+                 * rte->functions.  Also, this would likely be the wrong thing
+                 * for domain-over-composite, but functions returning those
+                 * won't get inlined either.
+                 */
+                fexpr = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;
+                toid = get_typ_typrelid(exprType(fexpr));
+                if (OidIsValid(toid))
+                    rte->relid = toid;
+
                 /* Clear fields that should not be set in a subquery RTE */
                 rte->functions = NIL;
                 rte->funcordinality = false;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 23c9e3c5abf..ef8e4eab9e0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1086,7 +1086,10 @@ typedef struct RangeTblEntry
      * containing the view's query.  We still need to perform run-time locking
      * and permission checks on the view, even though it's not directly used
      * in the query anymore, and the most expedient way to do that is to
-     * retain these fields from the old state of the RTE.
+     * retain these fields from the old state of the RTE.  relid (but not the
+     * other fields) is also set when converting an RTE_FUNCTION RTE to an
+     * RTE_SUBQUERY by inlining a set-returning function that returns a named
+     * composite type.
      *
      * As a special case, RTE_NAMEDTUPLESTORE can also set relid to indicate
      * that the tuple format of the tuplestore is the same as the referenced
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index d1394c67833..9659cae9ce7 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -286,6 +286,42 @@ SELECT * FROM voo;
  16 | zoo2
 (2 rows)

+-- Check use of a whole-row variable for an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1
+  RETURNING foo_v;
+      foo_v
+-----------------
+ (2,more,42,141)
+ (16,zoo2,57,99)
+(2 rows)
+
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4
+----+------+----+-----
+  2 | more | 42 | 141
+ 16 | zoo2 | 57 |  99
+(2 rows)
+
+-- Check use of a whole-row variable for an inlined set-returning function
+CREATE FUNCTION foo_f() RETURNS SETOF foo AS
+  $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
+  RETURNING foo_f;
+      foo_f
+-----------------
+ (2,more,42,141)
+ (16,zoo2,57,99)
+(2 rows)
+
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4
+----+------+----+-----
+  2 | more | 42 | 141
+ 16 | zoo2 | 57 |  99
+(2 rows)
+
+DROP FUNCTION foo_f();
 -- Try a join case
 CREATE TEMP TABLE joinme (f2j text, other int);
 INSERT INTO joinme VALUES('more', 12345);
@@ -726,8 +762,9 @@ NOTICE:  UPDATE: (3,zoo2,58,99,54321) -> (3,zoo2,59,7,54321)

 -- Test wholerow & dropped column handling
 ALTER TABLE foo DROP COLUMN f3 CASCADE;
-NOTICE:  drop cascades to 3 other objects
+NOTICE:  drop cascades to 4 other objects
 DETAIL:  drop cascades to rule voo_i on view voo
+drop cascades to view foo_v
 drop cascades to view joinview
 drop cascades to rule foo_del_rule on table foo
 UPDATE foo SET f4 = f4 + 1 RETURNING old.f3;  -- should fail
diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql
index 54caf56244c..9aea7fb609f 100644
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -132,6 +132,20 @@ DELETE FROM foo WHERE f2 = 'zit' RETURNING *;
 SELECT * FROM foo;
 SELECT * FROM voo;

+-- Check use of a whole-row variable for an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1
+  RETURNING foo_v;
+SELECT * FROM foo;
+
+-- Check use of a whole-row variable for an inlined set-returning function
+CREATE FUNCTION foo_f() RETURNS SETOF foo AS
+  $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
+  RETURNING foo_f;
+SELECT * FROM foo;
+DROP FUNCTION foo_f();
+
 -- Try a join case

 CREATE TEMP TABLE joinme (f2j text, other int);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index c85d8fe9751..691950857b6 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -157,6 +157,34 @@ makeWholeRowVar(RangeTblEntry *rte,
                              varlevelsup);
             break;

+        case RTE_SUBQUERY:
+
+            /*
+             * For a standard subquery, the Var should be of RECORD type.
+             * However, if we're looking at a subquery that was expanded from
+             * a view or SRF (only possible during planning), we must use the
+             * appropriate rowtype, so that the resulting Var has the same
+             * type that we would have produced from the original RTE.
+             */
+            if (OidIsValid(rte->relid))
+            {
+                toid = get_rel_type_id(rte->relid);
+                if (!OidIsValid(toid))
+                    ereport(ERROR,
+                            (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                             errmsg("relation \"%s\" does not have a composite type",
+                                    get_rel_name(rte->relid))));
+            }
+            else
+                toid = RECORDOID;
+            result = makeVar(varno,
+                             InvalidAttrNumber,
+                             toid,
+                             -1,
+                             InvalidOid,
+                             varlevelsup);
+            break;
+
         case RTE_FUNCTION:

             /*
@@ -213,8 +241,8 @@ makeWholeRowVar(RangeTblEntry *rte,
         default:

             /*
-             * RTE is a join, subselect, tablefunc, or VALUES.  We represent
-             * this as a whole-row Var of RECORD type. (Note that in most
+             * RTE is a join, tablefunc, VALUES, CTE, etc.  We represent these
+             * cases as a whole-row Var of RECORD type.  (Note that in most
              * cases the Var will be expanded to a RowExpr during planning,
              * but that is not our concern here.)
              */
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 0efcc3b24bc..aa578b61f00 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -39,6 +39,7 @@
 #include "parser/parse_relation.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/lsyscache.h"


 typedef struct pullup_replace_vars_context
@@ -741,9 +742,28 @@ preprocess_function_rtes(PlannerInfo *root)
             if (funcquery)
             {
                 /* Successful expansion, convert the RTE to a subquery */
+                Node       *fexpr;
+                Oid            toid;
+
                 rte->rtekind = RTE_SUBQUERY;
                 rte->subquery = funcquery;
                 rte->security_barrier = false;
+
+                /*
+                 * If the SRF returned a named composite type (not RECORD), we
+                 * must also set rte->relid so that makeWholeRowVar can still
+                 * produce the same output for the RTE as it did before.  This
+                 * code relies on the fact that inline_set_returning_function
+                 * won't have succeeded unless there is exactly one entry in
+                 * rte->functions.  Also, this would likely be the wrong thing
+                 * for domain-over-composite, but functions returning those
+                 * won't get inlined either.
+                 */
+                fexpr = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;
+                toid = get_typ_typrelid(exprType(fexpr));
+                if (OidIsValid(toid))
+                    rte->relid = toid;
+
                 /* Clear fields that should not be set in a subquery RTE */
                 rte->functions = NIL;
                 rte->funcordinality = false;
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index d2a0e501d1e..30ae22e43df 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1859,8 +1859,12 @@ ApplyRetrieveRule(Query *parsetree,
     rte->rtekind = RTE_SUBQUERY;
     rte->subquery = rule_action;
     rte->security_barrier = RelationIsSecurityView(relation);
-    /* Clear fields that should not be set in a subquery RTE */
-    rte->relid = InvalidOid;
+
+    /*
+     * Clear fields that should not be set in a subquery RTE.  However, we
+     * retain the relid to support correct operation of makeWholeRowVar during
+     * planning.
+     */
     rte->relkind = 0;
     rte->rellockmode = 0;
     rte->tablesample = NULL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 6944362c7ac..056902dccef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1017,6 +1017,13 @@ typedef struct RangeTblEntry
     /*
      * Fields valid for a plain relation RTE (else zero):
      *
+     * As a special case, relid can also be set in RTE_SUBQUERY RTEs.  This
+     * happens when an RTE for a view or composite-returning function is
+     * transformed to an RTE_SUBQUERY during rewriting.  We keep the relid
+     * because it is useful during planning, cf makeWholeRowVar.  (It cannot
+     * be relied on during execution, because it will not propagate to
+     * parallel workers.)
+     *
      * As a special case, RTE_NAMEDTUPLESTORE can also set relid to indicate
      * that the tuple format of the tuplestore is the same as the referenced
      * relation.  This allows plans referencing AFTER trigger transition
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index cb51bb86876..461f9fdefa6 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -286,6 +286,42 @@ SELECT * FROM voo;
  16 | zoo2
 (2 rows)

+-- Check use of a whole-row variable for an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1
+  RETURNING foo_v;
+      foo_v
+-----------------
+ (2,more,42,141)
+ (16,zoo2,57,99)
+(2 rows)
+
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4
+----+------+----+-----
+  2 | more | 42 | 141
+ 16 | zoo2 | 57 |  99
+(2 rows)
+
+-- Check use of a whole-row variable for an inlined set-returning function
+CREATE FUNCTION foo_f() RETURNS SETOF foo AS
+  $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
+  RETURNING foo_f;
+      foo_f
+-----------------
+ (2,more,42,141)
+ (16,zoo2,57,99)
+(2 rows)
+
+SELECT * FROM foo;
+ f1 |  f2  | f3 | f4
+----+------+----+-----
+  2 | more | 42 | 141
+ 16 | zoo2 | 57 |  99
+(2 rows)
+
+DROP FUNCTION foo_f();
 -- Try a join case
 CREATE TEMP TABLE joinme (f2j text, other int);
 INSERT INTO joinme VALUES('more', 12345);
diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql
index a460f82fb7c..08bfdec2d1a 100644
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -132,6 +132,20 @@ DELETE FROM foo WHERE f2 = 'zit' RETURNING *;
 SELECT * FROM foo;
 SELECT * FROM voo;

+-- Check use of a whole-row variable for an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1
+  RETURNING foo_v;
+SELECT * FROM foo;
+
+-- Check use of a whole-row variable for an inlined set-returning function
+CREATE FUNCTION foo_f() RETURNS SETOF foo AS
+  $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;
+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = foo.f1
+  RETURNING foo_f;
+SELECT * FROM foo;
+DROP FUNCTION foo_f();
+
 -- Try a join case

 CREATE TEMP TABLE joinme (f2j text, other int);