CREATE SCHEMA ... CREATE DOMAIN support

On Tue, Nov 12, 2024 at 8:55 PM Kirill Reshke <reshkekirill@gmail.com> wrote:

Patch obviously leaks doc & regression tests, but I'm posting it to
see if this contribution is needed in PostgreSQL

the following two statement should fail:
CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
CREATE table t(a ss)
create domain public.ss as text not null default 'hello' constraint
nn check (value <> 'hello');

CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
CREATE table t(a ss)
create domain postgres.public.ss as text not null default 'hello'
constraint nn check (value <> 'hello');

we aslo need to consider the dependency issue. like the following should be ok.
CREATE SCHEMA regress_schema_3 AUTHORIZATION CURRENT_ROLE
create view test as select 'hello'::ss as test
CREATE table t(a ss)
create domain ss as text not null;

i fixed these two issues, and add the above example as tests in
src/test/regress/sql/create_schema.sql

I didn't add a doc entry. I will do it later.

On Sat, Nov 23, 2024 at 1:19 PM jian he <jian.universality@gmail.com> wrote:

I didn't add a doc entry. I will do it later.

hi
attached patch with thorough tests and documentation.

one issue i still have is:
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.
like what we did with create schema create table:

CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
create table t(a int, b x);
ERROR: type "x" does not exist
LINE 2: create table t(a int, b x);
^

jian he <jian.universality@gmail.com> writes:

one issue i still have is:
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.

That doesn't happen in the base case either:

regression=# create domain ss1 as ss;
ERROR: type "ss" does not exist

I doubt that fixing it should be part of this patch.

regards, tom lane

On Wed, 27 Nov 2024 at 08:42, jian he <jian.universality@gmail.com> wrote:

On Sat, Nov 23, 2024 at 1:19 PM jian he <jian.universality@gmail.com> wrote:

I didn't add a doc entry. I will do it later.

hi
attached patch with thorough tests and documentation.

Hi! Thanks for pushing this further.

one issue i still have is:
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.
like what we did with create schema create table:

CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
create table t(a int, b x);
ERROR: type "x" does not exist
LINE 2: create table t(a int, b x);
^

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.

`make check` on v3 runs successfully. Test & doc seems fine to me.

PFA v4. The only change I made is for a commit message, and pg indent
run on this diff.

--
Best regards,
Kirill Reshke

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

On Wed, 27 Nov 2024 at 23:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:

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

Indeed, my analysis is wrong.

Turns out passing parsestate to DefineDomain is itself enhancement.

Before this patch:
```
db1=# create domain ss1 as ss;
ERROR: type "ss" does not exist
```

after:

```
db1=# create domain ss1 as ss;
ERROR: type "ss" does not exist
LINE 1: create domain ss1 as ss;
^
```
PFA as an independent patch then. Or should we combine these two into one?

--
Best regards,
Kirill Reshke

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

new patch, add tab complete for it.

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]/messages/by-id/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com.

[0]: /messages/by-id/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.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
/messages/by-id/3C7F8A49.CC4EF0BE@redhat.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

On Sun, 1 Dec 2024 at 04:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:

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.

Agreed.

So attached is a draft patch that simplifies the rule to "do the
subcommands in the order written".

+1 on this idea.

Here is my 2c to this draft:
1) Report error position on newly added check for temp relation in
non-temp schema.

+                 errmsg("cannot create temporary relation in non-temporary schema"),
+                 parser_errposition(pstate, relation->location)));

2)
I added some regression tests that might be worth adding:
a) check for a temporary table created within a non-temporary schema
in create_table.sql, akin to existing check in create_view.sql.
b) Also explicitly check that old-style sql creation does not work.
That is, for example,
CREATE SCHEMA test_ns_schema_3
CREATE VIEW abcd_view AS
SELECT a FROM abcd
CREATE TABLE abcd (
a serial
);

fails.

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.

P.S.
This section in SQL-92 is the only information I could find about
order of creation.

```
3) Those objects defined by <schema element>s (base tables, views,
constraints, domains, assertions, character sets, translations,
collations, privileges) and their associated descriptors are
effectively created.
```
Look like we are 100% to do it in order of definition

--
Best regards,
Kirill Reshke

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

On Sun, Dec 01, 2024 at 05:30:20PM -0500, Tom Lane wrote:

Which certainly begs the question of how smart their re-ordering
algorithm is, or what they do about ambiguity between new and existing
objects.

Perhaps because they are able to track efficiently all schema
references, like checking the internal of functions at creation time
rather than just at runtime? The ambiguity between new and existing
objects may be tricky, indeed.

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?

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.

As Jian has mentioned, 9075-2-2023 around 11.1 for CREATE SCHEMA does
not include any ordering assumptions when the elements are created, so
my guess is that this is left up to each implementation depending on
how they need to handle their dependencies with their meta-data
lookup? The result would be the same once the query has finished
running, as long as the elements created are consistent with their
inner dependencies.
--
Michael

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