Fix corruption of tableElts list by MergeAttributes().

Since commit e7b3349a8ad7afaad565c573fbd65fb46af6abbe, MergeAttributes
destructively modifies the input List, to which the caller's
CreateStmt still points.  One may wonder whether this was already a
bug, but commit f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63 made things
noticeably worse by adding additional destructive modifications so
that the caller's List might, in the case of creation a partitioned
table, no longer even be structurally valid.  Restore the status quo
ante by assigning the return value of MergeAttributes back to
stmt->tableElts in the caller.

In most of the places where DefineRelation is called, it doesn't
matter what stmt->tableElts points to here or whether it's valid or
not, because the caller doesn't use the statement for anything after
DefineRelation returns anyway.  However, ProcessUtilitySlow passes it
to EventTriggerCollectSimpleCommand, and that function tries to invoke
copyObject on it.  If any of the CreateStmt's substructure is invalid
at that point, undefined behavior will result.

One might wonder whether this whole area needs further revision -
perhaps DefineRelation() ought not to be destructively modifying the
caller-provided CreateStmt at all.  However, that would be a behavior
change for any event triggers using C code to inspect the CreateStmt,
so for now, just fix the crash.

Report by Amit Langote, who provided a somewhat different patch for it.

Discussion: http://postgr.es/m/bf6a39a7-100a-74bd-1156-3c16a1429d88@lab.ntt.co.jp

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 99c51b812d48fdff2f04fcdaff709b054ad97488..fb961e46c4ac90707e892e74c0fb14f53edc753d 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -491,7 +491,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 {
    char        relname[NAMEDATALEN];
    Oid         namespaceId;
-   List       *schema = stmt->tableElts;
    Oid         relationId;
    Oid         tablespaceId;
    Relation    rel;
@@ -614,19 +613,21 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 
    /*
     * Look up inheritance ancestors and generate relation schema, including
-    * inherited attributes.
+    * inherited attributes.  (Note that stmt->tableElts is destructively
+    * modified by MergeAttributes.)
     */
-   schema = MergeAttributes(schema, stmt->inhRelations,
-                            stmt->relation->relpersistence,
-                            stmt->partbound != NULL,
-                            &inheritOids, &old_constraints, &parentOidCount);
+   stmt->tableElts =
+       MergeAttributes(stmt->tableElts, stmt->inhRelations,
+                       stmt->relation->relpersistence,
+                       stmt->partbound != NULL,
+                       &inheritOids, &old_constraints, &parentOidCount);
 
    /*
     * Create a tuple descriptor from the relation schema.  Note that this
     * deals with column names, types, and NOT NULL constraints, but not
     * default values or CHECK constraints; we handle those below.
     */
-   descriptor = BuildDescForRelation(schema);
+   descriptor = BuildDescForRelation(stmt->tableElts);
 
    /*
     * Notice that we allow OIDs here only for plain tables and partitioned
@@ -667,7 +668,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
    cookedDefaults = NIL;
    attnum = 0;
 
-   foreach(listptr, schema)
+   foreach(listptr, stmt->tableElts)
    {
        ColumnDef  *colDef = lfirst(listptr);