pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT )  will coredump
14+ messages / 4 participants
[nested] [flat]

* BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT )  will coredump
@ 2023-04-27 03:44  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 14+ messages in thread

From: PG Bug reporting form @ 2023-04-27 03:44 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: hysong0101@163.com

The following bug has been logged on the website:

Bug reference:      17909
Logged by:          Song Hongyu
Email address:      hysong0101@163.com
PostgreSQL version: 15.0
Operating system:   centos7
Description:        

When we CREATE SCHEMA AUTHORIZATION rolname CREATE TABLE/SEQUENCE/VIEW
sch.obj and sch is not NULL,
the database will coredump.

The reason is in parse_utilcmd.c we will check whether schemaName and
rolname are same or not. The pointer is not checked in strcmp there, so the
database coredump is caused.



^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT )  will coredump
@ 2023-04-27 07:34  Michael Paquier <michael@paquier.xyz>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-27 07:34 UTC (permalink / raw)
  To: hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 03:44:04AM +0000, PG Bug reporting form wrote:
> When we CREATE SCHEMA AUTHORIZATION rolname CREATE TABLE/SEQUENCE/VIEW
> sch.obj and sch is not NULL,
> the database will coredump.

It took me a couple of minutes to get what you meant here.  The point
is that schema-qualifying any of the object specified after the CREATE
SCHEMA with a schema name different than the rolname would cause a
crash, when no schema is directly given.  We should fail with the same
error than when a schema is specified, as of, except that the rolename
needs to be specified:
=# create schema popo authorization postgres create table lala.aa (a int);
ERROR:  42P15: CREATE specifies a schema (lala) different from the one being created (popo)

That seems quite old, at quick glance (v11 fails), so this needs to be
fixed all the way down.  Will fix, nice catch!
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEolfwBHm6+znydH@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 08:59  Richard Guo <guofenglinux@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Richard Guo @ 2023-04-27 08:59 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 3:34 PM Michael Paquier <michael@paquier.xyz> wrote:

> It took me a couple of minutes to get what you meant here.  The point
> is that schema-qualifying any of the object specified after the CREATE
> SCHEMA with a schema name different than the rolname would cause a
> crash, when no schema is directly given.  We should fail with the same
> error than when a schema is specified, as of, except that the rolename
> needs to be specified:
> =# create schema popo authorization postgres create table lala.aa (a int);
> ERROR:  42P15: CREATE specifies a schema (lala) different from the one
> being created (popo)


Aha, now I get the scenario that would crash.

# create schema authorization postgres create table lala.aa (a int);
server closed the connection unexpectedly

In this case the CreateSchemaStmtContext.schemaname is NULL since it is
not explicitly specified, while the schemaname in the schema element is
not NULL as it is specified, and setSchemaName cannot copy with such
situation.  Maybe we should check against RoleSpec.rolename in this case
since that is also the schema's name?


> That seems quite old, at quick glance (v11 fails), so this needs to be
> fixed all the way down.


Yes.  I can see this crash from master all back to v9.5.

Thanks
Richard

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 09:31  Michael Paquier <michael@paquier.xyz>
  parent: Richard Guo <guofenglinux@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-27 09:31 UTC (permalink / raw)
  To: Richard Guo <guofenglinux@gmail.com>; +Cc: hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 04:59:13PM +0800, Richard Guo wrote:
> In this case the CreateSchemaStmtContext.schemaname is NULL since it is
> not explicitly specified, while the schemaname in the schema element is
> not NULL as it is specified, and setSchemaName cannot copy with such
> situation.  Maybe we should check against RoleSpec.rolename in this case
> since that is also the schema's name?

In this case, it is cleaner to just set the schema name in
CreateSchemaStmtContext.schemaname to the role in the RoleSpec if there
is no schema set in the query, because the schema name will have the
same name as the role.  That also makes the handling of each element
in schemaElts simpler.

The regression tests cruelly lacks of checks here.  This is not a
pattern of CREATE SCHEMA known a lot, but we should do better.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEpA1SgNzEqhgCvA@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 10:28  Richard Guo <guofenglinux@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Richard Guo @ 2023-04-27 10:28 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 5:31 PM Michael Paquier <michael@paquier.xyz> wrote:

> On Thu, Apr 27, 2023 at 04:59:13PM +0800, Richard Guo wrote:
> > In this case the CreateSchemaStmtContext.schemaname is NULL since it is
> > not explicitly specified, while the schemaname in the schema element is
> > not NULL as it is specified, and setSchemaName cannot copy with such
> > situation.  Maybe we should check against RoleSpec.rolename in this case
> > since that is also the schema's name?
>
> In this case, it is cleaner to just set the schema name in
> CreateSchemaStmtContext.schemaname to the role in the RoleSpec if there
> is no schema set in the query, because the schema name will have the
> same name as the role.  That also makes the handling of each element
> in schemaElts simpler.


I noticed that in CreateSchemaCommand there is logic that fills schema
name with the role name if it is not specified.  Do you think we can
save the new-filled schema name into CreateSchemaStmt.schemaname there?


> The regression tests cruelly lacks of checks here.  This is not a
> pattern of CREATE SCHEMA known a lot, but we should do better.


Agreed.  It's better to have a case covering this pattern of CREATE
SCHEMA.

Thanks
Richard

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 23:05  Michael Paquier <michael@paquier.xyz>
  parent: Richard Guo <guofenglinux@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-27 23:05 UTC (permalink / raw)
  To: Richard Guo <guofenglinux@gmail.com>; +Cc: hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 06:28:28PM +0800, Richard Guo wrote:
> I noticed that in CreateSchemaCommand there is logic that fills schema
> name with the role name if it is not specified.  Do you think we can
> save the new-filled schema name into CreateSchemaStmt.schemaname there?

It's actually much trickier than that, on second look, as a RoleSpec
is embedded in this portion of a query because we need to support
CURRENT_ROLE, CURRENT_SESSION and SESSION_USER.  For example, this
query assigns neither role name nor schema name we could rely on at
transformation for the objects:
create schema authorization current_role create table aa (a int);

Anyway, semantically, something could go very wrong if we decide to
enforce a schema name for the objects based on the RoleSpec at the
time of transformation, because we may finish by executing the CREATE
SCHEMA command under an entirely different context than what we could
assign.  So, I think that we should do the following in the
transformation path if an object is schema-qualified:
- Fail immediately if there is no schema and no role name at hand,
just give up.  This needs a new error message, say:
"CREATE specifies a schema (%object_schema) without providing a schema
name."
- If there is a role name, aka RoleSpec points to a ROLESPEC_CSTRING,
use it as a comparison with the objects schema-qualified.  Note that
there is no case for public, because we would fail on
get_rolespec_oid() when the schema is created.

There is also a very fancy case, if "foo" matches to the role that
would be assigned by GetUserIdAndSecContext() when executing the
schema command:
create schema authorization session_role create table foo.aa (a int);

One could say that this should work, and my proposal would cause an
error to make the query more predictible at an earlier step.  IMO, I
think that this is just saner.  And this case crashes today like the
others.

Any thoughts or objections about doing that?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEr%2Fshjg+8TClUm%2F@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 23:48  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2023-04-27 23:48 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> It's actually much trickier than that, on second look, as a RoleSpec
> is embedded in this portion of a query because we need to support
> CURRENT_ROLE, CURRENT_SESSION and SESSION_USER.  For example, this
> query assigns neither role name nor schema name we could rely on at
> transformation for the objects:
> create schema authorization current_role create table aa (a int);

Seems like the answer needs to involve postponing examination of
the contained DDL until we've figured out what the new schema's
name is going to be.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-27 23:56  Michael Paquier <michael@paquier.xyz>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-27 23:56 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Thu, Apr 27, 2023 at 07:48:49PM -0400, Tom Lane wrote:
> Seems like the answer needs to involve postponing examination of
> the contained DDL until we've figured out what the new schema's
> name is going to be.

Actually, wait a min..  The transformation of the objects is applied
during the execution of the CREATE SCHEMA command, but nowhere else,
so if you give to transformCreateSchemaStmt() the name of the expected
schema rather than rely on the schema name from the query this should
work OK.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEsLq0ee%2FZKwjf%2Fr@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-28 01:42  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-28 01:42 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Fri, Apr 28, 2023 at 08:56:27AM +0900, Michael Paquier wrote:
> Actually, wait a min..  The transformation of the objects is applied
> during the execution of the CREATE SCHEMA command, but nowhere else,
> so if you give to transformCreateSchemaStmt() the name of the expected
> schema rather than rely on the schema name from the query this should
> work OK.

So, the source of my confusion is the design currently used for
transformCreateSchemaStmt():
- The schema name is extracted from the query itself, but we have a
schema compiled from a role specification, depending on how the
beginning of CreateSchemaCommand() feels it.
- This routine includes a reference to the role specification in the
context, but makes no use of it.  Perhaps somebody would be
interested in this information in the future if the query support is
improved, but one could also be tempted to feed the schema name based
on the RoleSpec, which I'd rather avoid for the moment.

Attached is what I am finishing with, where I have reworked
transformCreateSchemaStmt() so as it uses in input the list of
elements from CREATE SCHEMA and the schema name computed depending on
the security context, documenting requirements on the way (note the
extra unconstify for the RangeVars' schemas).  I have added a couple
of regression tests for all the object types that have schema
qualication checks, mixed with role specs and schema names.

Thoughts, comments or objections?
--
Michael

Attachments:

  [text/x-diff] 0001-Fix-crashes-with-CREATE-SCHEMA-AUTHORIZATION.patch (15.8K, ../../ZEskninhkXCQ5jZO@paquier.xyz/2-0001-Fix-crashes-with-CREATE-SCHEMA-AUTHORIZATION.patch)
  download | inline diff:
From 1de9f837a2d1dc52f78d430734cf9040f6d10c90 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 28 Apr 2023 10:37:26 +0900
Subject: [PATCH] Fix crashes with CREATE SCHEMA AUTHORIZATION

Blah blah..

Backpatch to 11 required..
---
 src/include/parser/parse_utilcmd.h          |  3 +-
 src/backend/commands/schemacmds.c           |  3 +-
 src/backend/parser/parse_utilcmd.c          | 36 ++++----
 src/test/regress/expected/create_schema.out | 98 +++++++++++++++++++++
 src/test/regress/parallel_schedule          |  2 +-
 src/test/regress/sql/create_schema.sql      | 70 +++++++++++++++
 6 files changed, 192 insertions(+), 20 deletions(-)
 create mode 100644 src/test/regress/expected/create_schema.out
 create mode 100644 src/test/regress/sql/create_schema.sql

diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h
index ea4e4f44c1..2b40a0b5b1 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 *transformCreateSchemaStmt(CreateSchemaStmt *stmt);
+extern List *transformCreateSchemaStmtElements(List *schemaElts,
+											   const char *schemaName);
 extern PartitionBoundSpec *transformPartitionBound(ParseState *pstate, Relation parent,
 												   PartitionBoundSpec *spec);
 extern List *expandTableLikeClause(RangeVar *heapRel,
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 90de935267..48590247f8 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -179,7 +179,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
 	 * we cannot, in general, run parse analysis on one statement until we
 	 * have actually executed the prior ones.
 	 */
-	parsetree_list = transformCreateSchemaStmt(stmt);
+	parsetree_list = transformCreateSchemaStmtElements(stmt->schemaElts,
+													   schemaName);
 
 	/*
 	 * Execute each command contained in the CREATE SCHEMA.  Since the grammar
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index b0f6fe4fa6..b7c1bd3bcc 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -95,12 +95,11 @@ typedef struct
 	bool		ofType;			/* true if statement contains OF typename */
 } CreateStmtContext;
 
-/* State shared by transformCreateSchemaStmt and its subroutines */
+/* State shared by transformCreateSchemaStmtElements and its subroutines */
 typedef struct
 {
 	const char *stmtType;		/* "CREATE SCHEMA" or "ALTER SCHEMA" */
-	char	   *schemaname;		/* name of schema */
-	RoleSpec   *authrole;		/* owner of schema */
+	const char *schemaname;		/* name of schema */
 	List	   *sequences;		/* CREATE SEQUENCE items */
 	List	   *tables;			/* CREATE TABLE items */
 	List	   *views;			/* CREATE VIEW items */
@@ -134,7 +133,7 @@ static void transformCheckConstraints(CreateStmtContext *cxt,
 static void transformConstraintAttrs(CreateStmtContext *cxt,
 									 List *constraintList);
 static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column);
-static void setSchemaName(char *context_schema, char **stmt_schema_name);
+static void setSchemaName(const char *context_schema, char **stmt_schema_name);
 static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
 static List *transformPartitionRangeBounds(ParseState *pstate, List *blist,
 										   Relation parent);
@@ -3784,14 +3783,18 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column)
 
 
 /*
- * transformCreateSchemaStmt -
- *	  analyzes the CREATE SCHEMA statement
+ * transformCreateSchemaStmtElements -
+ *	  analyzes the elements of a CREATE SCHEMA statement
  *
- * Split the schema element list 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.
+ * 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.
+ *
+ * "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
+ * 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
@@ -3807,15 +3810,14 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column)
  * extent.
  */
 List *
-transformCreateSchemaStmt(CreateSchemaStmt *stmt)
+transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
 {
 	CreateSchemaStmtContext cxt;
 	List	   *result;
 	ListCell   *elements;
 
 	cxt.stmtType = "CREATE SCHEMA";
-	cxt.schemaname = stmt->schemaname;
-	cxt.authrole = (RoleSpec *) stmt->authrole;
+	cxt.schemaname = schemaName;
 	cxt.sequences = NIL;
 	cxt.tables = NIL;
 	cxt.views = NIL;
@@ -3827,7 +3829,7 @@ transformCreateSchemaStmt(CreateSchemaStmt *stmt)
 	 * Run through each schema element in the schema element list. Separate
 	 * statements by type, and do preliminary analysis.
 	 */
-	foreach(elements, stmt->schemaElts)
+	foreach(elements, schemaElts)
 	{
 		Node	   *element = lfirst(elements);
 
@@ -3912,10 +3914,10 @@ transformCreateSchemaStmt(CreateSchemaStmt *stmt)
  *		Set or check schema name in an element of a CREATE SCHEMA command
  */
 static void
-setSchemaName(char *context_schema, char **stmt_schema_name)
+setSchemaName(const char *context_schema, char **stmt_schema_name)
 {
 	if (*stmt_schema_name == NULL)
-		*stmt_schema_name = context_schema;
+		*stmt_schema_name = unconstify(char *, context_schema);
 	else if (strcmp(context_schema, *stmt_schema_name) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_SCHEMA_DEFINITION),
diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out
new file mode 100644
index 0000000000..953b233a20
--- /dev/null
+++ b/src/test/regress/expected/create_schema.out
@@ -0,0 +1,98 @@
+--
+-- CREATE_SCHEMA
+--
+-- Schema creation with elements.
+CREATE ROLE regress_create_schema_role SUPERUSER;
+-- Cases where schema creation fails as objects are qualified with a schema
+-- that does not match with what's expected.
+-- This checks all the query with schema qualifications.
+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)
+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)
+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)
+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)
+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)
+-- 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)
+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)
+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)
+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)
+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)
+-- 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)
+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)
+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)
+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)
+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)
+RESET ROLE;
+-- Cases where the schema creation succeeds.
+-- The schema created matches the role name.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+      Table "regress_create_schema_role.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_create_schema_role CASCADE;
+NOTICE:  drop cascades to table regress_create_schema_role.tab
+-- Again, with a different role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+      Table "regress_create_schema_role.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_create_schema_role CASCADE;
+NOTICE:  drop cascades to table tab
+-- Again, with a schema name and a role specification
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
+  CREATE TABLE regress_schema_1.tab (id int);
+\d regress_schema_1.tab
+           Table "regress_schema_1.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_schema_1 CASCADE;
+NOTICE:  drop cascades to table regress_schema_1.tab
+RESET ROLE;
+-- Clean up
+DROP ROLE regress_create_schema_role;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 3624035639..cf46fa3359 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -42,7 +42,7 @@ test: copy copyselect copydml insert insert_conflict
 # More groups of parallel tests
 # Note: many of the tests in later groups depend on create_index
 # ----------
-test: create_function_c create_misc create_operator create_procedure create_table create_type
+test: create_function_c create_misc create_operator create_procedure create_table create_type create_schema
 test: create_index create_index_spgist create_view index_including index_including_gist
 
 # ----------
diff --git a/src/test/regress/sql/create_schema.sql b/src/test/regress/sql/create_schema.sql
new file mode 100644
index 0000000000..de75a9aedf
--- /dev/null
+++ b/src/test/regress/sql/create_schema.sql
@@ -0,0 +1,70 @@
+--
+-- CREATE_SCHEMA
+--
+
+-- Schema creation with elements.
+
+CREATE ROLE regress_create_schema_role SUPERUSER;
+
+-- Cases where schema creation fails as objects are qualified with a schema
+-- that does not match with what's expected.
+-- This checks all the query with schema qualifications.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE SEQUENCE schema_not_existing.seq;
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE schema_not_existing.tab (id int);
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  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();
+-- 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;
+CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
+  CREATE TABLE schema_not_existing.tab (id int);
+CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
+  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();
+-- Again, with a schema name and a role specification.
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
+  CREATE SEQUENCE schema_not_existing.seq;
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
+  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;
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
+  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();
+RESET ROLE;
+
+-- Cases where the schema creation succeeds.
+-- The schema created matches the role name.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+DROP SCHEMA regress_create_schema_role CASCADE;
+-- Again, with a different role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_ROLE
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+DROP SCHEMA regress_create_schema_role CASCADE;
+-- Again, with a schema name and a role specification
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_ROLE
+  CREATE TABLE regress_schema_1.tab (id int);
+\d regress_schema_1.tab
+DROP SCHEMA regress_schema_1 CASCADE;
+RESET ROLE;
+
+-- Clean up
+DROP ROLE regress_create_schema_role;
-- 
2.40.0

  [application/pgp-signature] signature.asc (832B, ../../ZEskninhkXCQ5jZO@paquier.xyz/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-28 03:29  Richard Guo <guofenglinux@gmail.com>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Richard Guo @ 2023-04-28 03:29 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Fri, Apr 28, 2023 at 9:43 AM Michael Paquier <michael@paquier.xyz> wrote:

> Attached is what I am finishing with, where I have reworked
> transformCreateSchemaStmt() so as it uses in input the list of
> elements from CREATE SCHEMA and the schema name computed depending on
> the security context, documenting requirements on the way (note the
> extra unconstify for the RangeVars' schemas).  I have added a couple
> of regression tests for all the object types that have schema
> qualication checks, mixed with role specs and schema names.
>
> Thoughts, comments or objections?


+1.  I like the refactor of transformCreateSchemaStmt.

BTW, the comment states that CreateSchemaStmtContext.stmtType is "CREATE
SCHEMA" or "ALTER SCHEMA".   But it seems that there is no chance to set
it to "ALTER SCHEMA".  So should we update that comment, or go even
further to remove CreateSchemaStmtContext.stmtType since it is not used?

Thanks
Richard

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-28 03:56  Michael Paquier <michael@paquier.xyz>
  parent: Richard Guo <guofenglinux@gmail.com>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-28 03:56 UTC (permalink / raw)
  To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Fri, Apr 28, 2023 at 11:29:11AM +0800, Richard Guo wrote:
> BTW, the comment states that CreateSchemaStmtContext.stmtType is "CREATE
> SCHEMA" or "ALTER SCHEMA".   But it seems that there is no chance to set
> it to "ALTER SCHEMA".  So should we update that comment, or go even
> further to remove CreateSchemaStmtContext.stmtType since it is not used?

Indeed.  I'd be OK with adjusting the comment, without removing
stmtType to keep some consistency with CreateStmt, and it could be
useful for debugging, perhaps..  (See 46379d6 as one origin point).
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEtD9rCQGXV10TWV@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-28 04:53  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 14+ messages in thread

From: Tom Lane @ 2023-04-28 04:53 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> On Fri, Apr 28, 2023 at 11:29:11AM +0800, Richard Guo wrote:
>> BTW, the comment states that CreateSchemaStmtContext.stmtType is "CREATE
>> SCHEMA" or "ALTER SCHEMA".   But it seems that there is no chance to set
>> it to "ALTER SCHEMA".  So should we update that comment, or go even
>> further to remove CreateSchemaStmtContext.stmtType since it is not used?

> Indeed.  I'd be OK with adjusting the comment, without removing
> stmtType to keep some consistency with CreateStmt, and it could be
> useful for debugging, perhaps..  (See 46379d6 as one origin point).

I'd be okay with just dropping that field.  It seems to be much
older than 46379d6, and if it ever had any real use it doesn't now.
(There's no ALTER SCHEMA in the SQL spec at all, let alone one that
has some overlap with CREATE SCHEMA options, so I don't foresee a
future use either.)

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-28 05:07  Michael Paquier <michael@paquier.xyz>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 14+ messages in thread

From: Michael Paquier @ 2023-04-28 05:07 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Fri, Apr 28, 2023 at 12:53:39AM -0400, Tom Lane wrote:
> I'd be okay with just dropping that field.  It seems to be much
> older than 46379d6, and if it ever had any real use it doesn't now.
> (There's no ALTER SCHEMA in the SQL spec at all, let alone one that
> has some overlap with CREATE SCHEMA options, so I don't foresee a
> future use either.)

WFM as well.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZEtUmPXlfn394sTK@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread

* Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump
@ 2023-04-30 22:09  Michael Paquier <michael@paquier.xyz>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 14+ messages in thread

From: Michael Paquier @ 2023-04-30 22:09 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Richard Guo <guofenglinux@gmail.com>; hysong0101@163.com; pgsql-bugs@lists.postgresql.org

On Fri, Apr 28, 2023 at 02:07:36PM +0900, Michael Paquier wrote:
> WFM as well.

Note that this has been applied as of 4dadd66 down to v11.  Thanks for
the report, the reviews and the discussion!
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZE7nLtCHx4CVCues@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 14+ messages in thread


end of thread, other threads:[~2023-04-30 22:09 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-27 03:44 BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT )  will coredump PG Bug reporting form <noreply@postgresql.org>
2023-04-27 07:34 ` Michael Paquier <michael@paquier.xyz>
2023-04-27 08:59   ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Richard Guo <guofenglinux@gmail.com>
2023-04-27 09:31     ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-27 10:28       ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Richard Guo <guofenglinux@gmail.com>
2023-04-27 23:05         ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-27 23:48           ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Tom Lane <tgl@sss.pgh.pa.us>
2023-04-27 23:56             ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-28 01:42               ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-28 03:29                 ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Richard Guo <guofenglinux@gmail.com>
2023-04-28 03:56                   ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-28 04:53                     ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Tom Lane <tgl@sss.pgh.pa.us>
2023-04-28 05:07                       ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>
2023-04-30 22:09                         ` Re: BUG #17909: CREATE SCHEMA AUTHORIZATION sch CREATE TABLE foo ( id INT ) will coredump Michael Paquier <michael@paquier.xyz>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox