From ac8d7e1b834e252c9aa8d5750f70a86ca74228b8 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 19 May 2017 15:02:16 -0400 Subject: [PATCH] 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 --- src/backend/commands/tablecmds.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 99c51b812d..fb961e46c4 100644 --- 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); -- 2.40.0