From 0bd11d9711b88e72d2022e25b9227c480aca4978 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 25 Apr 2015 21:37:39 -0400 Subject: [PATCH] Add comments warning against generalizing default_with_oids. pg_dump has historically assumed that default_with_oids affects only plain tables and not other relkinds. Conceivably we could make it apply to some newly invented relkind if we did so from the get-go, but changing the behavior for existing object types will break existing dump scripts. Add code comments warning about this interaction. Also, make sure that default_with_oids doesn't cause parse_utilcmd.c to think that CREATE FOREIGN TABLE will create an OID column. I think this is only a latent bug right now, since we don't allow UNIQUE/PKEY constraints in CREATE FOREIGN TABLE, but it's better to be consistent and future-proof. --- src/backend/commands/tablecmds.c | 8 ++++++++ src/backend/parser/parse_utilcmd.c | 13 ++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 06e4332d2a..bedd8aeb78 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -579,6 +579,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, */ descriptor = BuildDescForRelation(schema); + /* + * Notice that we allow OIDs here only for plain tables, even though some + * other relkinds can support them. This is necessary because the + * default_with_oids GUC must apply only to plain tables and not any other + * relkind; doing otherwise would break existing pg_dump files. We could + * allow explicit "WITH OIDS" while not allowing default_with_oids to + * affect other relkinds, but it would complicate interpretOidsOption(). + */ localHasOids = interpretOidsOption(stmt->options, (relkind == RELKIND_RELATION)); descriptor->tdhasoid = (localHasOids || parentOidCount > 0); diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 1fc8c2cbe1..73924ae12e 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -222,7 +222,18 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) cxt.blist = NIL; cxt.alist = NIL; cxt.pkey = NULL; - cxt.hasoids = interpretOidsOption(stmt->options, true); + + /* + * Notice that we allow OIDs here only for plain tables, even though + * foreign tables also support them. This is necessary because the + * default_with_oids GUC must apply only to plain tables and not any other + * relkind; doing otherwise would break existing pg_dump files. We could + * allow explicit "WITH OIDS" while not allowing default_with_oids to + * affect other relkinds, but it would complicate interpretOidsOption(), + * and right now there's no WITH OIDS option in CREATE FOREIGN TABLE + * anyway. + */ + cxt.hasoids = interpretOidsOption(stmt->options, !cxt.isforeign); Assert(!stmt->ofTypename || !stmt->inhRelations); /* grammar enforces */