pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
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 15:11:20 -0400
Message-ID: <1207317.1741633880@sss.pgh.pa.us> (raw)
In-Reply-To: <971078.1741629162@sss.pgh.pa.us>
References: <3518c50a-ab18-482f-b916-a37263622501@deepbluecap.com>
	<CAHewXNmuO70GHqZev500b9cGXRbHzU9cDsn2tMWqhv6bXWJ5qQ@mail.gmail.com>
	<CAEZATCXguOaadS0LAkjo154RMru3zDS0ohCsE4r=z63tDhqb7Q@mail.gmail.com>
	<971078.1741629162@sss.pgh.pa.us>

I wrote:
> 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.

OK, the attached patch for v15 passes check-world, with or without
force_parallel_mode.  I'm inclined to commit the rewriteHandler.c
and parsenodes.h bits in a separate patch for commit log visibility.

It would be easy enough to leave the RTE's relkind and/or rellockmode
alone too, but I think the conservative approach is to not change
more than we have to in these old branches.  There is some attraction
to making the behavior more like the newer branches, but still ...

			regards, tom lane

Attachments:

generate-correct-vartype-for-whole-row-view-refs-15.patch (text/x-diff, 4.5K) download diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index c85d8fe9751..c16263056e3 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 (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: /* @@ -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/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..4b9e41cb6b6 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 a RTE_RELATION RTE for a view is transformed to a + * 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/expected/returning.out index cb51bb86876..82d8c0ed6d1 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); diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql index a460f82fb7c..0b224b54419 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);

view thread (23+ messages)  latest in thread

Message-ID: <1207317.1741633880@sss.pgh.pa.us>
Permalink:  ../1207317.1741633880@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1207317.1741633880@sss.pgh.pa.us

 ·  · 

reply

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: <1207317.1741633880@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