Subject: Re: CREATE SCHEMA ... CREATE DOMAIN support

From: Tom Lane <[email-masked]>
Date: Wed, 27 Nov 2024 13:39:40 -0500

Kirill Reshke <[email-masked]> writes:
> On Wed, 27 Nov 2024 at 08:42, jian he <[email-masked]> 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

---

From: Kirill Reshke <[email-masked]>
Date: Thu, 28 Nov 2024 10:27:00 +0500

> a passed-down ParseState, but not DefineDomain.

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

---

From: Tom Lane <[email-masked]>
Date: Thu, 28 Nov 2024 00:52:54 -0500

> 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

---

From: Kirill Reshke <[email-masked]>
Date: Thu, 28 Nov 2024 14:58:41 +0500

> No, I don't think this should be part of the patch discussed in this

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

---

From: jian he <[email-masked]>
Date: Fri, 29 Nov 2024 21:47:45 +0800

new patch, add tab complete for it.
From 91d05e547ca722d4537ff7420b8248a3fcce3b58 Mon Sep 17 00:00:00 2001
From: jian he <[email-masked]>
Date: Fri, 29 Nov 2024 21:44:54 +0800
Subject: [PATCH v5 1/1] support CREATE SCHEMA CREATE DOMAIN

SQL standard allow domain to be specified with CREATE SCHEMA
statement. This patch adds support in PostgreSQL for that.

For example:
    CREATE SCHEMA schema_name AUTHORIZATION CURRENT_ROLE
    create view test as select 'hello'::ss as test
    CREATE table t(a ss)
    create domain ss as text not null;

The domain will be created within the to be created schema.
The domain name can be schema-qualified or database-qualified,
however it's not allowed to let domain create within a different schema.

Author: Kirill Reshke <[email-masked]>
Author: Jian He <[email-masked]>
Reviewed-by: Alvaro Herrera <[email-masked]>
Reviewed-by: Tom Lane <[email-masked]>
---
 doc/src/sgml/ref/create_schema.sgml         |  2 +-
 src/backend/parser/gram.y                   |  1 +
 src/backend/parser/parse_utilcmd.c          | 27 +++++++++++
 src/bin/psql/tab-complete.in.c              | 12 ++---
 src/test/regress/expected/create_schema.out | 51 +++++++++++++++++++++
 src/test/regress/sql/create_schema.sql      | 33 +++++++++++++
 6 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/ref/create_schema.sgml b/doc/src/sgml/ref/create_schema.sgml
index ed69298ccc..06f6314a5b 100644
--- a/doc/src/sgml/ref/create_schema.sgml
+++ b/doc/src/sgml/ref/create_schema.sgml
@@ -100,7 +100,7 @@ CREATE SCHEMA IF NOT EXISTS AUTHORIZATION <replaceable class="parameter">role_sp
       <listitem>
        <para>
         An SQL statement defining an object to be created within the
-        schema. Currently, only <command>CREATE
+        schema. Currently, only <command>CREATE DOMAIN</command>, <command>CREATE
         TABLE</command>, <command>CREATE VIEW</command>, <command>CREATE
         INDEX</command>, <command>CREATE SEQUENCE</command>, <command>CREATE
         TRIGGER</command> and <command>GRANT</command> are accepted as clauses
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 67eb96396a..ad8d9270ac 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1584,6 +1584,7 @@ schema_stmt:
 			| CreateTrigStmt
 			| GrantStmt
 			| ViewStmt
+			| CreateDomainStmt
 		;

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 0f324ee4e3..45328eea16 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -105,6 +105,7 @@ typedef struct
 	List	   *indexes;		/* CREATE INDEX items */
 	List	   *triggers;		/* CREATE TRIGGER items */
 	List	   *grants;			/* GRANT items */
+	List	   *domains;		/* DOMAIN items */
 } CreateSchemaStmtContext;

@@ -4039,6 +4040,7 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
 	cxt.indexes = NIL;
 	cxt.triggers = NIL;
 	cxt.grants = NIL;
+	cxt.domains = NIL;
 
 	/*
 	 * Run through each schema element in the schema element list. Separate
@@ -4107,6 +4109,30 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
 				cxt.grants = lappend(cxt.grants, element);
 				break;
 
+			case T_CreateDomainStmt:
+				{
+					CreateDomainStmt *elp = (CreateDomainStmt *) element;
+					char	   *domain_schema = NULL;
+
+					/*
+					 * DOMAIN's schema must the same as the to be created
+					 * schema if length of domainname > 3 will fail at
+					 * DeconstructQualifiedName,
+					 */
+					if (list_length(elp->domainname) == 2)
+					{
+						domain_schema = strVal(list_nth(elp->domainname, 0));
+						setSchemaName(cxt.schemaname, &domain_schema);
+					}
+					else if (list_length(elp->domainname) == 3)
+					{
+						domain_schema = strVal(list_nth(elp->domainname, 1));
+						setSchemaName(cxt.schemaname, &domain_schema);
+					}
+					cxt.domains = lappend(cxt.domains, element);
+				}
+				break;
+
 			default:
 				elog(ERROR, "unrecognized node type: %d",
 					 (int) nodeTag(element));
@@ -4114,6 +4140,7 @@ transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
 	}
 
 	result = NIL;
+	result = list_concat(result, cxt.domains);
 	result = list_concat(result, cxt.sequences);
 	result = list_concat(result, cxt.tables);
 	result = list_concat(result, cxt.views);
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index bbd08770c3..542429712a 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -2135,7 +2135,7 @@ match_previous_words(int pattern_id,
 	{
 		/* only some object types can be created as part of CREATE SCHEMA */
 		if (HeadMatches("CREATE", "SCHEMA"))
-			COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER",
+			COMPLETE_WITH("TABLE", "VIEW", "INDEX", "SEQUENCE", "TRIGGER", "DOMAIN",
 			/* for INDEX and TABLE/SEQUENCE, respectively */
 						  "UNIQUE", "UNLOGGED");
 		else
@@ -3293,15 +3293,15 @@ match_previous_words(int pattern_id,
 	else if (Matches("CREATE", "DATABASE", MatchAny, "STRATEGY"))
 		COMPLETE_WITH("WAL_LOG", "FILE_COPY");
 
-	/* CREATE DOMAIN */
-	else if (Matches("CREATE", "DOMAIN", MatchAny))
+	/* CREATE DOMAIN --- is allowed inside CREATE SCHEMA, so use TailMatches */	
+	else if (TailMatches("CREATE", "DOMAIN", MatchAny))
 		COMPLETE_WITH("AS");
-	else if (Matches("CREATE", "DOMAIN", MatchAny, "AS"))
+	else if (TailMatches("CREATE", "DOMAIN", MatchAny, "AS"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_datatypes);
-	else if (Matches("CREATE", "DOMAIN", MatchAny, "AS", MatchAny))
+	else if (TailMatches("CREATE", "DOMAIN", MatchAny, "AS", MatchAny))
 		COMPLETE_WITH("COLLATE", "DEFAULT", "CONSTRAINT",
 					  "NOT NULL", "NULL", "CHECK (");
-	else if (Matches("CREATE", "DOMAIN", MatchAny, "COLLATE"))
+	else if (TailMatches("CREATE", "DOMAIN", MatchAny, "COLLATE"))
 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_collations);
 
 	/* CREATE EXTENSION */
diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out
index 93302a07ef..d2b97911cc 100644
--- a/src/test/regress/expected/create_schema.out
+++ b/src/test/regress/expected/create_schema.out
@@ -58,6 +58,57 @@ CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
   EXECUTE FUNCTION schema_trig.no_func();
 ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
 RESET ROLE;
+-- Cases where the schema creation with domain.
+--fail. cannot create domain to other schema
+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');
+ERROR:  CREATE specifies a schema (public) different from the one being created (regress_schema_2)
+--fail. cannot create domain to other schema
+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');
+ERROR:  CREATE specifies a schema (public) different from the one being created (regress_schema_2)
+--fail. forward references, need reorder.
+CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
+  create domain ss1 as ss
+  create domain ss as text;
+ERROR:  type "ss" does not exist
+--ok, qualified schema name for domain should be same as the created schema.
+CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
+  create table t(a regress_schema_2.ss)
+  create domain regress_schema_2.ss as text not null;
+\dD regress_schema_2.*
+                             List of domains
+      Schema      | Name | Type | Collation | Nullable | Default | Check 
+------------------+------+------+-----------+----------+---------+-------
+ regress_schema_2 | ss   | text |           | not null |         | 
+(1 row)
+
+--ok, no qualified schema name for domain.
+CREATE SCHEMA regress_schema_3 AUTHORIZATION CURRENT_ROLE
+  create view test as select 'hello'::ss as test
+  create table t(a ss1)
+  create domain ss as text not null
+  create domain ss1 as ss;
+\dD regress_schema_3.*
+                                    List of domains
+      Schema      | Name |        Type         | Collation | Nullable | Default | Check 
+------------------+------+---------------------+-----------+----------+---------+-------
+ regress_schema_3 | ss   | text                |           | not null |         | 
+ regress_schema_3 | ss1  | regress_schema_3.ss |           |          |         | 
+(2 rows)
+
+DROP SCHEMA regress_schema_2 CASCADE;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to type regress_schema_2.ss
+drop cascades to table regress_schema_2.t
+DROP SCHEMA regress_schema_3 CASCADE;
+NOTICE:  drop cascades to 4 other objects
+DETAIL:  drop cascades to type regress_schema_3.ss
+drop cascades to type regress_schema_3.ss1
+drop cascades to table regress_schema_3.t
+drop cascades to view regress_schema_3.test
 -- Cases where the schema creation succeeds.
 -- The schema created matches the role name.
 CREATE SCHEMA AUTHORIZATION regress_create_schema_role
diff --git a/src/test/regress/sql/create_schema.sql b/src/test/regress/sql/create_schema.sql
index 1b7064247a..421aaa424e 100644
--- a/src/test/regress/sql/create_schema.sql
+++ b/src/test/regress/sql/create_schema.sql
@@ -47,6 +47,39 @@ CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
   EXECUTE FUNCTION schema_trig.no_func();
 RESET ROLE;
 
+-- Cases where the schema creation with domain.
+
+--fail. cannot create domain to other schema
+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');
+--fail. cannot create domain to other schema
+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');
+
+--fail. forward references, need reorder.
+CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
+  create domain ss1 as ss
+  create domain ss as text;
+
+--ok, qualified schema name for domain should be same as the created schema.
+CREATE SCHEMA regress_schema_2 AUTHORIZATION CURRENT_ROLE
+  create table t(a regress_schema_2.ss)
+  create domain regress_schema_2.ss as text not null;
+\dD regress_schema_2.*
+
+--ok, no qualified schema name for domain.
+CREATE SCHEMA regress_schema_3 AUTHORIZATION CURRENT_ROLE
+  create view test as select 'hello'::ss as test
+  create table t(a ss1)
+  create domain ss as text not null
+  create domain ss1 as ss;
+\dD regress_schema_3.*
+
+DROP SCHEMA regress_schema_2 CASCADE;
+DROP SCHEMA regress_schema_3 CASCADE;
+
 -- Cases where the schema creation succeeds.
 -- The schema created matches the role name.
 CREATE SCHEMA AUTHORIZATION regress_create_schema_role
-- 
2.34.1

---

From: Kirill Reshke <[email-masked]>
Date: Fri, 29 Nov 2024 19:00:37 +0500

> 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