From db1071d4ee9f0e348ab646e7c13184d480d40516 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 18 Sep 2018 15:08:28 -0400 Subject: [PATCH] Fix some minor issues exposed by outfuncs/readfuncs testing. A test patch to pass parse and plan trees through outfuncs + readfuncs exposed several issues that need to be fixed to get clean matches: Query.withCheckOptions failed to get copied; it's intentionally ignored by outfuncs/readfuncs on the grounds that it'd always be NIL anyway in stored rules. This seems less than future-proof, and it's not even saving very much, so just undo the decision and treat the field like all others. Several places that convert a view RTE into a subquery RTE, or similar manipulations, failed to clear out fields that were specific to the original RTE type and should be zero in a subquery RTE. Since readfuncs.c will leave such fields as zero, equalfuncs.c thinks the nodes are different leading to a reported mismatch. It seems like a good idea to clear out the no-longer-needed fields, even though in principle nothing should look at them; the node ought to be indistinguishable from how it would look if we'd built a new node instead of scribbling on the old one. BuildOnConflictExcludedTargetlist randomly set the resname of some TargetEntries to "" not NULL. outfuncs/readfuncs don't distinguish those cases, and so the string will read back in as NULL ... but equalfuncs.c does distinguish. Perhaps we ought to try to make things more consistent in this area --- but it's just useless extra code space for BuildOnConflictExcludedTargetlist to not use NULL here, so I fixed it for now by making it do that. catversion bumped because the change in handling of Query.withCheckOptions affects stored rules. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us --- src/backend/nodes/outfuncs.c | 2 +- src/backend/nodes/readfuncs.c | 2 +- src/backend/optimizer/prep/prepjointree.c | 5 ++++- src/backend/parser/analyze.c | 2 +- src/backend/rewrite/rewriteHandler.c | 11 +++++++---- src/include/catalog/catversion.h | 2 +- src/include/nodes/parsenodes.h | 5 ++--- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 69fd5b2980..93f1e2c4eb 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -3003,7 +3003,7 @@ _outQuery(StringInfo str, const Query *node) WRITE_NODE_FIELD(rowMarks); WRITE_NODE_FIELD(setOperations); WRITE_NODE_FIELD(constraintDeps); - /* withCheckOptions intentionally omitted, see comment in parsenodes.h */ + WRITE_NODE_FIELD(withCheckOptions); WRITE_LOCATION_FIELD(stmt_location); WRITE_LOCATION_FIELD(stmt_len); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index ca5c21aac1..81f568b3ee 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -269,7 +269,7 @@ _readQuery(void) READ_NODE_FIELD(rowMarks); READ_NODE_FIELD(setOperations); READ_NODE_FIELD(constraintDeps); - /* withCheckOptions intentionally omitted, see comment in parsenodes.h */ + READ_NODE_FIELD(withCheckOptions); READ_LOCATION_FIELD(stmt_location); READ_LOCATION_FIELD(stmt_len); diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index c3f46a26c3..688b3a1c39 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -586,10 +586,13 @@ inline_set_returning_functions(PlannerInfo *root) funcquery = inline_set_returning_function(root, rte); if (funcquery) { - /* Successful expansion, replace the rtable entry */ + /* Successful expansion, convert the RTE to a subquery */ 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; + rte->funcordinality = false; } } } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index c601b6d40d..c020600955 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1116,7 +1116,7 @@ BuildOnConflictExcludedTargetlist(Relation targetrel, * the Const claims to be. */ var = (Var *) makeNullConst(INT4OID, -1, InvalidOid); - name = ""; + name = NULL; } else { diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index d830569641..327e5c33d7 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1582,15 +1582,18 @@ ApplyRetrieveRule(Query *parsetree, rule_action = fireRIRrules(rule_action, activeRIRs); /* - * Now, plug the view query in as a subselect, replacing the relation's - * original RTE. + * Now, plug the view query in as a subselect, converting the relation's + * original RTE to a subquery RTE. */ rte = rt_fetch(rt_index, parsetree->rtable); rte->rtekind = RTE_SUBQUERY; - rte->relid = InvalidOid; - rte->security_barrier = RelationIsSecurityView(relation); rte->subquery = rule_action; + rte->security_barrier = RelationIsSecurityView(relation); + /* Clear fields that should not be set in a subquery RTE */ + rte->relid = InvalidOid; + rte->relkind = 0; + rte->tablesample = NULL; rte->inh = false; /* must not be set for a subquery */ /* diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index f898a2225f..30bf93f7c3 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201809052 +#define CATALOG_VERSION_NO 201809181 #endif diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 9cd45a388a..62209a8f10 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -168,9 +168,8 @@ typedef struct Query List *constraintDeps; /* a list of pg_constraint OIDs that the query * depends on to be semantically valid */ - List *withCheckOptions; /* a list of WithCheckOption's, which are - * only added during rewrite and therefore - * are not written out as part of Query. */ + List *withCheckOptions; /* a list of WithCheckOption's (added + * during rewrite) */ /* * The following two fields identify the portion of the source text string