Thread: Re: CREATE SCHEMA ... CREATE DOMAIN support
Kirill Reshke <reshkekirill@gmail.com> writes:
> On Wed, 27 Nov 2024 at 08:42, jian he <jian.universality@gmail.com> wrote:
>> CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
>> create domain ss1 as ss
>> create domain ss as text;
>> ERROR: type "ss" does not exist
>>
>> the error message seems not that OK,
>> if we can point out the error position, that would be great.
> To implement this, we need to include `ParseLoc location` to the
> `CreateDomainStmt` struct, which is doubtful, because I don't see any
> other type of create *something* that does this.
No, that error is thrown from typenameType(), which has a perfectly
good location in the TypeName. What it's lacking is a ParseState
containing the source query string.
Breakpoint 1, typenameType (pstate=pstate@entry=0x0, typeName=0x25d6b58,
typmod_p=typmod_p@entry=0x7ffe7dcd641c) at parse_type.c:268
268 tup = LookupTypeName(pstate, typeName, typmod_p, false);
(gdb) p pstate
$2 = (ParseState *) 0x0
(gdb) p typeName->location
$3 = 21
We've fixed a few utility statements so that they can receive
a passed-down ParseState, but not DefineDomain.
regards, tom lane
Kirill Reshke <reshkekirill@gmail.com> writes:
> On Wed, 27 Nov 2024 at 23:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> We've fixed a few utility statements so that they can receive
>> a passed-down ParseState, but not DefineDomain.
> PFA as an independent patch then. Or should we combine these two into one?
No, I don't think this should be part of the patch discussed in this
thread.
It feels rather random to me to be fixing only DefineDomain;
I'm sure there's more in the same vein. I'd like to see a
patch with a scope along the lines of "fix everything reachable
within CREATE SCHEMA" or perhaps "fix all calls of typenameType".
(A quick grep shows that an outright majority of the callers of that
are passing null ParseState. I didn't look to see if any of them
have a good excuse beyond "we didn't do the plumbing work".)
regards, tom lane
On Thu, 28 Nov 2024 at 10:52, Tom Lane <tgl@sss.pgh.pa.us> wrote: > No, I don't think this should be part of the patch discussed in this > thread. Ok, I created a separate thread for this. How about this one? Do you think the suggested idea is good? Is it worthwhile to do this, in your opinion? -- Best regards, Kirill Reshke
On Fri, 29 Nov 2024 at 18:47, jian he <jian.universality@gmail.com> wrote: > > new patch, add tab complete for it. Thank you. You may also be interested in reviewing [0]. [0] https://www.postgresql.org/message-id/CALdSSPhqfvKbDwqJaY%3DyEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg%40mail.gmail.com -- Best regards, Kirill Reshke
[ Looping in Peter E. for commentary on SQL-spec compatibility ] I spent some time looking at this patch, and came away with two main thoughts: 1. It doesn't make any sense to me to support CREATE DOMAIN within CREATE SCHEMA but not any of our other commands for creating types. It's not a consistent feature this way, and there's no support for it in the SQL standard either, because the spec calls out both <domain definition> and <user-defined type definition> as permissible schema elements. So I think we need a bit more ambition in the scope of the patch: it should allow every variant of CREATE TYPE too. (Since the spec also lists <schema routine>, I'd personally like to see us cover functions/procedures as well as types. But functions could be left for later I guess.) 2. transformCreateSchemaStmtElements is of the opinion that it's responsible for ordering the schema elements in a way that will work, but it just about completely fails at that task. Ordering the objects by kind is surely not sufficient, and adding CREATE DOMAIN will make that worse. (Example: a domain could be used in a table definition, but we also allow domains to be created over tables' composite types.) Yet we have no infrastructure that would allow us to discover the real dependencies between unparsed DDL commands, nor is it likely that anyone will ever undertake building such. I think we ought to nuke that concept from orbit and just execute the schema elements in the order presented. I looked at several iterations of the SQL standard and cannot find any support for the idea that CREATE SCHEMA needs to be any smarter than that. I'd also argue that doing anything else is a POLA violation. It's especially a POLA violation if the code rearranges a valid user-written command order into an invalid order, which is inevitable if we stick with the current approach. The notion that we ought to sort the objects by kind appears to go all the way back to 95ef6a344 of 2002-03-21, which I guess makes it my fault. There must have been some prior mailing list discussion, but I couldn't find much. There is a predecessor of the committed patch in https://www.postgresql.org/message-id/flat/3C7F8A49.CC4EF0BE%40redhat.com but no discussion of why sorting by kind is a good idea. (The last message in the thread suggests that there was more discussion among the Red Hat RHDB team, but if so it's lost to history now.) Thoughts? regards, tom lane
I wrote:
> 2. transformCreateSchemaStmtElements is of the opinion that it's
> responsible for ordering the schema elements in a way that will work,
> but it just about completely fails at that task. Ordering the objects
> by kind is surely not sufficient, and adding CREATE DOMAIN will make
> that worse. (Example: a domain could be used in a table definition,
> but we also allow domains to be created over tables' composite types.)
> Yet we have no infrastructure that would allow us to discover the real
> dependencies between unparsed DDL commands, nor is it likely that
> anyone will ever undertake building such. I think we ought to nuke
> that concept from orbit and just execute the schema elements in the
> order presented. I looked at several iterations of the SQL standard
> and cannot find any support for the idea that CREATE SCHEMA needs to
> be any smarter than that. I'd also argue that doing anything else is
> a POLA violation. It's especially a POLA violation if the code
> rearranges a valid user-written command order into an invalid order,
> which is inevitable if we stick with the current approach.
Further to this: I don't think "re-order into a safe order" is even
a well-defined requirement. What should happen with
CREATE VIEW public.v1 AS SELECT * FROM foo;
CREATE SCHEMA s1
CREATE VIEW v0 AS SELECT * FROM v1;
CREATE VIEW v1 AS SELECT * FROM bar;
If we re-order the CREATE VIEW subcommands, this means something
different than if we don't. Maybe the user meant us to re-order,
but it's hardly an open-and-shut argument. And historically we
have not re-ordered CREATE VIEW subcommands, so there's a hazard
of compatibility problems if we did ever try to do that.
So attached is a draft patch that simplifies the rule to "do the
subcommands in the order written". I took the opportunity to clean up
some other small infelicities about transformCreateSchemaStmtElements,
too.
regards, tom lane
From 14e47a5b64fb115928630613d9ab0ae2d6e05eec Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 30 Nov 2024 18:11:04 -0500
Subject: [PATCH v1] Don't try to re-order the subcommands of CREATE SCHEMA.
transformCreateSchemaStmtElements has always believed that it is
supposed to re-order the subcommands of CREATE SCHEMA into a safe
execution order. However, it is nowhere near being capable of doing
that correctly. Nor is there reason to think that it ever will be,
or that that is a well-defined requirement, or that there's any basis
in the SQL standard for it. Moreover, the problem will get worse as
we add more subcommand types. Let's just drop the whole idea and
execute the commands in the order given, which seems like a much less
astonishment-prone definition anyway.
Along the way, pass down a ParseState so that we can provide an
error cursor for the "wrong schema name" error, and fix
transformCreateSchemaStmtElements so that it doesn't scribble
on the parsetree passed to it.
Discussion: https://postgr.es/m/1075425.1732993688@sss.pgh.pa.us
---
doc/src/sgml/ref/create_schema.sgml | 10 +-
src/backend/commands/extension.c | 9 +-
src/backend/commands/schemacmds.c | 22 ++--
src/backend/parser/parse_utilcmd.c | 130 ++++++++------------
src/backend/tcop/utility.c | 7 +-
src/include/commands/schemacmds.h | 7 +-
src/include/parser/parse_utilcmd.h | 3 +-
src/test/regress/expected/create_schema.out | 30 +++++
src/test/regress/expected/event_trigger.out | 2 +-
src/test/regress/expected/namespace.out | 9 +-
src/test/regress/sql/namespace.sql | 12 +-
11 files changed, 123 insertions(+), 118 deletions(-)
diff --git a/doc/src/sgml/ref/create_schema.sgml b/doc/src/sgml/ref/create_schema.sgml
index ed69298ccc..625793a6b6 100644
--- a/doc/src/sgml/ref/create_schema.sgml
+++ b/doc/src/sgml/ref/create_schema.sgml
@@ -193,12 +193,10 @@ CREATE VIEW hollywood.winners AS
</para>
<para>
- The SQL standard specifies that the subcommands in <command>CREATE
- SCHEMA</command> can appear in any order. The present
- <productname>PostgreSQL</productname> implementation does not
- handle all cases of forward references in subcommands; it might
- sometimes be necessary to reorder the subcommands in order to avoid
- forward references.
+ <productname>PostgreSQL</productname> executes the subcommands
+ in <command>CREATE SCHEMA</command> in the order given. Other
+ implementations may try to rearrange the subcommands into dependency
+ order, but that is hard if not impossible to do correctly.
</para>
<para>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index af6bd8ff42..a2eb42dc7f 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1687,14 +1687,19 @@ CreateExtensionInternal(char *extensionName,
if (!OidIsValid(schemaOid))
{
+ ParseState *pstate = make_parsestate(NULL);
CreateSchemaStmt *csstmt = makeNode(CreateSchemaStmt);
+ pstate->p_sourcetext = "(generated CREATE SCHEMA command)";
+ pstate->p_stmt_location = -1;
+ pstate->p_stmt_len = -1;
+
csstmt->schemaname = schemaName;
csstmt->authrole = NULL; /* will be created by current user */
csstmt->schemaElts = NIL;
csstmt->if_not_exists = false;
- CreateSchemaCommand(csstmt, "(generated CREATE SCHEMA command)",
- -1, -1);
+
+ CreateSchemaCommand(pstate, csstmt);
/*
* CreateSchemaCommand includes CommandCounterIncrement, so new
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 233f8ad1d4..4470b9a402 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -42,15 +42,14 @@ static void AlterSchemaOwner_internal(HeapTuple tup, Relation rel, Oid newOwnerI
/*
* CREATE SCHEMA
*
- * Note: caller should pass in location information for the whole
+ * Note: pstate should pass in location information for the whole
* CREATE SCHEMA statement, which in turn we pass down as the location
* of the component commands. This comports with our general plan of
* reporting location/len for the whole command even when executing
* a subquery.
*/
Oid
-CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
- int stmt_location, int stmt_len)
+CreateSchemaCommand(ParseState *pstate, CreateSchemaStmt *stmt)
{
const char *schemaName = stmt->schemaname;
Oid namespaceId;
@@ -189,12 +188,13 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
/*
* Examine the list of commands embedded in the CREATE SCHEMA command, and
- * 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.
+ * do preliminary transformations (mostly, verify that none are trying to
+ * create objects outside the new schema). 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(stmt->schemaElts,
+ parsetree_list = transformCreateSchemaStmtElements(pstate,
+ stmt->schemaElts,
schemaName);
/*
@@ -213,12 +213,12 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
wrapper->commandType = CMD_UTILITY;
wrapper->canSetTag = false;
wrapper->utilityStmt = stmt;
- wrapper->stmt_location = stmt_location;
- wrapper->stmt_len = stmt_len;
+ wrapper->stmt_location = pstate->p_stmt_location;
+ wrapper->stmt_len = pstate->p_stmt_len;
/* do this step */
ProcessUtility(wrapper,
- queryString,
+ pstate->p_sourcetext,
false,
PROCESS_UTILITY_SUBCOMMAND,
NULL,
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 0f324ee4e3..70993637ad 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -95,18 +95,6 @@ typedef struct
bool ofType; /* true if statement contains OF typename */
} CreateStmtContext;
-/* State shared by transformCreateSchemaStmtElements and its subroutines */
-typedef struct
-{
- 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 */
-} CreateSchemaStmtContext;
-
static void transformColumnDefinition(CreateStmtContext *cxt,
ColumnDef *column);
@@ -133,7 +121,8 @@ static void transformCheckConstraints(CreateStmtContext *cxt,
static void transformConstraintAttrs(CreateStmtContext *cxt,
List *constraintList);
static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column);
-static void setSchemaName(const char *context_schema, char **stmt_schema_name);
+static void checkSchemaName(ParseState *pstate, const char *context_schema,
+ RangeVar *relation);
static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
static List *transformPartitionRangeBounds(ParseState *pstate, List *blist,
Relation parent);
@@ -4002,51 +3991,35 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column)
* transformCreateSchemaStmtElements -
* analyzes the elements of a CREATE SCHEMA statement
*
- * Split the schema element list from a CREATE SCHEMA statement into
- * individual commands and place them in the result list in an order
- * 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.
+ * This is now somewhat vestigial: its only real responsibility is to complain
+ * if any of the elements are trying to create objects outside the new schema.
+ * 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.
*
* "schemaName" is the name of the schema that will be used for the creation
- * of the objects listed, that may be compiled from the schema name defined
+ * of the objects listed. It may be obtained from the schema name defined
* in the statement or a role specification.
*
- * SQL also allows constraints to make forward references, so thumb through
- * the table columns and move forward references to a posterior alter-table
- * command.
- *
* The result is a list of parse nodes that still need to be analyzed ---
* but we can't analyze the later commands until we've executed the earlier
* ones, because of possible inter-object references.
- *
- * Note: this breaks the rules a little bit by modifying schema-name fields
- * within passed-in structs. However, the transformation would be the same
- * if done over, so it should be all right to scribble on the input to this
- * extent.
*/
List *
-transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
+transformCreateSchemaStmtElements(ParseState *pstate, List *schemaElts,
+ const char *schemaName)
{
- CreateSchemaStmtContext cxt;
- List *result;
- ListCell *elements;
-
- cxt.schemaname = schemaName;
- cxt.sequences = NIL;
- cxt.tables = NIL;
- cxt.views = NIL;
- cxt.indexes = NIL;
- cxt.triggers = NIL;
- cxt.grants = NIL;
+ List *elements = NIL;
+ ListCell *lc;
/*
- * Run through each schema element in the schema element list. Separate
- * statements by type, and do preliminary analysis.
+ * Run through each schema element in the schema element list. Check
+ * target schema names, and collect the list of actions to be done.
*/
- foreach(elements, schemaElts)
+ foreach(lc, schemaElts)
{
- Node *element = lfirst(elements);
+ Node *element = lfirst(lc);
switch (nodeTag(element))
{
@@ -4054,8 +4027,8 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
{
CreateSeqStmt *elp = (CreateSeqStmt *) element;
- setSchemaName(cxt.schemaname, &elp->sequence->schemaname);
- cxt.sequences = lappend(cxt.sequences, element);
+ checkSchemaName(pstate, schemaName, elp->sequence);
+ elements = lappend(elements, element);
}
break;
@@ -4063,12 +4036,8 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
{
CreateStmt *elp = (CreateStmt *) element;
- setSchemaName(cxt.schemaname, &elp->relation->schemaname);
-
- /*
- * XXX todo: deal with constraints
- */
- cxt.tables = lappend(cxt.tables, element);
+ checkSchemaName(pstate, schemaName, elp->relation);
+ elements = lappend(elements, element);
}
break;
@@ -4076,12 +4045,8 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
{
ViewStmt *elp = (ViewStmt *) element;
- setSchemaName(cxt.schemaname, &elp->view->schemaname);
-
- /*
- * XXX todo: deal with references between views
- */
- cxt.views = lappend(cxt.views, element);
+ checkSchemaName(pstate, schemaName, elp->view);
+ elements = lappend(elements, element);
}
break;
@@ -4089,8 +4054,8 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
{
IndexStmt *elp = (IndexStmt *) element;
- setSchemaName(cxt.schemaname, &elp->relation->schemaname);
- cxt.indexes = lappend(cxt.indexes, element);
+ checkSchemaName(pstate, schemaName, elp->relation);
+ elements = lappend(elements, element);
}
break;
@@ -4098,13 +4063,13 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
{
CreateTrigStmt *elp = (CreateTrigStmt *) element;
- setSchemaName(cxt.schemaname, &elp->relation->schemaname);
- cxt.triggers = lappend(cxt.triggers, element);
+ checkSchemaName(pstate, schemaName, elp->relation);
+ elements = lappend(elements, element);
}
break;
case T_GrantStmt:
- cxt.grants = lappend(cxt.grants, element);
+ elements = lappend(elements, element);
break;
default:
@@ -4113,32 +4078,39 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
}
}
- 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);
-
- return result;
+ return elements;
}
/*
- * setSchemaName
- * Set or check schema name in an element of a CREATE SCHEMA command
+ * checkSchemaName
+ * Check schema name in an element of a CREATE SCHEMA command
+ *
+ * It's okay if the command doesn't specify a target schema name, because
+ * CreateSchemaCommand will set up the default creation schema to be the
+ * new schema. But if a target schema name is given, it had better match.
+ * We also have to check that the command doesn't say CREATE TEMP, since
+ * that would likewise put the object into the wrong schema.
*/
static void
-setSchemaName(const char *context_schema, char **stmt_schema_name)
+checkSchemaName(ParseState *pstate, const char *context_schema,
+ RangeVar *relation)
{
- if (*stmt_schema_name == NULL)
- *stmt_schema_name = unconstify(char *, context_schema);
- else if (strcmp(context_schema, *stmt_schema_name) != 0)
+ if (relation->schemaname != NULL &&
+ strcmp(context_schema, relation->schemaname) != 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_SCHEMA_DEFINITION),
errmsg("CREATE specifies a schema (%s) "
"different from the one being created (%s)",
- *stmt_schema_name, context_schema)));
+ relation->schemaname, context_schema),
+ parser_errposition(pstate, relation->location)));
+
+ if (relation->relpersistence == RELPERSISTENCE_TEMP)
+ {
+ /* spell this error the same as in RangeVarAdjustRelationPersistence */
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("cannot create temporary relation in non-temporary schema")));
+ }
}
/*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index f28bf37105..ad0a314630 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -587,6 +587,8 @@ standard_ProcessUtility(PlannedStmt *pstmt,
pstate = make_parsestate(NULL);
pstate->p_sourcetext = queryString;
+ pstate->p_stmt_location = pstmt->stmt_location;
+ pstate->p_stmt_len = pstmt->stmt_len;
pstate->p_queryEnv = queryEnv;
switch (nodeTag(parsetree))
@@ -1121,10 +1123,7 @@ ProcessUtilitySlow(ParseState *pstate,
* relation and attribute manipulation
*/
case T_CreateSchemaStmt:
- CreateSchemaCommand((CreateSchemaStmt *) parsetree,
- queryString,
- pstmt->stmt_location,
- pstmt->stmt_len);
+ CreateSchemaCommand(pstate, (CreateSchemaStmt *) parsetree);
/*
* EventTriggerCollectSimpleCommand called by
diff --git a/src/include/commands/schemacmds.h b/src/include/commands/schemacmds.h
index 5598dfa5d7..e6861994d0 100644
--- a/src/include/commands/schemacmds.h
+++ b/src/include/commands/schemacmds.h
@@ -16,12 +16,9 @@
#define SCHEMACMDS_H
#include "catalog/objectaddress.h"
-#include "nodes/parsenodes.h"
-
-extern Oid CreateSchemaCommand(CreateSchemaStmt *stmt,
- const char *queryString,
- int stmt_location, int stmt_len);
+#include "parser/parse_node.h"
+extern Oid CreateSchemaCommand(ParseState *pstate, CreateSchemaStmt *stmt);
extern ObjectAddress RenameSchema(const char *oldname, const char *newname);
extern ObjectAddress AlterSchemaOwner(const char *name, Oid newOwnerId);
extern void AlterSchemaOwner_oid(Oid schemaoid, Oid newOwnerId);
diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h
index 1406589477..5d22ea988c 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/src/include/parser/parse_utilcmd.h
@@ -30,7 +30,8 @@ extern CreateStatsStmt *transformStatsStmt(Oid relid, CreateStatsStmt *stmt,
const char *queryString);
extern void transformRuleStmt(RuleStmt *stmt, const char *queryString,
List **actions, Node **whereClause);
-extern List *transformCreateSchemaStmtElements(List *schemaElts,
+extern List *transformCreateSchemaStmtElements(ParseState *pstate,
+ List *schemaElts,
const char *schemaName);
extern PartitionBoundSpec *transformPartitionBound(ParseState *pstate, Relation parent,
PartitionBoundSpec *spec);
diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out
index 93302a07ef..554f0f3c50 100644
--- a/src/test/regress/expected/create_schema.out
+++ b/src/test/regress/expected/create_schema.out
@@ -9,54 +9,84 @@ CREATE ROLE regress_create_schema_role SUPERUSER;
CREATE SCHEMA AUTHORIZATION regress_create_schema_role
CREATE SEQUENCE schema_not_existing.seq;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE SEQUENCE schema_not_existing.seq;
+ ^
CREATE SCHEMA AUTHORIZATION regress_create_schema_role
CREATE TABLE schema_not_existing.tab (id int);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE TABLE schema_not_existing.tab (id int);
+ ^
CREATE SCHEMA AUTHORIZATION regress_create_schema_role
CREATE VIEW schema_not_existing.view AS SELECT 1;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE VIEW schema_not_existing.view AS SELECT 1;
+ ^
CREATE SCHEMA AUTHORIZATION regress_create_schema_role
CREATE INDEX ON schema_not_existing.tab (id);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE INDEX ON schema_not_existing.tab (id);
+ ^
CREATE SCHEMA AUTHORIZATION regress_create_schema_role
CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
EXECUTE FUNCTION schema_trig.no_func();
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_exi...
+ ^
-- Again, with a role specification and no schema names.
SET ROLE regress_create_schema_role;
CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
CREATE SEQUENCE schema_not_existing.seq;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE SEQUENCE schema_not_existing.seq;
+ ^
CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
CREATE TABLE schema_not_existing.tab (id int);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE TABLE schema_not_existing.tab (id int);
+ ^
CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
CREATE VIEW schema_not_existing.view AS SELECT 1;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE VIEW schema_not_existing.view AS SELECT 1;
+ ^
CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
CREATE INDEX ON schema_not_existing.tab (id);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE INDEX ON schema_not_existing.tab (id);
+ ^
CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
EXECUTE FUNCTION schema_trig.no_func();
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created
(regress_create_schema_role)
+LINE 2: CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_exi...
+ ^
-- Again, with a schema name and a role specification.
CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
CREATE SEQUENCE schema_not_existing.seq;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+LINE 2: CREATE SEQUENCE schema_not_existing.seq;
+ ^
CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
CREATE TABLE schema_not_existing.tab (id int);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+LINE 2: CREATE TABLE schema_not_existing.tab (id int);
+ ^
CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
CREATE VIEW schema_not_existing.view AS SELECT 1;
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+LINE 2: CREATE VIEW schema_not_existing.view AS SELECT 1;
+ ^
CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
CREATE INDEX ON schema_not_existing.tab (id);
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+LINE 2: CREATE INDEX ON schema_not_existing.tab (id);
+ ^
CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
EXECUTE FUNCTION schema_trig.no_func();
ERROR: CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+LINE 2: CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_exi...
+ ^
RESET ROLE;
-- Cases where the schema creation succeeds.
-- The schema created matches the role name.
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 7b2198eac6..bafde8706f 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -411,12 +411,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=ALTER 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 INDEX type=index identity=evttrig.one_idx
-- Partitioned tables with a partitioned index
CREATE TABLE evttrig.parted (
id int PRIMARY KEY)
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out
index dbbda72d39..2e582e783c 100644
--- a/src/test/regress/expected/namespace.out
+++ b/src/test/regress/expected/namespace.out
@@ -10,13 +10,14 @@ SELECT pg_catalog.set_config('search_path', ' ', false);
(1 row)
CREATE SCHEMA test_ns_schema_1
- 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
- );
+ )
+ 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
+;
-- verify that the correct search_path restored on abort
SET search_path to public;
BEGIN;
diff --git a/src/test/regress/sql/namespace.sql b/src/test/regress/sql/namespace.sql
index 306cdc2d8c..62a75e63d2 100644
--- a/src/test/regress/sql/namespace.sql
+++ b/src/test/regress/sql/namespace.sql
@@ -7,15 +7,17 @@
SELECT pg_catalog.set_config('search_path', ' ', false);
CREATE SCHEMA test_ns_schema_1
- 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
- );
+ )
+
+ 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
+;
-- verify that the correct search_path restored on abort
SET search_path to public;
--
2.43.5
Kirill Reshke <reshkekirill@gmail.com> writes:
> 3) Why do we delete this in `create_schema.sgml`? Is this untrue? It
> is about order of definition, not creation, isn't it?
>> - The SQL standard specifies that the subcommands in <command>CREATE
>> - SCHEMA</command> can appear in any order.
In context with the following sentence, what that is really trying
to say is that the spec requires us to re-order the subcommands
to eliminate forward references. After studying the text I cannot
find any such statement. Maybe I missed something --- there's a
lot of text --- but it's sure not to be detected in any obvious
place like 11.1 <schema definition>.
(I'd be curious to know how other major implementations handle
this. Are we the only implementation that ever read the spec
that way?)
regards, tom lane
On Sun, Dec 1, 2024 at 1:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Kirill Reshke <reshkekirill@gmail.com> writes: > > 3) Why do we delete this in `create_schema.sgml`? Is this untrue? It > > is about order of definition, not creation, isn't it? > > >> - The SQL standard specifies that the subcommands in <command>CREATE > >> - SCHEMA</command> can appear in any order. > > In context with the following sentence, what that is really trying > to say is that the spec requires us to re-order the subcommands > to eliminate forward references. After studying the text I cannot > find any such statement. Maybe I missed something --- there's a > lot of text --- but it's sure not to be detected in any obvious > place like 11.1 <schema definition>. > I checked, you didn't miss anything 11.1 didn't mention "order" at all. > (I'd be curious to know how other major implementations handle > this. Are we the only implementation that ever read the spec > that way?) > quote from https://learn.microsoft.com/en-us/sql/t-sql/statements/create-schema-transact-sql?view=sql-server-ver16 <<>> CREATE SCHEMA can create a schema, the tables and views it contains, and GRANT, REVOKE, or DENY permissions on any securable in a single statement. This statement must be executed as a separate batch. Objects created by the CREATE SCHEMA statement are created inside the schema that is being created. Securables to be created by CREATE SCHEMA can be listed in any order, except for views that reference other views. In that case, the referenced view must be created before the view that references it. Therefore, a GRANT statement can grant permission on an object before the object itself is created, or a CREATE VIEW statement can appear before the CREATE TABLE statements that create the tables referenced by the view. Also, CREATE TABLE statements can declare foreign keys to tables that are defined later in the CREATE SCHEMA statement. <<>>
jian he <jian.universality@gmail.com> writes: > On Sun, Dec 1, 2024 at 1:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> (I'd be curious to know how other major implementations handle >> this. Are we the only implementation that ever read the spec >> that way?) > quote from https://learn.microsoft.com/en-us/sql/t-sql/statements/create-schema-transact-sql?view=sql-server-ver16 > <<>> > CREATE SCHEMA can create a schema, the tables and views it contains, and GRANT, > REVOKE, or DENY permissions on any securable in a single statement. This > statement must be executed as a separate batch. Objects created by the CREATE > SCHEMA statement are created inside the schema that is being created. > Securables to be created by CREATE SCHEMA can be listed in any order, except for > views that reference other views. In that case, the referenced view must be > created before the view that references it. > Therefore, a GRANT statement can grant permission on an object before the object > itself is created, or a CREATE VIEW statement can appear before the CREATE TABLE > statements that create the tables referenced by the view. Also, CREATE TABLE > statements can declare foreign keys to tables that are defined later in the > CREATE SCHEMA statement. > <<>> Interesting. But I suspect this tells us more about SQL Server's internal implementation of DDL actions than about spec requirements. I looked at DB2's reference page: https://www.ibm.com/docs/en/db2/11.5?topic=statements-create-schema It doesn't have much of anything explicit on this topic, but they do give an example showing that you can create two tables with mutually referencing foreign keys, which means they postpone FK constraint creation till the end. There's also this interesting tidbit: "Unqualified object names in any SQL statement within the CREATE SCHEMA statement are implicitly qualified by the name of the created schema." which eliminates some of the is-that-an-external-reference-or-a- forward-reference ambiguities I was concerned about yesterday. That ship sailed decades ago for us, however. I'm also interested to note that like SQL Server, DB2 has strict limits on the types of objects that can be created, much narrower than what the spec suggests. For DB2 it's: CREATE TABLE statement, excluding typed tables and materialized query tables CREATE VIEW statement, excluding typed views CREATE INDEX statement COMMENT statement GRANT statement That suggests, even though they don't say so, that they're trying to do forward-reference removal; there'd be little reason for the restriction otherwise. MySQL doesn't have CREATE SCHEMA (it's a synonym for CREATE DATABASE), so nothing to be learned there. Whether or not the standard has an opinion on this topic, it's pretty clear that real implementations are all over the place and have plenty of ad-hoc restrictions. I'm still thinking that "let's forget all that and do the subcommands in order" is a win for sanity and explainability. regards, tom lane
I wrote: > I looked at DB2's reference page: > https://www.ibm.com/docs/en/db2/11.5?topic=statements-create-schema Oh, how did I forget Oracle? https://docs.oracle.com/en/database/oracle/oracle-database/23/sqlrf/CREATE-SCHEMA.html Theirs is restricted to CREATE TABLE, CREATE VIEW, and GRANT; also this curious restriction: "The CREATE SCHEMA statement supports the syntax of these statements only as defined by standard SQL, rather than the complete syntax supported by Oracle Database." But then they say: "The order in which you list the CREATE TABLE, CREATE VIEW, and GRANT statements is unimportant. The statements within a CREATE SCHEMA statement can reference existing objects or objects you create in other statements within the same CREATE SCHEMA statement." Which certainly begs the question of how smart their re-ordering algorithm is, or what they do about ambiguity between new and existing objects. But at any rate, it looks like everybody is at least trying to do some amount of re-ordering, which makes me wonder what it is that I'm missing in the spec. That's an awful lot of effort to be expending on something that the spec doesn't seem to require. regards, tom lane
Michael Paquier <michael@paquier.xyz> writes:
> If I'm parsing the spec right, the doc mentions in its 5)~6) of the
> syntax rules in CREATE SCHEMA that non-schema-qualified objects should
> use the new schema name defined in the CREATE SCHEMA query. So that
> pretty much settles the rules to use when having a new object that has
> a reference to a non-qualified object created in the same CREATE
> SCHEMA query?
I don't see where you're getting that from? DB2 says that unqualified
reference names (not to be confused with unqualified creation-target
names) are taken to be in the new schema, but I don't see any
corresponding restriction in the spec.
What I do see (11.1 SR 6 in SQL:2021) is:
If <schema path specification> is not specified, then a <schema
path specification> containing an implementation-defined <schema
name list> that contains the <schema name> contained in <schema
name clause> is implicit.
What I read this as is that the "search path" during schema-element
creation must include at least the new schema, but can also include
some other schemas as defined by the implementation. That makes
our behavior compliant, because we can define the other schemas
as those in the session's prevailing search_path. (DB2's behavior
is also compliant, but they're defining the path as containing only
the new schema.)
Also, if SQL intended to constrain the search path for unqualified
identifiers to be only the new schema, they'd hardly need a concept
of <schema path specification> at all.
regards, tom lane
On 02/12/2024 03:15, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> If I'm parsing the spec right, the doc mentions in its 5)~6) of the >> syntax rules in CREATE SCHEMA that non-schema-qualified objects should >> use the new schema name defined in the CREATE SCHEMA query. So that >> pretty much settles the rules to use when having a new object that has >> a reference to a non-qualified object created in the same CREATE >> SCHEMA query? > I don't see where you're getting that from? DB2 says that unqualified > reference names (not to be confused with unqualified creation-target > names) are taken to be in the new schema, but I don't see any > corresponding restriction in the spec. > > What I do see (11.1 SR 6 in SQL:2021) is: > > If <schema path specification> is not specified, then a <schema > path specification> containing an implementation-defined <schema > name list> that contains the <schema name> contained in <schema > name clause> is implicit. > > What I read this as is that the "search path" during schema-element > creation must include at least the new schema, but can also include > some other schemas as defined by the implementation. That makes > our behavior compliant, because we can define the other schemas > as those in the session's prevailing search_path. (DB2's behavior > is also compliant, but they're defining the path as containing only > the new schema.) > > Also, if SQL intended to constrain the search path for unqualified > identifiers to be only the new schema, they'd hardly need a concept > of <schema path specification> at all. I looked up the original paper (MUN-051) that introduced the <schema path specification> and it says, "The paper is proposing the use of paths only to resolve unqualified routine invocations." That doesn't seem to have been explained much by the rest of the spec, but it is visible in the definition of <path specification> which says, "Specify an order for searching for an SQL-invoked routine." I can find nowhere that says that the path can or cannot be used for other objects. -- Vik Fearing
Vik Fearing <vik@postgresfriends.org> writes:
> On 02/12/2024 03:15, Tom Lane wrote:
>> Also, if SQL intended to constrain the search path for unqualified
>> identifiers to be only the new schema, they'd hardly need a concept
>> of <schema path specification> at all.
> I looked up the original paper (MUN-051) that introduced the <schema
> path specification> and it says, "The paper is proposing the use of
> paths only to resolve unqualified routine invocations."
Interesting. But still, the spec allows <schema routine> within
<schema definition>, so even that narrow interpretation opens them
to the is-this-an-external-reference-or-a-forward-reference problem.
For us, that's clouded further for functions by our overloading rules.
If foo(bigint) exists in the search path, and we have a view or
whatever that references foo() with an int argument, and there is a
CREATE FUNCTION for foo(float8) later in the <schema definition>, what
are we supposed to think is the user's intent? (Just to save people
doing the experiment: we'd prefer foo(float8) if both are visible,
but foo(bigint) would be perfectly acceptable if not. Other choices
of the argument types would yield different results, and none of them
seem especially open-and-shut to me.) I don't know offhand if the
spec allows function overloading in the same way.
regards, tom lane
On 02/12/2024 17:56, Tom Lane wrote: > Vik Fearing <vik@postgresfriends.org> writes: >> On 02/12/2024 03:15, Tom Lane wrote: >>> Also, if SQL intended to constrain the search path for unqualified >>> identifiers to be only the new schema, they'd hardly need a concept >>> of <schema path specification> at all. >> I looked up the original paper (MUN-051) that introduced the <schema >> path specification> and it says, "The paper is proposing the use of >> paths only to resolve unqualified routine invocations." > Interesting. The standard actually does say that that is what it is for. Section 11.1 <schema definition> SR 8: "The <schema name list> of the explicit or implicit <schema path specification> is used as the SQL- path of the schema. The SQL-path is used to effectively qualify unqualified <routine name>s that are immediately contained in <routine invocation>s that are contained in the <schema definition>." > But still, the spec allows <schema routine> within > <schema definition>, so even that narrow interpretation opens them > to the is-this-an-external-reference-or-a-forward-reference problem. Surely that is determined by the placement of the schema in its own SQL-path. > For us, that's clouded further for functions by our overloading rules. > If foo(bigint) exists in the search path, and we have a view or > whatever that references foo() with an int argument, and there is a > CREATE FUNCTION for foo(float8) later in the <schema definition>, what > are we supposed to think is the user's intent? (Just to save people > doing the experiment: we'd prefer foo(float8) if both are visible, > but foo(bigint) would be perfectly acceptable if not. Other choices > of the argument types would yield different results, and none of them > seem especially open-and-shut to me.) My answer is the same as above, for unqualified names. However, since there is nothing that says anything either way about forward references, my preference is to just execute them all in the order written. In your example, that would mean choosing otherschema.foo(bigint) over thisschema.foo(float8) if the latter hasn't been created yet. > I don't know offhand if the > spec allows function overloading in the same way. Feature T-321 has a note saying, "Support for overloaded functions and procedures is not part of Core SQL." -- Vik Fearing
On 30.11.24 20:08, Tom Lane wrote: > 2. transformCreateSchemaStmtElements is of the opinion that it's > responsible for ordering the schema elements in a way that will work, > but it just about completely fails at that task. Ordering the objects > by kind is surely not sufficient, and adding CREATE DOMAIN will make > that worse. (Example: a domain could be used in a table definition, > but we also allow domains to be created over tables' composite types.) > Yet we have no infrastructure that would allow us to discover the real > dependencies between unparsed DDL commands, nor is it likely that > anyone will ever undertake building such. I think we ought to nuke > that concept from orbit and just execute the schema elements in the > order presented. I looked at several iterations of the SQL standard > and cannot find any support for the idea that CREATE SCHEMA needs to > be any smarter than that. I'd also argue that doing anything else is > a POLA violation. It's especially a POLA violation if the code > rearranges a valid user-written command order into an invalid order, > which is inevitable if we stick with the current approach. > > The notion that we ought to sort the objects by kind appears to go > all the way back to 95ef6a344 of 2002-03-21, which I guess makes it > my fault. There must have been some prior mailing list discussion, > but I couldn't find much. There is a predecessor of the committed > patch in > https://www.postgresql.org/message-id/flat/3C7F8A49.CC4EF0BE%40redhat.com > but no discussion of why sorting by kind is a good idea. (The last > message in the thread suggests that there was more discussion among > the Red Hat RHDB team, but if so it's lost to history now.) SQL/Framework subclause "Descriptors" says: """ The execution of an SQL-statement may result in the creation of many descriptors. An SQL object that is created as a result of an SQL-statement may depend on other descriptors that are only created as a result of the execution of that SQL statement. NOTE 8 — This is particularly relevant in the case of the <schema definition> SQL-statement. A <schema definition> can, for example, contain many <table definition>s that in turn contain <table constraint>s. A single <table constraint> in one <table definition> can reference a second table being created by a separate <table definition> which itself is able to contain a reference to the first table. The dependencies of each table on the descriptors of the other are valid provided that all necessary descriptors are created during the execution of the complete <schema definition>. """ So this says effectively that forward references are allowed. Whether reordering the statements is a good way to implement that is dubious, as we are discovering.
Peter Eisentraut <peter@eisentraut.org> writes:
> On 30.11.24 20:08, Tom Lane wrote:
>> ... I think we ought to nuke
>> that concept from orbit and just execute the schema elements in the
>> order presented. I looked at several iterations of the SQL standard
>> and cannot find any support for the idea that CREATE SCHEMA needs to
>> be any smarter than that.
> SQL/Framework subclause "Descriptors" says:
> """
> The execution of an SQL-statement may result in the creation of many
> descriptors. An SQL object that is created as a result of an
> SQL-statement may depend on other descriptors that are only created as a
> result of the execution of that SQL statement.
> NOTE 8 — This is particularly relevant in the case of the <schema
> definition> SQL-statement. A <schema definition> can, for example,
> contain many <table definition>s that in turn contain <table
> constraint>s. A single <table constraint> in one <table definition> can
> reference a second table being created by a separate <table definition>
> which itself is able to contain a reference to the first table. The
> dependencies of each table on the descriptors of the other are valid
> provided that all necessary descriptors are created during the execution
> of the complete <schema definition>.
> """
Ah, thanks for the pointer.
> So this says effectively that forward references are allowed. Whether
> reordering the statements is a good way to implement that is dubious, as
> we are discovering.
Yeah, I think it's a long way from this text to the conclusion that
the implementation is responsible for reordering the subcommands
to remove forward references. And it really offers no help at all
for the ensuing problem of distinguishing forward references from
external references.
The one aspect of the spec's definition that seems useful to me in
practice is the ability to create quasi-circular foreign keys (that
is, t1 has an FK to t2 and t2 has an FK to t1). But that is something
we have never implemented in 22 years and nobody has complained of
the lack. I'm totally willing to throw that possibility overboard
permanently in order to expand the set of creatable object types
without introducing a ton of restrictions and weird behaviors.
What do you think?
regards, tom lane
PS: if we were really excited about allowing circular FKs to be
made within CREATE SCHEMA, a possible though non-standard answer
would be to allow ALTER TABLE ADD CONSTRAINT as a <schema element>.
Looks like this thread does not move forward. So I'm posting my thoughts to bump it. On Wed, 4 Dec 2024 at 01:07, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm totally willing to throw that possibility overboard > permanently in order to expand the set of creatable object types > without introducing a ton of restrictions and weird behaviors. > What do you think? Im +1 on this, but can you please elaborate, which exact objects cannot be created now? What will be expanded after v2-0002-Dont_try-to-reoder....? > PS: if we were really excited about allowing circular FKs to be > made within CREATE SCHEMA, a possible though non-standard answer > would be to allow ALTER TABLE ADD CONSTRAINT as a <schema element>. That's a nice feature to have by itself? -- Best regards, Kirill Reshke
Kirill Reshke <reshkekirill@gmail.com> writes:
> On Wed, 4 Dec 2024 at 01:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm totally willing to throw that possibility overboard
>> permanently in order to expand the set of creatable object types
>> without introducing a ton of restrictions and weird behaviors.
>> What do you think?
> Im +1 on this, but can you please elaborate, which exact objects
> cannot be created now? What will be expanded after
> v2-0002-Dont_try-to-reoder....?
The problem is not too awful right now, because of the very limited
set of object types that CREATE SCHEMA supports. The only case
I can think of offhand is a table referencing a view's rowtype,
for example
create schema s1
create view v1 as select ...
create table t1 (compositecol v1, ...);
Since transformCreateSchemaStmtElements re-orders views after
tables, this'll fail, and there is no way to fix that except
by giving up use of the elements-in-CREATE-SCHEMA feature.
Admittedly it's a strange usage, and probably no one has tried it.
However, once we start adding in data types and functions,
the hazard grows substantially, because there are more usage
patterns and they can't all be satisfied by a simple object-type
ordering. For example, domains are already enough to cause
trouble, because we allow domains over composites:
create schema s1
create table t1 (...)
create domain d1 as t1 check(...);
Re-ordering domains before tables would break this case, but
the other order has other problems. Looking a bit further
down the road, how would you handle creation of a base type
within CREATE SCHEMA?
create schema s1
create type myscalar
create function myscalar_in(cstring) returns myscalar ...
create function myscalar_out(myscalar) returns cstring ...
create type myscalar (input = myscalar_in, ...);
This cannot possibly work if an object-type-based re-ordering
is done to it.
So IMV, we have three possibilities:
1. CREATE SCHEMA's schema-element feature remains forevermore
a sad joke that (a) doesn't cover nearly enough to be useful and
(b) doesn't come close to doing what the spec says it should.
2. We invest an enormous amount of engineering effort on trying
to extract dependencies from not-yet-analyzed parse trees, after
which we invest a bunch more effort figuring out heuristics for
ordering the subcommands in the face of circular dependencies.
(Some of that could be stolen from pg_dump, but not all: pg_dump
only has to resolve a limited set of cases.)
3. We bypass the need for #2 by decreeing that we'll execute
the subcommands in order.
>> PS: if we were really excited about allowing circular FKs to be
>> made within CREATE SCHEMA, a possible though non-standard answer
>> would be to allow ALTER TABLE ADD CONSTRAINT as a <schema element>.
> That's a nice feature to have by itself?
Not unless we abandon the idea of subcommand reordering, because
where are you going to put the ALTER TABLE subcommands?
regards, tom lane