From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>
Cc: Duncan Sands <duncan.sands@deepbluecap.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: Attribute of type record has wrong type error with MERGE ... WHEN NOT MATCHED BY SOURCE THEN DELETE
Date: Mon, 10 Mar 2025 13:52:42 -0400
Message-ID: <971078.1741629162@sss.pgh.pa.us> (raw)
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>
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
Attachments:generate-correct-vartype-for-whole-row-view-refs.patch (text/x-diff, 3.4K) downloaddiff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.cindex 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 = 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/test/regress/expected/returning.out b/src/test/regress/expected/returning.outindex 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 = 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)+
-- 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 = f4 + 1 RETURNING old.f3; -- should fail
diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sqlindex 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 = '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 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1+ RETURNING foo_v;+SELECT * FROM foo;+
-- Try a join case
CREATE TEMP TABLE joinme (f2j text, other int);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, dean.a.rasheed@gmail.com, tndrwang@gmail.com, 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: <971078.1741629162@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox