diff --git a/doc/src/sgml/ref/create_schema.sgml b/doc/src/sgml/ref/create_schema.sgml index 96bc496e777..31c381a2b7a 100644 --- a/doc/src/sgml/ref/create_schema.sgml +++ b/doc/src/sgml/ref/create_schema.sgml @@ -131,14 +131,6 @@ CREATE SCHEMA IF NOT EXISTS AUTHORIZATION role_sp CREATE privilege for the current database. (Of course, superusers bypass this check.) - - - The schema_element - subcommands, if any, are executed in the order they are written. - An exception is that foreign key constraint clauses in CREATE - TABLE subcommands are postponed and added at the end. - This allows circular foreign key references, which are sometimes useful. - @@ -201,12 +193,15 @@ CREATE VIEW hollywood.winners AS - Some other SQL implementations attempt to allow more kinds of forward - references to objects defined in - later schema_element - subcommands than just foreign key constraints. This is difficult or - impossible to do correctly in general, and it is not clear that the SQL - standard requires any such behavior except for foreign keys. + The SQL standard suggests, without saying so in so many words, that the + subcommands in CREATE SCHEMA can appear in any + order; but the only actual requirement it gives is that foreign keys + referencing table definitions that appear in later CREATE + SCHEMA subcommands should be accepted. The + present PostgreSQL implementation handles + that case, but does not handle all other cases of forward references in + subcommands; it might sometimes be necessary to reorder the subcommands + in order to avoid forward references. diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index de5bbd5662c..b016983f904 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -189,9 +189,10 @@ CreateSchemaCommand(ParseState *pstate, CreateSchemaStmt *stmt, /* * Examine the list of commands embedded in the CREATE SCHEMA command, and - * do preliminary transformations. Note that the result is still a list - * of raw parsetrees --- we cannot, in general, run parse analysis on one - * statement until we have actually executed the prior ones. + * reorganize them into a sequentially executable order with no forward + * references. Note that the result is still a list of raw parsetrees --- + * we cannot, in general, run parse analysis on one statement until we + * have actually executed the prior ones. */ parsetree_list = transformCreateSchemaStmtElements(pstate, stmt->schemaElts, diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index c532a49b21d..f838311090b 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -96,6 +96,20 @@ typedef struct bool ofType; /* true if statement contains OF typename */ } CreateStmtContext; +/* State shared by transformCreateSchemaStmtElements and its subroutines */ +typedef struct +{ + ParseState *pstate; /* overall parse state */ + const char *schemaname; /* name of schema */ + List *sequences; /* CREATE SEQUENCE items */ + List *tables; /* CREATE TABLE items */ + List *views; /* CREATE VIEW items */ + List *indexes; /* CREATE INDEX items */ + List *triggers; /* CREATE TRIGGER items */ + List *grants; /* GRANT items */ + List *foreign_keys; /* generated ALTER ADD FOREIGN KEY items */ +} CreateSchemaStmtContext; + static void transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column); @@ -122,8 +136,7 @@ static void transformCheckConstraints(CreateStmtContext *cxt, static void transformConstraintAttrs(ParseState *pstate, List *constraintList); static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column); -static void checkSchemaNameRV(ParseState *pstate, const char *context_schema, - RangeVar *relation); +static void checkSchemaNameRV(CreateSchemaStmtContext *cxt, RangeVar *relation); static CreateStmt *transformCreateSchemaCreateTable(ParseState *pstate, CreateStmt *stmt, List **fk_elements); @@ -4112,17 +4125,17 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column) * transformCreateSchemaStmtElements - * analyzes the elements of a CREATE SCHEMA statement * - * This presently has two responsibilities. We verify that no subcommands are - * trying to create objects outside the new schema. We also pull out any - * foreign-key constraint clauses embedded in CREATE TABLE subcommands, and - * convert them to ALTER TABLE ADD CONSTRAINT commands appended to the list. - * This supports forward references in foreign keys, which is required by the - * SQL standard. - * - * We used to try to re-order the commands in a way that would work even if - * the user-written order would not, but that's too hard (perhaps impossible) - * to do correctly with not-yet-parse-analyzed commands. Now we'll just - * execute the elements in the order given, except for foreign keys. + * This presently has two responsibilities. We verify that no subcommands + * are trying to create objects outside the new schema. We also attempt to + * re-order the subcommands such that there are no forward references + * (e.g. GRANT to a table created later in the list). Note that the logic + * we use for determining forward references is presently quite incomplete, + * and it's unlikely that we can do significantly better while working with + * non-parse-analyzed commands. The only case that the SQL standard calls + * out as required is to support forward references in foreign-key constraint + * clauses in CREATE TABLE subcommands. We do handle that, by pulling out + * such clauses and converting them to ALTER TABLE ADD CONSTRAINT commands + * appended to the list. * * "schemaName" is the name of the schema that will be used for the creation * of the objects listed. It may be obtained from the schema name defined @@ -4140,17 +4153,28 @@ List * transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, const char *schemaName) { - List *elements = NIL; - List *fk_elements = NIL; - ListCell *lc; + CreateSchemaStmtContext cxt; + List *result; + ListCell *elements; + + cxt.pstate = pstate; + cxt.schemaname = schemaName; + cxt.sequences = NIL; + cxt.tables = NIL; + cxt.views = NIL; + cxt.indexes = NIL; + cxt.triggers = NIL; + cxt.grants = NIL; + cxt.foreign_keys = NIL; /* * Run through each schema element in the schema element list. Check - * target schema names, and collect the list of actions to be done. + * target schema names, separate statements by type, and do preliminary + * analysis. */ - foreach(lc, schemaElts) + foreach(elements, schemaElts) { - Node *element = lfirst(lc); + Node *element = lfirst(elements); switch (nodeTag(element)) { @@ -4158,8 +4182,8 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, { CreateSeqStmt *elp = (CreateSeqStmt *) element; - checkSchemaNameRV(pstate, schemaName, elp->sequence); - elements = lappend(elements, element); + checkSchemaNameRV(&cxt, elp->sequence); + cxt.sequences = lappend(cxt.sequences, element); } break; @@ -4167,12 +4191,16 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, { CreateStmt *elp = (CreateStmt *) element; - checkSchemaNameRV(pstate, schemaName, elp->relation); + checkSchemaNameRV(&cxt, elp->relation); /* Pull out any foreign key clauses, add to fk_elements */ elp = transformCreateSchemaCreateTable(pstate, elp, - &fk_elements); - elements = lappend(elements, elp); + &cxt.foreign_keys); + + /* + * XXX todo: deal with other constraints + */ + cxt.tables = lappend(cxt.tables, elp); } break; @@ -4180,8 +4208,12 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, { ViewStmt *elp = (ViewStmt *) element; - checkSchemaNameRV(pstate, schemaName, elp->view); - elements = lappend(elements, element); + checkSchemaNameRV(&cxt, elp->view); + + /* + * XXX todo: deal with references between views + */ + cxt.views = lappend(cxt.views, element); } break; @@ -4189,8 +4221,8 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, { IndexStmt *elp = (IndexStmt *) element; - checkSchemaNameRV(pstate, schemaName, elp->relation); - elements = lappend(elements, element); + checkSchemaNameRV(&cxt, elp->relation); + cxt.indexes = lappend(cxt.indexes, element); } break; @@ -4198,13 +4230,13 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, { CreateTrigStmt *elp = (CreateTrigStmt *) element; - checkSchemaNameRV(pstate, schemaName, elp->relation); - elements = lappend(elements, element); + checkSchemaNameRV(&cxt, elp->relation); + cxt.triggers = lappend(cxt.triggers, element); } break; case T_GrantStmt: - elements = lappend(elements, element); + cxt.grants = lappend(cxt.grants, element); break; default: @@ -4213,7 +4245,16 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, } } - return list_concat(elements, fk_elements); + result = NIL; + result = list_concat(result, cxt.sequences); + result = list_concat(result, cxt.tables); + result = list_concat(result, cxt.views); + result = list_concat(result, cxt.indexes); + result = list_concat(result, cxt.triggers); + result = list_concat(result, cxt.grants); + result = list_concat(result, cxt.foreign_keys); + + return result; } /* @@ -4228,17 +4269,16 @@ transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts, * that would likewise put the object into the wrong schema. */ static void -checkSchemaNameRV(ParseState *pstate, const char *context_schema, - RangeVar *relation) +checkSchemaNameRV(CreateSchemaStmtContext *cxt, RangeVar *relation) { if (relation->schemaname != NULL && - strcmp(context_schema, relation->schemaname) != 0) + strcmp(cxt->schemaname, relation->schemaname) != 0) ereport(ERROR, (errcode(ERRCODE_INVALID_SCHEMA_DEFINITION), errmsg("CREATE specifies a schema (%s) " "different from the one being created (%s)", - relation->schemaname, context_schema), - parser_errposition(pstate, relation->location))); + relation->schemaname, cxt->schemaname), + parser_errposition(cxt->pstate, relation->location))); if (relation->relpersistence == RELPERSISTENCE_TEMP) { @@ -4246,7 +4286,7 @@ checkSchemaNameRV(ParseState *pstate, const char *context_schema, ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot create temporary relation in non-temporary schema"), - parser_errposition(pstate, relation->location))); + parser_errposition(cxt->pstate, relation->location))); } } diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out index b34b9988962..7a9b71f97b3 100644 --- a/src/test/regress/expected/create_schema.out +++ b/src/test/regress/expected/create_schema.out @@ -88,13 +88,6 @@ ERROR: CREATE specifies a schema (schema_not_existing) different from the one b LINE 2: CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_exi... ^ RESET ROLE; --- Forward references no longer work in general. -CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE - CREATE VIEW abcd_view AS SELECT a FROM abcd - CREATE TABLE abcd (a int); -ERROR: relation "abcd" does not exist -LINE 2: CREATE VIEW abcd_view AS SELECT a FROM abcd - ^ -- Cases where the schema creation succeeds. -- The schema created matches the role name. CREATE SCHEMA AUTHORIZATION regress_create_schema_role diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 065f586310f..f57e8ffa7a5 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -425,12 +425,12 @@ NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.one NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_pkey NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_a_seq NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_c_seq -NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_idx NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.two NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.id_col_d_seq NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.id NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.id_col_d_seq NOTICE: END: command_tag=CREATE VIEW type=view identity=evttrig.one_view +NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_idx NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.two -- View with column additions CREATE OR REPLACE VIEW evttrig.one_view AS SELECT * FROM evttrig.two, evttrig.id; diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out index 2e582e783c2..dbbda72d395 100644 --- a/src/test/regress/expected/namespace.out +++ b/src/test/regress/expected/namespace.out @@ -10,14 +10,13 @@ SELECT pg_catalog.set_config('search_path', ' ', false); (1 row) CREATE SCHEMA test_ns_schema_1 - CREATE TABLE abc ( - a serial, - b int UNIQUE - ) CREATE UNIQUE INDEX abc_a_idx ON abc (a) CREATE VIEW abc_view AS SELECT a+1 AS a, b+1 AS b FROM abc -; + CREATE TABLE abc ( + a serial, + b int UNIQUE + ); -- verify that the correct search_path restored on abort SET search_path to public; BEGIN; diff --git a/src/test/regress/sql/create_schema.sql b/src/test/regress/sql/create_schema.sql index 0f2accc59ec..57014ac4930 100644 --- a/src/test/regress/sql/create_schema.sql +++ b/src/test/regress/sql/create_schema.sql @@ -47,11 +47,6 @@ CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE EXECUTE FUNCTION schema_trig.no_func(); RESET ROLE; --- Forward references no longer work in general. -CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE - CREATE VIEW abcd_view AS SELECT a FROM abcd - CREATE TABLE abcd (a int); - -- Cases where the schema creation succeeds. -- The schema created matches the role name. CREATE SCHEMA AUTHORIZATION regress_create_schema_role diff --git a/src/test/regress/sql/namespace.sql b/src/test/regress/sql/namespace.sql index a75d4f580d3..306cdc2d8c6 100644 --- a/src/test/regress/sql/namespace.sql +++ b/src/test/regress/sql/namespace.sql @@ -7,16 +7,15 @@ SELECT pg_catalog.set_config('search_path', ' ', false); CREATE SCHEMA test_ns_schema_1 - CREATE TABLE abc ( - a serial, - b int UNIQUE - ) - CREATE UNIQUE INDEX abc_a_idx ON abc (a) CREATE VIEW abc_view AS SELECT a+1 AS a, b+1 AS b FROM abc -; + + CREATE TABLE abc ( + a serial, + b int UNIQUE + ); -- verify that the correct search_path restored on abort SET search_path to public; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index da868c3b47f..cdf5a7d0b11 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -587,6 +587,7 @@ CreateRangeStmt CreateReplicationSlotCmd CreateRoleStmt CreateSchemaStmt +CreateSchemaStmtContext CreateSeqStmt CreateStatsStmt CreateStmt