]> granicus.if.org Git - postgresql/commitdiff
Fix some minor issues exposed by outfuncs/readfuncs testing.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 18 Sep 2018 19:08:28 +0000 (15:08 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 18 Sep 2018 19:08:28 +0000 (15:08 -0400)
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
src/backend/nodes/readfuncs.c
src/backend/optimizer/prep/prepjointree.c
src/backend/parser/analyze.c
src/backend/rewrite/rewriteHandler.c
src/include/catalog/catversion.h
src/include/nodes/parsenodes.h

index 69fd5b298050f3c1d758a77ea80140135c6e1e9a..93f1e2c4ebcbe8a464c2114ffad9038df7161b11 100644 (file)
@@ -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);
 }
index ca5c21aac1e68b264132aa9d9edc551bcc20204d..81f568b3ee1135cf9abe542dbd9188f430ca4722 100644 (file)
@@ -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);
 
index c3f46a26c3aa7e576a685dfd0d5abf2ad6c0590b..688b3a1c396708b81aff7bc5e5d1e8741ca50af4 100644 (file)
@@ -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;
                        }
                }
        }
index c601b6d40d16106e3c89c83c901bd13437963b75..c020600955910ad29e7f72da11c44d68b6752c7c 100644 (file)
@@ -1116,7 +1116,7 @@ BuildOnConflictExcludedTargetlist(Relation targetrel,
                         * the Const claims to be.
                         */
                        var = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
-                       name = "";
+                       name = NULL;
                }
                else
                {
index d830569641804bbc3472a7342b04d451bf818b35..327e5c33d7ae7c5531fa902b9087bcaeda66bb6d 100644 (file)
@@ -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 */
 
        /*
index f898a2225fef6ebe302cc6e832ac7fb840129076..30bf93f7c30d9183bde4e24d8f3a3bc75e46f9fb 100644 (file)
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     201809052
+#define CATALOG_VERSION_NO     201809181
 
 #endif
index 9cd45a388a0aa73d0b2e519774b58bad6648757b..62209a8f102c7f0559dcc4bf45cee1f9c4f19154 100644 (file)
@@ -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