From 98dcf085e35e393bdd6740e6a8f8684b27ac2039 Mon Sep 17 00:00:00 2001 From: Neil Conway Date: Sat, 10 Jan 2004 23:28:45 +0000 Subject: [PATCH] Implement "WITH / WITHOID OIDS" clause for CREATE TABLE AS. This is intended to allow application authors to insulate themselves from changes to the default value of 'default_with_oids' in future releases of PostgreSQL. This patch also fixes a bug in the earlier implementation of the 'default_with_oids' GUC variable: code in gram.y should not examine the value of GUC variables directly due to synchronization issues. --- doc/src/sgml/ref/create_table_as.sgml | 43 +++++++++++++++++------ doc/src/sgml/runtime.sgml | 22 ++++++------ src/backend/commands/sequence.c | 4 +-- src/backend/commands/tablecmds.c | 23 ++++++++++-- src/backend/commands/typecmds.c | 4 +-- src/backend/commands/view.c | 4 +-- src/backend/executor/execMain.c | 11 ++---- src/backend/nodes/copyfuncs.c | 4 ++- src/backend/nodes/equalfuncs.c | 4 ++- src/backend/parser/analyze.c | 18 +++++++++- src/backend/parser/gram.y | 36 ++++++++++++------- src/include/nodes/parsenodes.h | 19 ++++++++-- src/test/regress/expected/without_oid.out | 24 +++++++++++++ src/test/regress/sql/without_oid.sql | 27 ++++++++++++++ 14 files changed, 185 insertions(+), 58 deletions(-) diff --git a/doc/src/sgml/ref/create_table_as.sgml b/doc/src/sgml/ref/create_table_as.sgml index 3bc5668b9f..e406bfa4b5 100644 --- a/doc/src/sgml/ref/create_table_as.sgml +++ b/doc/src/sgml/ref/create_table_as.sgml @@ -1,5 +1,5 @@ @@ -20,7 +20,7 @@ PostgreSQL documentation -CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name [ (column_name [, ...] ) ] +CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name [ (column_name [, ...] ) ] [ [ WITH | WITHOUT ] OIDS ] AS query @@ -98,6 +98,20 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name + + WITH OIDS + WITHOUT OIDS + + + This optional clause specifies whether the table created by + CREATE TABLE AS should include OIDs. If + neither form of this clause if specified, the value of the + default_with_oids configuration parameter is + used. + + + + query @@ -121,23 +135,30 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name This command is functionally similar to , but it is preferred since it is less likely to be confused with other uses of - the SELECT INTO syntax. + the SELECT INTO syntax. Furthermore, CREATE + TABLE AS offers a superset of the functionality offerred + by SELECT INTO. - Prior to PostgreSQL 7.5, CREATE TABLE - AS always included OIDs in the table it + Prior to PostgreSQL 7.5, CREATE + TABLE AS always included OIDs in the table it produced. Furthermore, these OIDs were newly generated: they were distinct from the OIDs of any of the rows in the source tables of the SELECT or EXECUTE statement. Therefore, if CREATE TABLE AS was frequently executed, the OID counter would be rapidly - incremented. As of PostgreSQL 7.5, the inclusion of - OIDs in the table generated by CREATE TABLE AS - is controlled by the default_with_oids - configuration variable. This variable currently defaults to true, - but will likely default to false in a future release of - PostgreSQL. + incremented. As of PostgresSQL 7.5, + the CREATE TABLE AS command allows the user to + explicitely specify whether OIDs should be included. If the + presence of OIDs is not explicitely specified, + the default_with_oids configuration variable is + used. While this variable currently defaults to true, the default + value may be changed in the future. Therefore, applications that + require OIDs in the table created by CREATE TABLE + AS should explicitely specify WITH + OIDS to ensure compatibility with future versions + of PostgreSQL. diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 5ec155d24d..e464e66878 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,5 +1,5 @@ @@ -2437,17 +2437,17 @@ dynamic_library_path = '/usr/local/lib/postgresql:/home/my_project/lib:$libdir' default_with_oids (boolean) - This controls whether CREATE TABLE will - include OIDs in newly-created tables, if neither WITH - OIDS or WITHOUT OIDS have been - specified. It also determines whether OIDs will be included in - the table generated by SELECT INTO and - CREATE TABLE AS. In + This controls whether CREATE TABLE + and CREATE TABLE AS will include OIDs in + newly-created tables, if neither WITH OIDS + or WITHOUT OIDS have been specified. It + also determines whether OIDs will be included in the table + created by SELECT INTO. In PostgreSQL &version; - default_with_oids defaults to true. This is - also the behavior of previous versions of - PostgreSQL. However, assuming that - tables will contain OIDs by default is not + default_with_oids defaults to + true. This is also the behavior of previous versions + of PostgreSQL. However, assuming + that tables will contain OIDs by default is not encouraged. Therefore, this option will default to false in a future release of PostgreSQL. diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 3fcc76a72b..1d487b6d98 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.107 2004/01/07 18:56:25 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.108 2004/01/10 23:28:44 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -178,7 +178,7 @@ DefineSequence(CreateSeqStmt *seq) stmt->relation = seq->sequence; stmt->inhRelations = NIL; stmt->constraints = NIL; - stmt->hasoids = false; + stmt->hasoids = MUST_NOT_HAVE_OIDS; stmt->oncommit = ONCOMMIT_NOOP; seqoid = DefineRelation(stmt, RELKIND_SEQUENCE); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 18330bc466..c16aede78c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.94 2003/11/29 19:51:47 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.95 2004/01/10 23:28:44 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -47,6 +47,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/guc.h" #include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/relcache.h" @@ -185,7 +186,25 @@ DefineRelation(CreateStmt *stmt, char relkind) */ descriptor = BuildDescForRelation(schema); - descriptor->tdhasoid = (stmt->hasoids || parentHasOids); + if (parentHasOids) + descriptor->tdhasoid = true; + else + { + switch (stmt->hasoids) + { + case MUST_HAVE_OIDS: + descriptor->tdhasoid = true; + break; + + case MUST_NOT_HAVE_OIDS: + descriptor->tdhasoid = false; + break; + + case DEFAULT_OIDS: + descriptor->tdhasoid = default_with_oids; + break; + } + } if (old_constraints != NIL) { diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c8321213b3..530fb1f573 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.51 2003/11/29 19:51:47 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.52 2004/01/10 23:28:44 neilc Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -1066,7 +1066,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist) createStmt->tableElts = coldeflist; createStmt->inhRelations = NIL; createStmt->constraints = NIL; - createStmt->hasoids = false; + createStmt->hasoids = MUST_NOT_HAVE_OIDS; createStmt->oncommit = ONCOMMIT_NOOP; /* diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 7dc6658c04..346867bbfc 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.79 2003/11/29 19:51:48 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.80 2004/01/10 23:28:44 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -141,7 +141,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace) createStmt->tableElts = attrList; createStmt->inhRelations = NIL; createStmt->constraints = NIL; - createStmt->hasoids = false; + createStmt->hasoids = MUST_NOT_HAVE_OIDS; createStmt->oncommit = ONCOMMIT_NOOP; /* diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index cf581907a4..9d64c979e0 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -26,7 +26,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.225 2004/01/07 18:56:26 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.226 2004/01/10 23:28:44 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -593,14 +593,7 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly) if (operation == CMD_SELECT && parseTree->into != NULL) { do_select_into = true; - - /* - * The presence of OIDs in the result set of SELECT INTO is - * controlled by the default_with_oids GUC parameter. The - * behavior in versions of PostgreSQL prior to 7.5 is to - * always include OIDs. - */ - estate->es_force_oids = default_with_oids; + estate->es_force_oids = parseTree->intoHasOids; } /* diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index b47c51e462..5a340ebaa9 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -15,7 +15,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.275 2004/01/06 23:55:18 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.276 2004/01/10 23:28:44 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -1549,6 +1549,7 @@ _copyQuery(Query *from) COPY_NODE_FIELD(utilityStmt); COPY_SCALAR_FIELD(resultRelation); COPY_NODE_FIELD(into); + COPY_SCALAR_FIELD(intoHasOids); COPY_SCALAR_FIELD(hasAggs); COPY_SCALAR_FIELD(hasSubLinks); COPY_NODE_FIELD(rtable); @@ -1623,6 +1624,7 @@ _copySelectStmt(SelectStmt *from) COPY_NODE_FIELD(distinctClause); COPY_NODE_FIELD(into); COPY_NODE_FIELD(intoColNames); + COPY_SCALAR_FIELD(intoHasOids); COPY_NODE_FIELD(targetList); COPY_NODE_FIELD(fromClause); COPY_NODE_FIELD(whereClause); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 61e04b1fbd..932d79f31f 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -18,7 +18,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.213 2004/01/06 23:55:18 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.214 2004/01/10 23:28:45 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -604,6 +604,7 @@ _equalQuery(Query *a, Query *b) COMPARE_NODE_FIELD(utilityStmt); COMPARE_SCALAR_FIELD(resultRelation); COMPARE_NODE_FIELD(into); + COMPARE_SCALAR_FIELD(intoHasOids); COMPARE_SCALAR_FIELD(hasAggs); COMPARE_SCALAR_FIELD(hasSubLinks); COMPARE_NODE_FIELD(rtable); @@ -667,6 +668,7 @@ _equalSelectStmt(SelectStmt *a, SelectStmt *b) COMPARE_NODE_FIELD(distinctClause); COMPARE_NODE_FIELD(into); COMPARE_NODE_FIELD(intoColNames); + COMPARE_SCALAR_FIELD(intoHasOids); COMPARE_NODE_FIELD(targetList); COMPARE_NODE_FIELD(fromClause); COMPARE_NODE_FIELD(whereClause); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index a97606b05d..bc70cca429 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.293 2004/01/05 20:58:58 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.294 2004/01/10 23:28:45 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -41,6 +41,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/relcache.h" #include "utils/syscache.h" @@ -1974,6 +1975,21 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) if (stmt->intoColNames) applyColumnNames(qry->targetList, stmt->intoColNames); + switch (stmt->intoHasOids) + { + case MUST_HAVE_OIDS: + qry->intoHasOids = true; + break; + + case MUST_NOT_HAVE_OIDS: + qry->intoHasOids = false; + break; + + case DEFAULT_OIDS: + qry->intoHasOids = default_with_oids; + break; + } + /* mark column origins */ markTargetListOrigins(pstate, qry->targetList); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index cd8c092a45..535da4f2b5 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.444 2004/01/10 02:21:08 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.445 2004/01/10 23:28:45 neilc Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -63,7 +63,6 @@ #include "utils/numeric.h" #include "utils/datetime.h" #include "utils/date.h" -#include "utils/guc.h" extern List *parsetree; /* final parse result is delivered here */ @@ -109,6 +108,7 @@ static void doNegateFloat(Value *v); JoinType jtype; DropBehavior dbehavior; OnCommitAction oncommit; + ContainsOids withoids; List *list; FastList fastlist; Node *node; @@ -235,8 +235,9 @@ static void doNegateFloat(Value *v); %type func_arg %type func_return func_type aggr_argtype -%type arg_class TriggerForType OptTemp OptWithOids -%type OnCommitOption +%type arg_class TriggerForType OptTemp +%type OnCommitOption +%type OptWithOids WithOidsAs %type for_update_clause opt_for_update_clause update_list %type opt_all @@ -1824,14 +1825,9 @@ OptInherit: INHERITS '(' qualified_name_list ')' { $$ = $3; } ; OptWithOids: - WITH OIDS { $$ = TRUE; } - | WITHOUT OIDS { $$ = FALSE; } - /* - * If the user didn't explicitely specify WITH or WITHOUT - * OIDS, decide whether to include OIDs based on the - * "default_with_oids" GUC var - */ - | /*EMPTY*/ { $$ = default_with_oids; } + WITH OIDS { $$ = MUST_HAVE_OIDS; } + | WITHOUT OIDS { $$ = MUST_NOT_HAVE_OIDS; } + | /*EMPTY*/ { $$ = DEFAULT_OIDS; } ; OnCommitOption: ON COMMIT DROP { $$ = ONCOMMIT_DROP; } @@ -1847,7 +1843,7 @@ OnCommitOption: ON COMMIT DROP { $$ = ONCOMMIT_DROP; } */ CreateAsStmt: - CREATE OptTemp TABLE qualified_name OptCreateAs AS SelectStmt + CREATE OptTemp TABLE qualified_name OptCreateAs WithOidsAs SelectStmt { /* * When the SelectStmt is a set-operation tree, we must @@ -1864,10 +1860,23 @@ CreateAsStmt: $4->istemp = $2; n->into = $4; n->intoColNames = $5; + n->intoHasOids = $6; $$ = $7; } ; +/* + * To avoid a shift/reduce conflict in CreateAsStmt, we need to + * include the 'AS' terminal in the parsing of WITH/WITHOUT + * OIDS. Unfortunately that means this production is effectively a + * duplicate of OptWithOids. + */ +WithOidsAs: + WITH OIDS AS { $$ = MUST_HAVE_OIDS; } + | WITHOUT OIDS AS { $$ = MUST_NOT_HAVE_OIDS; } + | AS { $$ = DEFAULT_OIDS; } + ; + OptCreateAs: '(' CreateAsList ')' { $$ = $2; } | /*EMPTY*/ { $$ = NIL; } @@ -4531,6 +4540,7 @@ simple_select: n->targetList = $3; n->into = $4; n->intoColNames = NIL; + n->intoHasOids = DEFAULT_OIDS; n->fromClause = $5; n->whereClause = $6; n->groupClause = $7; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 9e6e66a337..8b6446d860 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.251 2004/01/06 23:55:19 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.252 2004/01/10 23:28:45 neilc Exp $ * *------------------------------------------------------------------------- */ @@ -55,6 +55,7 @@ typedef struct Query int resultRelation; /* target relation (index into rtable) */ RangeVar *into; /* target relation for SELECT INTO */ + bool intoHasOids; /* should target relation contain OIDs? */ bool hasAggs; /* has aggregates in tlist or havingQual */ bool hasSubLinks; /* has subquery SubLink */ @@ -595,6 +596,16 @@ typedef enum SetOperation SETOP_EXCEPT } SetOperation; +typedef enum ContainsOids +{ + MUST_HAVE_OIDS, /* WITH OIDS explicitely specified */ + MUST_NOT_HAVE_OIDS, /* WITHOUT OIDS explicitely specified */ + DEFAULT_OIDS /* neither specified; use the default, + * which is the value of the + * default_with_oids GUC var + */ +} ContainsOids; + typedef struct SelectStmt { NodeTag type; @@ -602,13 +613,15 @@ typedef struct SelectStmt /* * These fields are used only in "leaf" SelectStmts. * - * into and intoColNames are a kluge; they belong somewhere else... + * into, intoColNames and intoHasOids are a kluge; they belong + * somewhere else... */ List *distinctClause; /* NULL, list of DISTINCT ON exprs, or * lcons(NIL,NIL) for all (SELECT * DISTINCT) */ RangeVar *into; /* target table (for select into table) */ List *intoColNames; /* column names for into table */ + ContainsOids intoHasOids; /* should target table have OIDs? */ List *targetList; /* the target list (of ResTarget) */ List *fromClause; /* the FROM clause */ Node *whereClause; /* WHERE qualification */ @@ -895,7 +908,7 @@ typedef struct CreateStmt List *inhRelations; /* relations to inherit from (list of * inhRelation) */ List *constraints; /* constraints (list of Constraint nodes) */ - bool hasoids; /* should it have OIDs? */ + ContainsOids hasoids; /* should it have OIDs? */ OnCommitAction oncommit; /* what do we do at COMMIT? */ } CreateStmt; diff --git a/src/test/regress/expected/without_oid.out b/src/test/regress/expected/without_oid.out index 23a89a06c5..ef373e6e3c 100644 --- a/src/test/regress/expected/without_oid.out +++ b/src/test/regress/expected/without_oid.out @@ -36,3 +36,27 @@ SELECT min(relpages) < max(relpages), min(reltuples) - max(reltuples) DROP TABLE wi; DROP TABLE wo; +-- +-- WITH / WITHOUT OIDS in CREATE TABLE AS +-- +CREATE TABLE create_table_test ( + a int, + b int +); +COPY create_table_test FROM stdin; +CREATE TABLE create_table_test2 WITH OIDS AS + SELECT a + b AS c1, a - b AS c2 FROM create_table_test; +CREATE TABLE create_table_test3 WITHOUT OIDS AS + SELECT a + b AS c1, a - b AS c2 FROM create_table_test; +SELECT count(oid) FROM create_table_test2; + count +------- + 2 +(1 row) + +-- should fail +SELECT count(oid) FROM create_table_test3; +ERROR: column "oid" does not exist +DROP TABLE create_table_test; +DROP TABLE create_table_test2; +DROP TABLE create_table_test3; diff --git a/src/test/regress/sql/without_oid.sql b/src/test/regress/sql/without_oid.sql index 727e743d02..4cb961941a 100644 --- a/src/test/regress/sql/without_oid.sql +++ b/src/test/regress/sql/without_oid.sql @@ -33,3 +33,30 @@ SELECT min(relpages) < max(relpages), min(reltuples) - max(reltuples) DROP TABLE wi; DROP TABLE wo; + +-- +-- WITH / WITHOUT OIDS in CREATE TABLE AS +-- +CREATE TABLE create_table_test ( + a int, + b int +); + +COPY create_table_test FROM stdin; +5 10 +10 15 +\. + +CREATE TABLE create_table_test2 WITH OIDS AS + SELECT a + b AS c1, a - b AS c2 FROM create_table_test; + +CREATE TABLE create_table_test3 WITHOUT OIDS AS + SELECT a + b AS c1, a - b AS c2 FROM create_table_test; + +SELECT count(oid) FROM create_table_test2; +-- should fail +SELECT count(oid) FROM create_table_test3; + +DROP TABLE create_table_test; +DROP TABLE create_table_test2; +DROP TABLE create_table_test3;