mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Revert "Don't try to re-order the subcommands of CREATE SCHEMA".
This reverts commita9c350d9ee. While that intentionally changed our semantics for CREATE SCHEMA subcommands, it's being argued that the functionality gain does not justify potentially-subtle compatibility breakage. The most useful bit of functionality gain came from commit404db8f9e("Execute foreign key constraints in CREATE SCHEMA at the end"), which we're keeping because it's required by SQL spec and doesn't seem to create a compatibility hazard by itself. This is not an exact revert, partly because we're keeping404db8f9e, and partly because I kept the API changes that allowed passing down a ParseState (which allows providing an error cursor for many of the errors thrown in CREATE SCHEMA). Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20260904180712.cc.noahmisch@microsoft.com Backpatch-through: 19
This commit is contained in:
@@ -131,14 +131,6 @@ CREATE SCHEMA IF NOT EXISTS AUTHORIZATION <replaceable class="parameter">role_sp
|
||||
<literal>CREATE</literal> privilege for the current database.
|
||||
(Of course, superusers bypass this check.)
|
||||
</para>
|
||||
|
||||
<para>
|
||||
The <replaceable class="parameter">schema_element</replaceable>
|
||||
subcommands, if any, are executed in the order they are written.
|
||||
An exception is that foreign key constraint clauses in <command>CREATE
|
||||
TABLE</command> subcommands are postponed and added at the end.
|
||||
This allows circular foreign key references, which are sometimes useful.
|
||||
</para>
|
||||
</refsect1>
|
||||
|
||||
<refsect1>
|
||||
@@ -201,12 +193,15 @@ CREATE VIEW hollywood.winners AS
|
||||
</para>
|
||||
|
||||
<para>
|
||||
Some other SQL implementations attempt to allow more kinds of forward
|
||||
references to objects defined in
|
||||
later <replaceable class="parameter">schema_element</replaceable>
|
||||
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 <command>CREATE SCHEMA</command> can appear in any
|
||||
order; but the only actual requirement it gives is that foreign keys
|
||||
referencing table definitions that appear in later <command>CREATE
|
||||
SCHEMA</command> subcommands should be accepted. The
|
||||
present <productname>PostgreSQL</productname> 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.
|
||||
</para>
|
||||
|
||||
<para>
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
@@ -4096,17 +4109,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
|
||||
@@ -4124,17 +4137,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))
|
||||
{
|
||||
@@ -4142,8 +4166,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;
|
||||
|
||||
@@ -4151,12 +4175,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;
|
||||
|
||||
@@ -4164,8 +4192,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;
|
||||
|
||||
@@ -4173,8 +4205,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;
|
||||
|
||||
@@ -4182,13 +4214,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:
|
||||
@@ -4197,7 +4229,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;
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -4212,17 +4253,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)
|
||||
{
|
||||
@@ -4230,7 +4270,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)));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -582,6 +582,7 @@ CreateRangeStmt
|
||||
CreateReplicationSlotCmd
|
||||
CreateRoleStmt
|
||||
CreateSchemaStmt
|
||||
CreateSchemaStmtContext
|
||||
CreateSeqStmt
|
||||
CreateStatsStmt
|
||||
CreateStmt
|
||||
|
||||
Reference in New Issue
Block a user