From 1ead0208b2178089b024caa2d1465a3f3056a45c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 22 Dec 2016 16:23:33 -0500 Subject: [PATCH] Fix CREATE TABLE ... LIKE ... WITH OIDS. Having a WITH OIDS specification should result in the creation of an OID column, but commit b943f502b broke that in the case that there were LIKE tables without OIDS. Commentary in that patch makes it look like this was intentional, but if so it was based on a faulty reading of what inheritance does: the parent tables can add an OID column, but they can't subtract one. AFAICS, the behavior ought to be that you get an OID column if any of the inherited tables, LIKE tables, or WITH clause ask for one. Also, revert that patch's unnecessary split of transformCreateStmt's loop over the tableElts list into two passes. That seems to have been based on a misunderstanding as well: we already have two-pass processing here, we don't need three passes. Per bug #14474 from Jeff Dafoe. Back-patch to 9.6 where the misbehavior was introduced. Report: https://postgr.es/m/20161222145304.25620.47445@wrigleys.postgresql.org --- src/backend/parser/parse_utilcmd.c | 52 +++++++------------ .../regress/expected/create_table_like.out | 9 +++- src/test/regress/sql/create_table_like.sql | 5 +- 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index cc6a961bb4..4f74208633 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -61,7 +61,6 @@ #include "rewrite/rewriteManip.h" #include "utils/acl.h" #include "utils/builtins.h" -#include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/ruleutils.h" @@ -278,10 +277,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) /* * Run through each primary element in the table creation clause. Separate - * column defs from constraints, and do preliminary analysis. We have to - * process column-defining clauses first because it can control the - * presence of columns which are referenced by columns referenced by - * constraints. + * column defs from constraints, and do preliminary analysis. */ foreach(elements, stmt->tableElts) { @@ -293,17 +289,13 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) transformColumnDefinition(&cxt, (ColumnDef *) element); break; - case T_TableLikeClause: - if (!like_found) - { - cxt.hasoids = false; - like_found = true; - } - transformTableLikeClause(&cxt, (TableLikeClause *) element); + case T_Constraint: + transformTableConstraint(&cxt, (Constraint *) element); break; - case T_Constraint: - /* process later */ + case T_TableLikeClause: + like_found = true; + transformTableLikeClause(&cxt, (TableLikeClause *) element); break; default: @@ -313,27 +305,19 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) } } - if (like_found) - { - /* - * To match INHERITS, the existence of any LIKE table with OIDs causes - * the new table to have oids. For the same reason, WITH/WITHOUT OIDs - * is also ignored with LIKE. We prepend because the first oid option - * list entry is honored. Our prepended WITHOUT OIDS clause will be - * overridden if an inherited table has oids. - */ + /* + * If we had any LIKE tables, they may require creation of an OID column + * even though the command's own WITH clause didn't ask for one (or, + * perhaps, even specifically rejected having one). Insert a WITH option + * to ensure that happens. We prepend to the list because the first oid + * option will be honored, and we want to override anything already there. + * (But note that DefineRelation will override this again to add an OID + * column if one appears in an inheritance parent table.) + */ + if (like_found && cxt.hasoids) stmt->options = lcons(makeDefElem("oids", - (Node *) makeInteger(cxt.hasoids), -1), + (Node *) makeInteger(true), -1), stmt->options); - } - - foreach(elements, stmt->tableElts) - { - Node *element = lfirst(elements); - - if (nodeTag(element) == T_Constraint) - transformTableConstraint(&cxt, (Constraint *) element); - } /* * transformIndexConstraints wants cxt.alist to contain only index @@ -975,7 +959,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla } /* We use oids if at least one LIKE'ed table has oids. */ - cxt->hasoids = cxt->hasoids || relation->rd_rel->relhasoids; + cxt->hasoids |= relation->rd_rel->relhasoids; /* * Copy CHECK constraints if requested, being careful to adjust attribute diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out index d1cfb4aef0..a25b221703 100644 --- a/src/test/regress/expected/create_table_like.out +++ b/src/test/regress/expected/create_table_like.out @@ -254,4 +254,11 @@ SELECT oid FROM like_test4; ----- (0 rows) -DROP TABLE has_oid, no_oid, like_test, like_test2, like_test3, like_test4; +CREATE TABLE like_test5 (z INTEGER, LIKE no_oid) WITH OIDS; +SELECT oid FROM like_test5; + oid +----- +(0 rows) + +DROP TABLE has_oid, no_oid, like_test, like_test2, like_test3, + like_test4, like_test5; diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql index 6dded1fdef..900ca804cb 100644 --- a/src/test/regress/sql/create_table_like.sql +++ b/src/test/regress/sql/create_table_like.sql @@ -131,4 +131,7 @@ CREATE TABLE like_test3 (z INTEGER, LIKE has_oid, LIKE no_oid); SELECT oid FROM like_test3; CREATE TABLE like_test4 (z INTEGER, PRIMARY KEY(oid), LIKE has_oid); SELECT oid FROM like_test4; -DROP TABLE has_oid, no_oid, like_test, like_test2, like_test3, like_test4; +CREATE TABLE like_test5 (z INTEGER, LIKE no_oid) WITH OIDS; +SELECT oid FROM like_test5; +DROP TABLE has_oid, no_oid, like_test, like_test2, like_test3, + like_test4, like_test5;