From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
cc: Tender Wang <tndrwang@gmail.com>,
        Duncan Sands <duncan.sands@deepbluecap.com>,
        pgsql-bugs@lists.postgresql.org
Subject: Re: Attribute of type record has wrong type error with MERGE ... WHEN
 NOT MATCHED BY SOURCE THEN DELETE
In-reply-to: 
 <CAEZATCXguOaadS0LAkjo154RMru3zDS0ohCsE4r=z63tDhqb7Q@mail.gmail.com>
References: <3518c50a-ab18-482f-b916-a37263622501@deepbluecap.com>
 <CAHewXNmuO70GHqZev500b9cGXRbHzU9cDsn2tMWqhv6bXWJ5qQ@mail.gmail.com>
 <CAEZATCXguOaadS0LAkjo154RMru3zDS0ohCsE4r=z63tDhqb7Q@mail.gmail.com>
Comments: In-reply-to Dean Rasheed <dean.a.rasheed@gmail.com>
	message dated "Mon, 10 Mar 2025 15:11:16 -0000"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <971063.1741629155.0@sss.pgh.pa.us>
Date: Mon, 10 Mar 2025 13:52:42 -0400
Message-ID: <971078.1741629162@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/971078.1741629162%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <971063.1741629155.1@sss.pgh.pa.us>

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> Unfortunately, it looks like this bug pre-dates MERGE WHEN NOT MATCHED
> BY SOURCE, and even MERGE itself. All that's needed to trigger it is a
> query that causes 2 whole-row Vars to be added, one before and one
> after view expansion. That can be made to happen via the rowmarking
> mechanism in all supported branches as follows:

Ugh, right.  So I withdraw my objection to fixing this in
makeWholeRowVar: all of the post-rewrite calls have need for this
behavior.  However, the proposed code change is wrong in detail.
The existing places that are checking for this situation are doing
tests like
	(rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid)))
I don't believe that checking relkind instead is an improvement.

> Reading the commit message for 47bb9db7599 suggests that maybe it
> would be OK to further back-patch the changes to ApplyRetrieveRule()
> to retain relkind and relid on subquery RTEs for this purpose. That
> wouldn't affect stored rules, but I haven't looked to see what else it
> might affect.

Yeah, I think we can likely get away with that.  We cannot back-patch
the changes that added relid to the outfuncs/readfuncs representation,
which means that the RTE's relid won't propagate to parallel workers,
but I don't see why they'd need it.  We only need that info to get
as far as planning.  I've not tried though.

Draft HEAD patch attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="generate-correct-vartype-for-whole-row-view-refs.patch";
	charset="us-ascii"
Content-ID: <971063.1741629155.2@sss.pgh.pa.us>
Content-Description: generate-correct-vartype-for-whole-row-view-refs.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index dbbc2f1e30d..f54d85e7bba 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 (only possible during planning), we must use the view's
+			 * rowtype, so that the resulting Var has the same type that we
+			 * would have produced from the original RTE_RELATION RTE.
+			 */
+			if (OidIsValid(rte->relid))
+			{
+				toid =3D 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 =3D RECORDOID;
+			result =3D 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/test/regress/expected/returning.out b/src/test/regress/ex=
pected/returning.out
index d1394c67833..6de764e9dad 100644
--- a/src/test/regress/expected/returning.out
+++ b/src/test/regress/expected/returning.out
@@ -286,6 +286,23 @@ SELECT * FROM voo;
  16 | zoo2
 (2 rows)
 =

+-- Check use of an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 =3D foo_v.f2 FROM foo_v WHERE foo_v.f1 =3D 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)
+
 -- Try a join case
 CREATE TEMP TABLE joinme (f2j text, other int);
 INSERT INTO joinme VALUES('more', 12345);
@@ -726,8 +743,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 =3D f4 + 1 RETURNING old.f3;  -- should fail
diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/ret=
urning.sql
index 54caf56244c..df11caef502 100644
--- a/src/test/regress/sql/returning.sql
+++ b/src/test/regress/sql/returning.sql
@@ -132,6 +132,12 @@ DELETE FROM foo WHERE f2 =3D 'zit' RETURNING *;
 SELECT * FROM foo;
 SELECT * FROM voo;
 =

+-- Check use of an un-flattenable view
+CREATE TEMP VIEW foo_v AS SELECT * FROM foo OFFSET 0;
+UPDATE foo SET f2 =3D foo_v.f2 FROM foo_v WHERE foo_v.f1 =3D foo.f1
+  RETURNING foo_v;
+SELECT * FROM foo;
+
 -- Try a join case
 =

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

------- =_aaaaaaaaaa0--