From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: 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: Wed, 12 Mar 2025 21:39:05 -0400
Message-ID: <2625732.1741829945@sss.pgh.pa.us> (raw)
In-Reply-To: <2588244.1741828414@sss.pgh.pa.us>
References: <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>
<1834982.1741729881@sss.pgh.pa.us>
<CAEZATCWSTZSdzSZ9dnzbTpSa97Uit=KFz4KSgPfYLSwb4+P01w@mail.gmail.com>
<2160671.1741794569@sss.pgh.pa.us>
<m2gyjjk6hazud7hezz25t7aw7rjv73fthkct4jqbvrnu3ezqz3@nx3m53r7scce>
<2560748.1741824533@sss.pgh.pa.us>
<2588244.1741828414@sss.pgh.pa.us>
I wrote:
> Double ugh: that doesn't fix it, because we also do a round of
> WRITE_READ_PARSE_PLAN_TREES checks on the rewriter output.
> Not sure how to fix this, unless we lobotomize that write/read
> check somehow.
After some thought, I'm inclined to suggest that we just remove
the post-rewrite check in the affected branches. This is certainly
not an ideal solution, but unless someone can think of a
fundamentally different way to fix the original bug in these branches,
I don't see a better way. There are some mitigating points:
* Applying WRITE_READ_PARSE_PLAN_TREES here does not correspond to
any actual system functionality requirement. We have to be able
to write/read post-parse-analysis trees to store views and rules;
and we have to be able to write/read plan trees to transfer them
to parallel workers. But there's no real reason why we have to
be able to do it for the purely-transient output of rewriting.
* We aren't going to be changing the outfuncs/readfuncs code
in these branches anymore anyway, for precisely the same reasons
that this patchset can't do that. So it's not clear what the
test is doing for us.
So that's the best I've got for tonight. I've verified that the
attached patch for v15 fixes this failure.
regards, tom lane
Attachments:v1-fix-write-read-parse-tree-failure-15.patch (text/x-diff, 3.8K) downloaddiff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.cindex 4d05d59c5bd..59387c21b51 100644--- a/src/backend/optimizer/plan/setrefs.c+++ b/src/backend/optimizer/plan/setrefs.c@@ -512,6 +512,15 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
newrte->colcollations = NIL;
newrte->securityQuals = NIL;
+ /*+ * Also, if it's a subquery RTE, lose the relid that may have been kept to+ * signal that it had been a view. We don't want that to escape the+ * planner, mainly because doing so breaks -DWRITE_READ_PARSE_PLAN_TREES+ * testing thanks to outfuncs/readfuncs not preserving it.+ */+ if (newrte->rtekind == RTE_SUBQUERY)+ newrte->relid = InvalidOid;+
glob->finalrtable = lappend(glob->finalrtable, newrte);
/*
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.cindex 30ae22e43df..aa0a3bfe89b 100644--- a/src/backend/rewrite/rewriteHandler.c+++ b/src/backend/rewrite/rewriteHandler.c@@ -1862,8 +1862,8 @@ ApplyRetrieveRule(Query *parsetree,
/*
* Clear fields that should not be set in a subquery RTE. However, we
- * retain the relid to support correct operation of makeWholeRowVar during- * planning.+ * retain the relid for now, to support correct operation of+ * makeWholeRowVar during planning.
*/
rte->relkind = 0;
rte->rellockmode = 0;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.cindex 8a464fcf150..a6a13dc71ba 100644--- a/src/backend/tcop/postgres.c+++ b/src/backend/tcop/postgres.c@@ -813,45 +813,10 @@ pg_rewrite_query(Query *query)
}
#endif
-#ifdef WRITE_READ_PARSE_PLAN_TREES- /* Optional debugging check: pass querytree through outfuncs/readfuncs */- {- List *new_list = NIL;- ListCell *lc;-- /*- * We currently lack outfuncs/readfuncs support for most utility- * statement types, so only attempt to write/read non-utility queries.- */- foreach(lc, querytree_list)- {- Query *query = lfirst_node(Query, lc);-- if (query->commandType != CMD_UTILITY)- {- char *str = nodeToString(query);- Query *new_query = stringToNodeWithLocations(str);-- /*- * queryId is not saved in stored rules, but we must preserve- * it here to avoid breaking pg_stat_statements.- */- new_query->queryId = query->queryId;-- new_list = lappend(new_list, new_query);- pfree(str);- }- else- new_list = lappend(new_list, query);- }-- /* This checks both outfuncs/readfuncs and the equal() routines... */- if (!equal(new_list, querytree_list))- elog(WARNING, "outfuncs/readfuncs failed to produce equal parse tree");- else- querytree_list = new_list;- }-#endif+ /*+ * We don't apply WRITE_READ_PARSE_PLAN_TREES to rewritten query trees,+ * because it breaks the hack of preserving relid for rewritten views.+ */
if (Debug_print_rewritten)
elog_node_display(LOG, "rewritten parse tree", querytree_list,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.hindex abc87c59e6f..a77b5ba1b14 100644--- a/src/include/nodes/parsenodes.h+++ b/src/include/nodes/parsenodes.h@@ -1020,8 +1020,8 @@ typedef struct RangeTblEntry
* 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.)+ * during planning, cf makeWholeRowVar. (It will not be passed on to the+ * executor, however.)
*
* 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
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, andres@anarazel.de, 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: <2625732.1741829945@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