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 1ts7VZ-00FXgD-1Y for pgsql-bugs@arkaria.postgresql.org; Tue, 11 Mar 2025 21:51:29 +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 1ts7VX-007m0e-NG for pgsql-bugs@arkaria.postgresql.org; Tue, 11 Mar 2025 21:51:27 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1ts7VX-007m0W-Bp for pgsql-bugs@lists.postgresql.org; Tue, 11 Mar 2025 21:51:27 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1ts7VT-002JVA-1t for pgsql-bugs@lists.postgresql.org; Tue, 11 Mar 2025 21:51:27 +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 52BLpL7Q1834983; Tue, 11 Mar 2025 17:51:21 -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: <1638469.1741723564@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> <1620306.1741713271@sss.pgh.pa.us> <1638469.1741723564@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Tue, 11 Mar 2025 16:06:04 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1834910.1741729844.0@sss.pgh.pa.us> Date: Tue, 11 Mar 2025 17:51:21 -0400 Message-ID: <1834982.1741729881@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: <1834910.1741729844.1@sss.pgh.pa.us> I wrote: > Dean Rasheed 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v3-generate-correct-vartype-for-whole-row-vars-HEAD.patch"; charset="us-ascii" Content-ID: <1834910.1741729844.2@sss.pgh.pa.us> Content-Description: v3-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..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 =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 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 =3D ((RangeTblFunction *) linitial(rte->functions))->funcexpr; + toid =3D exprType(fexpr); + if (!type_is_rowtype(toid)) + toid =3D RECORDOID; + } + else + toid =3D RECORDOID; + result =3D 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/optim= izer/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 =3D RTE_SUBQUERY; rte->subquery =3D funcquery; rte->security_barrier =3D false; - /* Clear fields that should not be set in a subquery RTE */ - rte->functions =3D 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 =3D false; } } diff --git a/src/test/regress/expected/returning.out b/src/test/regress/ex= pected/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 =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(); +-- 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 =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(); +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 =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..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 =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(); + +-- 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 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D 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); ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="v3-0001-preserve-relid-15.patch"; charset="us-ascii" Content-ID: <1834910.1741729844.3@sss.pgh.pa.us> Content-Description: v3-0001-preserve-relid-15.patch Content-Transfer-Encoding: quoted-printable 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..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 usefu= l + * 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v3-0002-generate-correct-vartype-for-whole-row-vars-15.patc"; name*1="h"; charset="us-ascii" Content-ID: <1834910.1741729844.4@sss.pgh.pa.us> Content-Description: v3-0002-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..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 =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 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 =3D ((RangeTblFunction *) linitial(rte->functions))->funcexpr; + toid =3D exprType(fexpr); + if (!type_is_rowtype(toid)) + toid =3D RECORDOID; + } + else + toid =3D RECORDOID; + result =3D 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/optim= izer/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 =3D RTE_SUBQUERY; rte->subquery =3D funcquery; rte->security_barrier =3D false; - /* Clear fields that should not be set in a subquery RTE */ - rte->functions =3D 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 =3D false; } } diff --git a/src/test/regress/expected/returning.out b/src/test/regress/ex= pected/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 =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(); +-- 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 =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(); +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/ret= urning.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 =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(); + +-- 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 =3D foo_f.f2 FROM foo_f() WHERE foo_f.f1 =3D 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); ------- =_aaaaaaaaaa0--