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: Tue, 11 Mar 2025 17:51:21 -0400
Message-ID: <1834982.1741729881@sss.pgh.pa.us> (raw)
In-Reply-To: <1638469.1741723564@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>
<1207317.1741633880@sss.pgh.pa.us>
<CAEZATCUQjxowkRfG89YOWStPvJ3xpJW9wDNccnZOVJzSAHwJxA@mail.gmail.com>
<1213530.1741637326@sss.pgh.pa.us>
<1620306.1741713271@sss.pgh.pa.us>
<CAEZATCVcQXJ3KQUZyN-2=jzk6U1rByJ74Xfq84zeCGXxPQjEsg@mail.gmail.com>
<1638469.1741723564@sss.pgh.pa.us>
I wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> 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
Attachments:download all (3 files, .tar.gz)v3-generate-correct-vartype-for-whole-row-vars-HEAD.patch (text/x-diff, 6.5K) downloaddiff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.cindex 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 = 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 = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;+ toid = exprType(fexpr);+ if (!type_is_rowtype(toid))+ toid = RECORDOID;+ }+ else+ toid = RECORDOID;+ result = 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/optimizer/prep/prepjointree.cindex 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 = RTE_SUBQUERY;
rte->subquery = funcquery;
rte->security_barrier = false;
- /* Clear fields that should not be set in a subquery RTE */- rte->functions = 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 = false;
}
}
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.outindex 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 = 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)++-- Check use of a whole-row variable for an inlined set-returning function+CREATE FUNCTION foo_f() RETURNS SETOF foo AS+ $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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 = f4 + 1 RETURNING old.f3; -- should fail
diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sqlindex 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 = '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 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1+ RETURNING foo_v;+SELECT * FROM foo;++-- Check use of a whole-row variable for an inlined set-returning function+CREATE FUNCTION foo_f() RETURNS SETOF foo AS+ $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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);
v3-0001-preserve-relid-15.patch (text/x-diff, 1.6K) downloaddiff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.cindex 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.hindex 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 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
v3-0002-generate-correct-vartype-for-whole-row-vars-15.patch (text/x-diff, 6.1K) downloaddiff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.cindex 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 = 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 = ((RangeTblFunction *) linitial(rte->functions))->funcexpr;+ toid = exprType(fexpr);+ if (!type_is_rowtype(toid))+ toid = RECORDOID;+ }+ else+ toid = RECORDOID;+ result = 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/optimizer/prep/prepjointree.cindex 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 = RTE_SUBQUERY;
rte->subquery = funcquery;
rte->security_barrier = false;
- /* Clear fields that should not be set in a subquery RTE */- rte->functions = 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 = false;
}
}
diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.outindex 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 = 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)++-- Check use of a whole-row variable for an inlined set-returning function+CREATE FUNCTION foo_f() RETURNS SETOF foo AS+ $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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/returning.sqlindex 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 = '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 = foo_v.f2 FROM foo_v WHERE foo_v.f1 = foo.f1+ RETURNING foo_v;+SELECT * FROM foo;++-- Check use of a whole-row variable for an inlined set-returning function+CREATE FUNCTION foo_f() RETURNS SETOF foo AS+ $$ SELECT * FROM foo OFFSET 0 $$ LANGUAGE sql STABLE;+UPDATE foo SET f2 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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 = foo_f.f2 FROM foo_f() WHERE foo_f.f1 = 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);
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: <1834982.1741729881@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