Don't mess up pstate->p_next_resno in transformOnConflictClause().

transformOnConflictClause incremented p_next_resno while generating the
phony targetlist for the EXCLUDED pseudo-rel.  Then that field got
incremented some more during transformTargetList, possibly leading to
free_parsestate concluding that we'd overrun the allowed length of a tlist,
as reported by Justin Pryzby.

We could fix this by resetting p_next_resno to 1 after using it for the
EXCLUDED pseudo-rel tlist, but it seems easier and less coupled to other
places if we just don't use that field at all in this loop.  (Note that
this doesn't change anything about the resnos that end up appearing in
the main target list, because those are all replaced with target-column
numbers by updateTargetListEntry.)

In passing, fix incorrect type OID assigned to the whole-row Var for
"EXCLUDED.*" (somehow this escaped having any bad consequences so far,
but it's certainly wrong); remove useless assignment to var->location;
pstrdup the column names in case of a relcache flush; and improve
nearby comments.

Back-patch to 9.5 where ON CONFLICT was introduced.

Report: https://postgr.es/m/20161204163237.GA8030@telsasoft.com

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 36f8c548ebf908c4e9a4f626961f183ed0358e7e..1a541788eb1291832c32cc2b977280c309071d60 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -996,11 +996,10 @@ transformOnConflictClause(ParseState *pstate,
        exclRelIndex = list_length(pstate->p_rtable);
 
        /*
-        * Build a targetlist for the EXCLUDED pseudo relation. Have to be
-        * careful to use resnos that correspond to attnos of the underlying
-        * relation.
+        * Build a targetlist representing the columns of the EXCLUDED pseudo
+        * relation.  Have to be careful to use resnos that correspond to
+        * attnos of the underlying relation.
         */
-       Assert(pstate->p_next_resno == 1);
        for (attno = 0; attno < targetrel->rd_rel->relnatts; attno++)
        {
            Form_pg_attribute attr = targetrel->rd_att->attrs[attno];
@@ -1021,14 +1020,11 @@ transformOnConflictClause(ParseState *pstate,
                              attr->atttypid, attr->atttypmod,
                              attr->attcollation,
                              0);
-               var->location = -1;
-
-               name = NameStr(attr->attname);
+               name = pstrdup(NameStr(attr->attname));
            }
 
-           Assert(pstate->p_next_resno == attno + 1);
            te = makeTargetEntry((Expr *) var,
-                                pstate->p_next_resno++,
+                                attno + 1,
                                 name,
                                 false);
 
@@ -1037,15 +1033,16 @@ transformOnConflictClause(ParseState *pstate,
        }
 
        /*
-        * Additionally add a whole row tlist entry for EXCLUDED. That's
-        * really only needed for ruleutils' benefit, which expects to find
-        * corresponding entries in child tlists. Alternatively we could do
-        * this only when required, but that doesn't seem worth the trouble.
+        * Add a whole-row-Var entry to support references to "EXCLUDED.*".
+        * Like the other entries in exclRelTlist, its resno must match the
+        * Var's varattno, else the wrong things happen while resolving
+        * references in setrefs.c.  This is against normal conventions for
+        * targetlists, but it's okay since we don't use this as a real tlist.
         */
        var = makeVar(exclRelIndex, InvalidAttrNumber,
-                     RelationGetRelid(targetrel),
+                     targetrel->rd_rel->reltype,
                      -1, InvalidOid, 0);
-       te = makeTargetEntry((Expr *) var, 0, NULL, true);
+       te = makeTargetEntry((Expr *) var, InvalidAttrNumber, NULL, true);
        exclRelTlist = lappend(exclRelTlist, te);
 
        /*