Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1ts3Bg-00E5Fb-K6 for pgsql-bugs@arkaria.postgresql.org; Tue, 11 Mar 2025 17:14:40 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1ts3Be-0038x7-GM for pgsql-bugs@arkaria.postgresql.org; Tue, 11 Mar 2025 17:14:38 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1ts3Bd-0038vb-T4 for pgsql-bugs@lists.postgresql.org; Tue, 11 Mar 2025 17:14:38 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ts3Ba-002GSF-1C for pgsql-bugs@lists.postgresql.org; Tue, 11 Mar 2025 17:14:36 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 52BHEVXe1620307; Tue, 11 Mar 2025 13:14:31 -0400 From: Tom Lane To: Dean Rasheed cc: Tender Wang , Duncan Sands , 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: <1213530.1741637326@sss.pgh.pa.us> References: <3518c50a-ab18-482f-b916-a37263622501@deepbluecap.com> <971078.1741629162@sss.pgh.pa.us> <1207317.1741633880@sss.pgh.pa.us> <1213530.1741637326@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Mon, 10 Mar 2025 16:08:46 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1620296.1741713263.0@sss.pgh.pa.us> Date: Tue, 11 Mar 2025 13:14:31 -0400 Message-ID: <1620306.1741713271@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1620296.1741713263.1@sss.pgh.pa.us> 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v2-generate-correct-vartype-for-whole-row-vars-HEAD.patch"; charset="us-ascii" Content-ID: <1620296.1741713263.2@sss.pgh.pa.us> Content-Description: v2-generate-correct-vartype-for-whole-row-vars-HEAD.patch Content-Transfer-Encoding: quoted-printable 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 =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/backend/optimizer/prep/prepjointree.c b/src/backend/optim= izer/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 =3D RTE_SUBQUERY; rte->subquery =3D funcquery; rte->security_barrier =3D 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 =3D ((RangeTblFunction *) linitial(rte->functions))->funcexpr; + toid =3D get_typ_typrelid(exprType(fexpr)); + if (OidIsValid(toid)) + rte->relid =3D toid; + /* Clear fields that should not be set in a subquery RTE */ rte->functions =3D NIL; rte->funcordinality =3D 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 locki= ng * 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 th= e + * other fields) is also set when converting an RTE_FUNCTION RTE to an + * RTE_SUBQUERY by inlining a set-returning function that returns a name= d + * 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/ex= pected/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 =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) + +-- Check use of a whole-row variable for an inlined set-returning functio= n +CREATE FUNCTION foo_f() RETURNS SETOF foo AS + $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE; +UPDATE foo SET f2 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D 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 =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..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 =3D '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 =3D foo_v.f2 FROM foo_v WHERE foo_v.f1 =3D foo.f1 + RETURNING foo_v; +SELECT * FROM foo; + +-- Check use of a whole-row variable for an inlined set-returning functio= n +CREATE FUNCTION foo_f() RETURNS SETOF foo AS + $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE; +UPDATE foo SET f2 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D foo.f1 + RETURNING foo_f; +SELECT * FROM foo; +DROP FUNCTION foo_f(); + -- Try a join case = CREATE TEMP TABLE joinme (f2j text, other int); ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v2-generate-correct-vartype-for-whole-row-vars-15.patch"; charset="us-ascii" Content-ID: <1620296.1741713263.3@sss.pgh.pa.us> Content-Description: v2-generate-correct-vartype-for-whole-row-vars-15.patch Content-Transfer-Encoding: quoted-printable 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 =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: = /* @@ -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/optim= izer/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 =3D RTE_SUBQUERY; rte->subquery =3D funcquery; rte->security_barrier =3D 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 =3D ((RangeTblFunction *) linitial(rte->functions))->funcexpr; + toid =3D get_typ_typrelid(exprType(fexpr)); + if (OidIsValid(toid)) + rte->relid =3D toid; + /* Clear fields that should not be set in a subquery RTE */ rte->functions =3D NIL; rte->funcordinality =3D false; diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/re= writeHandler.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 =3D RTE_SUBQUERY; rte->subquery =3D rule_action; rte->security_barrier =3D RelationIsSecurityView(relation); - /* Clear fields that should not be set in a subquery RTE */ - rte->relid =3D InvalidOid; + + /* + * Clear fields that should not be set in a subquery RTE. However, we + * retain the relid to support correct operation of makeWholeRowVar duri= ng + * planning. + */ rte->relkind =3D 0; rte->rellockmode =3D 0; rte->tablesample =3D 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/ex= pected/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 =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) + +-- Check use of a whole-row variable for an inlined set-returning functio= n +CREATE FUNCTION foo_f() RETURNS SETOF foo AS + $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE; +UPDATE foo SET f2 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D 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/ret= urning.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 =3D '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 =3D foo_v.f2 FROM foo_v WHERE foo_v.f1 =3D foo.f1 + RETURNING foo_v; +SELECT * FROM foo; + +-- Check use of a whole-row variable for an inlined set-returning functio= n +CREATE FUNCTION foo_f() RETURNS SETOF foo AS + $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE; +UPDATE foo SET f2 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D foo.f1 + RETURNING foo_f; +SELECT * FROM foo; +DROP FUNCTION foo_f(); + -- Try a join case = CREATE TEMP TABLE joinme (f2j text, other int); ------- =_aaaaaaaaaa0--