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

I wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> Maybe we need to not clear rte->functions, and use that
>> instead.

> I'll have a go at coding it that way...

Yeah, that is a better way.  In exchange for a slightly dirtier
data structure, we now have essentially all the relevant code in
makeWholeRowVar: correctness only depends on its different case
branches agreeing, rather than on some not-terribly-similar code
way over in prepjointree.c.

I also took the opportunity to split off the old-branch adjustment
of rewriteHandler.c, so that the HEAD and v15 versions of the main
bug fix patch are nearly the same.

            regards, tom lane

diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index dbbc2f1e30d..939444b6c0d 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -161,6 +161,49 @@ 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))
+            {
+                /* Subquery was expanded from a view */
+                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 if (rte->functions)
+            {
+                /*
+                 * Subquery was expanded from a set-returning function.  That
+                 * would not have happened if there's more than one function
+                 * or ordinality was requested.  We also needn't worry about
+                 * the allowScalar case, since the planner doesn't use that.
+                 */
+                Assert(!allowScalar);
+                fexpr = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;
+                toid = exprType(fexpr);
+                if (!type_is_rowtype(toid))
+                    toid = RECORDOID;
+            }
+            else
+                toid = RECORDOID;
+            result = makeVar(varno,
+                             InvalidAttrNumber,
+                             toid,
+                             -1,
+                             InvalidOid,
+                             varlevelsup);
+            break;
+
         case RTE_FUNCTION:

             /*
@@ -217,8 +260,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..a9efc0b23a2 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -914,8 +914,14 @@ preprocess_function_rtes(PlannerInfo *root)
                 rte->rtekind = RTE_SUBQUERY;
                 rte->subquery = funcquery;
                 rte->security_barrier = false;
-                /* Clear fields that should not be set in a subquery RTE */
-                rte->functions = NIL;
+
+                /*
+                 * Clear fields that should not be set in a subquery RTE.
+                 * However, we leave rte->functions filled in for the moment,
+                 * in case makeWholeRowVar needs to consult it.  We'll clear
+                 * it in setrefs.c (see add_rte_to_flat_rtable) so that this
+                 * abuse of the data structure doesn't escape the planner.
+                 */
                 rte->funcordinality = false;
             }
         }
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index d1394c67833..341b689f766 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -286,6 +286,63 @@ 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();
+-- As above, but SRF is defined to return a composite type
+CREATE TYPE foo_t AS (f1 int, f2 text, f3 int, f4 int8);
+CREATE FUNCTION foo_f() RETURNS SETOF foo_t 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();
+DROP TYPE foo_t;
 -- Try a join case
 CREATE TEMP TABLE joinme (f2j text, other int);
 INSERT INTO joinme VALUES('more', 12345);
@@ -726,8 +783,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..cc99cb53f63 100644
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -132,6 +132,30 @@ 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();
+
+-- As above, but SRF is defined to return a composite type
+CREATE TYPE foo_t AS (f1 int, f2 text, f3 int, f4 int8);
+CREATE FUNCTION foo_f() RETURNS SETOF foo_t 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();
+DROP TYPE foo_t;
+
 -- Try a join case

 CREATE TEMP TABLE joinme (f2j text, other int);
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..abc87c59e6f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1017,6 +1017,12 @@ 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_RELATION RTE for a view 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/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index c85d8fe9751..90f726d265e 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -157,6 +157,49 @@ 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))
+            {
+                /* Subquery was expanded from a view */
+                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 if (rte->functions)
+            {
+                /*
+                 * Subquery was expanded from a set-returning function.  That
+                 * would not have happened if there's more than one function
+                 * or ordinality was requested.  We also needn't worry about
+                 * the allowScalar case, since the planner doesn't use that.
+                 */
+                Assert(!allowScalar);
+                fexpr = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;
+                toid = exprType(fexpr);
+                if (!type_is_rowtype(toid))
+                    toid = RECORDOID;
+            }
+            else
+                toid = RECORDOID;
+            result = makeVar(varno,
+                             InvalidAttrNumber,
+                             toid,
+                             -1,
+                             InvalidOid,
+                             varlevelsup);
+            break;
+
         case RTE_FUNCTION:

             /*
@@ -213,8 +256,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..a91c5c1e361 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -744,8 +744,14 @@ preprocess_function_rtes(PlannerInfo *root)
                 rte->rtekind = RTE_SUBQUERY;
                 rte->subquery = funcquery;
                 rte->security_barrier = false;
-                /* Clear fields that should not be set in a subquery RTE */
-                rte->functions = NIL;
+
+                /*
+                 * Clear fields that should not be set in a subquery RTE.
+                 * However, we leave rte->functions filled in for the moment,
+                 * in case makeWholeRowVar needs to consult it.  We'll clear
+                 * it in setrefs.c (see add_rte_to_flat_rtable) so that this
+                 * abuse of the data structure doesn't escape the planner.
+                 */
                 rte->funcordinality = false;
             }
         }
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out
index cb51bb86876..a5ebc8acc0f 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -286,6 +286,63 @@ 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();
+-- As above, but SRF is defined to return a composite type
+CREATE TYPE foo_t AS (f1 int, f2 text, f3 int, f4 int8);
+CREATE FUNCTION foo_f() RETURNS SETOF foo_t 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();
+DROP TYPE foo_t;
 -- 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..8a2a2a5861d 100644
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -132,6 +132,30 @@ 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();
+
+-- As above, but SRF is defined to return a composite type
+CREATE TYPE foo_t AS (f1 int, f2 text, f3 int, f4 int8);
+CREATE FUNCTION foo_f() RETURNS SETOF foo_t 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();
+DROP TYPE foo_t;
+
 -- Try a join case

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