Handle unqualified SEQUENCE NAME options properly in parse_utilcmd.c.

generateSerialExtraStmts() was sloppy about handling the case where
SEQUENCE NAME is given with a not-schema-qualified name.  It was generating
a CreateSeqStmt with an unqualified sequence name, and an AlterSeqStmt
whose "owned_by" DefElem contained a T_String Value with a null string
pointer in the schema-name position.  The generated nextval() argument was
also underqualified.  This accidentally failed to fail at runtime, but only
so long as the current default creation namespace at runtime is the right
namespace.  That's bogus; the parse-time transformation is supposed to be
inserting the right schema name in all cases, so as to avoid any possible
skew in that selection.  I'm not sure this could fail in pg_dump's usage,
but it's still wrong; we have had real bugs in this area before adopting
the policy that parse_utilcmd.c should generate only fully-qualified
auxiliary commands.  A slightly lesser problem, which is what led me to
notice this in the first place, is that pprint() dumped core on the
AlterSeqStmt because of the bogus T_String.

Noted while poking into the open problem with ALTER SEQUENCE breaking
pg_upgrade.

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 9134fb9d63c1c5f06f2ff097e9c573889c44e690..6bc9c531c736bdaf11f1da0e49fb45e26793fc8f 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -345,6 +345,15 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
        return result;
 }
 
+/*
+ * generateSerialExtraStmts
+ *             Generate CREATE SEQUENCE and ALTER SEQUENCE ... OWNED BY statements
+ *             to create the sequence for a serial or identity column.
+ *
+ * This includes determining the name the sequence will have.  The caller
+ * can ask to get back the name components by passing non-null pointers
+ * for snamespace_p and sname_p.
+ */
 static void
 generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
                                                 Oid seqtypid, List *seqoptions, bool for_identity,
@@ -373,7 +382,6 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
         * problem, especially since few people would need two serial columns in
         * one table.
         */
-
        foreach(option, seqoptions)
        {
                DefElem    *defel = lfirst_node(DefElem, option);
@@ -393,7 +401,17 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
                RangeVar   *rv = makeRangeVarFromNameList(castNode(List, nameEl->arg));
 
                snamespace = rv->schemaname;
+               if (!snamespace)
+               {
+                       /* Given unqualified SEQUENCE NAME, select namespace */
+                       if (cxt->rel)
+                               snamespaceid = RelationGetNamespace(cxt->rel);
+                       else
+                               snamespaceid = RangeVarGetCreationNamespace(cxt->relation);
+                       snamespace = get_namespace_name(snamespaceid);
+               }
                sname = rv->relname;
+               /* Remove the SEQUENCE NAME item from seqoptions */
                seqoptions = list_delete_ptr(seqoptions, nameEl);
        }
        else
@@ -433,7 +451,9 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
         * not our synthetic one.
         */
        if (seqtypid)
-               seqstmt->options = lcons(makeDefElem("as", (Node *) makeTypeNameFromOid(seqtypid, -1), -1),
+               seqstmt->options = lcons(makeDefElem("as",
+                                                                 (Node *) makeTypeNameFromOid(seqtypid, -1),
+                                                                                        -1),
                                                                 seqstmt->options);
 
        /*