Skip to content

Commit

Permalink
handle / throw an error for more create table / create schema variants
Browse files Browse the repository at this point in the history
  • Loading branch information
onurctirtir committed May 25, 2023
1 parent b355e65 commit 23687e7
Show file tree
Hide file tree
Showing 8 changed files with 271 additions and 20 deletions.
35 changes: 35 additions & 0 deletions src/backend/distributed/commands/schema.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ static List * FilterDistributedSchemas(List *schemas);
static bool SchemaHasDistributedTableWithFKey(char *schemaName);
static bool ShouldPropagateCreateSchemaStmt(void);
static List * GetGrantCommandsFromCreateSchemaStmt(Node *node);
static bool CreateSchemaStmtCreatesTable(CreateSchemaStmt *stmt);


/*
Expand Down Expand Up @@ -79,6 +80,16 @@ PostprocessCreateSchemaStmt(Node *node, const char *queryString)

if (ShouldUseSchemaBasedSharding(createSchemaStmt->schemaname))
{
/* for now, we don't allow creating tenant tables when creating the schema itself */
if (CreateSchemaStmtCreatesTable(createSchemaStmt))
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create tenant table in CREATE "
"SCHEMA statement"),
errhint("Use CREATE TABLE statement to create "
"tenant tables.")));
}

bool missingOk = false;
Oid schemaId = get_namespace_oid(createSchemaStmt->schemaname, missingOk);

Expand Down Expand Up @@ -427,3 +438,27 @@ GetGrantCommandsFromCreateSchemaStmt(Node *node)

return commands;
}


/*
* CreateSchemaStmtCreatesTable returns true if given CreateSchemaStmt
* creates a table using "schema_element" list.
*/
static bool
CreateSchemaStmtCreatesTable(CreateSchemaStmt *stmt)
{
Node *element = NULL;
foreach_ptr(element, stmt->schemaElts)
{
/*
* CREATE TABLE AS and CREATE FOREIGN TABLE commands cannot be
* used as schema_elements anyway, so we don't need to check them.
*/
if (IsA(element, CreateStmt))
{
return true;
}
}

return false;
}
75 changes: 69 additions & 6 deletions src/backend/distributed/commands/table.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,17 @@ PostprocessCreateTableStmt(CreateStmt *createStatement, const char *queryString)
{
PostprocessCreateTableStmtForeignKeys(createStatement);

bool missingOk = false;
Oid relationId = RangeVarGetRelid(createStatement->relation, NoLock, missingOk);
Oid schemaId = get_rel_namespace(relationId);
if (createStatement->ofTypename && IsTenantSchema(schemaId))
{
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create a tenant table by using CREATE TABLE "
"OF syntax")));
}

if (createStatement->inhRelations != NIL)
{
if (createStatement->partbound != NULL)
Expand All @@ -239,15 +250,31 @@ PostprocessCreateTableStmt(CreateStmt *createStatement, const char *queryString)
else
{
/* process CREATE TABLE ... INHERITS ... */

if (IsTenantSchema(schemaId))
{
ereport(ERROR, (errmsg("tenant tables cannot inherit or "
"be inherited")));
}

RangeVar *parentRelation = NULL;
foreach_ptr(parentRelation, createStatement->inhRelations)
{
bool missingOk = false;
Oid parentRelationId = RangeVarGetRelid(parentRelation, NoLock,
missingOk);
Assert(parentRelationId != InvalidOid);

if (IsCitusTable(parentRelationId))
/*
* Throw a better error message if the user tries to inherit a
* tenant table or if the user tries to inherit from a tenant
* table.
*/
if (IsTenantSchema(get_rel_namespace(parentRelationId)))
{
ereport(ERROR, (errmsg("tenant tables cannot inherit or "
"be inherited")));
}
else if (IsCitusTable(parentRelationId))
{
/* here we error out if inheriting a distributed table */
ereport(ERROR, (errmsg("non-distributed tables cannot inherit "
Expand Down Expand Up @@ -396,9 +423,8 @@ PostprocessCreateTableStmtPartitionOf(CreateStmt *createStatement, const
if (IsCitusTable(parentRelationId))
{
/*
* We can create Citus local tables and single-shard distributed tables
* right away, without switching to sequential mode, because they are going to
* have only one shard.
* We can create Citus local tables right away, without switching to
* sequential mode, because they are going to have only one shard.
*/
if (IsCitusTableType(parentRelationId, CITUS_LOCAL_TABLE))
{
Expand Down Expand Up @@ -608,6 +634,10 @@ DistributePartitionUsingParent(Oid parentCitusRelationId, Oid partitionRelationI
{
char *parentRelationName = generate_qualified_relation_name(parentCitusRelationId);

/*
* We can create tenant tables and single shard tables right away, without
* switching to sequential mode, because they are going to have only one shard.
*/
if (ShouldCreateTenantSchemaTable(partitionRelationId))
{
CreateTenantSchemaTable(partitionRelationId);
Expand Down Expand Up @@ -4066,20 +4096,53 @@ ErrorIfTableHasIdentityColumn(Oid relationId)
/*
* ConvertNewTableIfNecessary converts the given table to a tenant schema
* table or a Citus managed table if necessary.
*
* Input node is expected to be a CreateStmt or a CreateTableAsStmt.
*/
void
ConvertNewTableIfNecessary(CreateStmt *baseCreateTableStmt)
ConvertNewTableIfNecessary(Node *createStmt)
{
/*
* Need to increment command counter so that next command
* can see the new table.
*/
CommandCounterIncrement();

if (IsA(createStmt, CreateTableAsStmt))
{
CreateTableAsStmt *createTableAsStmt = (CreateTableAsStmt *) createStmt;

bool missingOk = false;
Oid createdRelationId = RangeVarGetRelid(createTableAsStmt->into->rel,
NoLock, missingOk);

if (ShouldCreateTenantSchemaTable(createdRelationId))
{
ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot create a tenant table using "
"CREATE TABLE AS or SELECT INTO "
"statements")));
}

/*
* We simply ignore the tables created by using that syntax when using
* Citus managed tables.
*/
return;
}

CreateStmt *baseCreateTableStmt = (CreateStmt *) createStmt;

bool missingOk = false;
Oid createdRelationId = RangeVarGetRelid(baseCreateTableStmt->relation,
NoLock, missingOk);

/* not try to convert the table if it already exists and IF NOT EXISTS syntax is used */
if (baseCreateTableStmt->if_not_exists && IsCitusTable(createdRelationId))
{
return;
}

/*
* Check ShouldCreateTenantSchemaTable() before ShouldAddNewTableToMetadata()
* because we don't want to unnecessarily add the table into metadata
Expand Down
27 changes: 18 additions & 9 deletions src/backend/distributed/commands/utility_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,26 @@ multi_ProcessUtility(PlannedStmt *pstmt,
*/
if (context == PROCESS_UTILITY_TOPLEVEL &&
(IsA(parsetree, CreateStmt) ||
IsA(parsetree, CreateForeignTableStmt)))
IsA(parsetree, CreateForeignTableStmt) ||
IsA(parsetree, CreateTableAsStmt)))
{
/*
* Not directly cast to CreateStmt to guard against the case where
* the definition of CreateForeignTableStmt changes in future.
*/
CreateStmt *baseCreateTableStmt =
IsA(parsetree, CreateStmt) ? (CreateStmt *) parsetree :
(CreateStmt *) &(((CreateForeignTableStmt *) parsetree)->base);
Node *createStmt = NULL;
if (IsA(parsetree, CreateTableAsStmt))
{
createStmt = parsetree;
}
else
{
/*
* Not directly cast to CreateStmt to guard against the case where
* the definition of CreateForeignTableStmt changes in future.
*/
createStmt =
IsA(parsetree, CreateStmt) ? parsetree :
(Node *) &(((CreateForeignTableStmt *) parsetree)->base);
}

ConvertNewTableIfNecessary(baseCreateTableStmt);
ConvertNewTableIfNecessary(createStmt);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/include/distributed/commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ extern char * GetAlterColumnWithNextvalDefaultCmd(Oid sequenceOid, Oid relationI

extern void ErrorIfTableHasUnsupportedIdentityColumn(Oid relationId);
extern void ErrorIfTableHasIdentityColumn(Oid relationId);
extern void ConvertNewTableIfNecessary(CreateStmt *baseCreateTableStmt);
extern void ConvertNewTableIfNecessary(Node *createStmt);

/* text_search.c - forward declarations */
extern List * GetCreateTextSearchConfigStatements(const ObjectAddress *address);
Expand Down
5 changes: 4 additions & 1 deletion src/test/regress/expected/auto_undist_citus_local.out
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,9 @@ SELECT logicalrelid, partmethod, repmodel FROM pg_dist_partition WHERE logicalre
DROP TABLE IF EXISTS citus_local_table_1, citus_local_table_2, citus_local_table_3;
-- this GUC will add the next three tables to metadata automatically
SET citus.use_citus_managed_tables TO ON;
CREATE TABLE citus_local_table_1(a INT UNIQUE);
-- try to create the table twice by using IF NOT EXISTS syntax
CREATE TABLE IF NOT EXISTS citus_local_table_1(a INT UNIQUE);
CREATE TABLE IF NOT EXISTS citus_local_table_1(a INT UNIQUE);
CREATE TABLE citus_local_table_2(a INT UNIQUE);
CREATE TABLE citus_local_table_3(a INT UNIQUE);
RESET citus.use_citus_managed_tables;
Expand Down Expand Up @@ -1340,3 +1342,4 @@ SELECT logicalrelid, partmethod, repmodel FROM pg_dist_partition WHERE logicalre
(1 row)

DROP SCHEMA drop_fkey_cascade CASCADE;
DROP USER another_user;
80 changes: 79 additions & 1 deletion src/test/regress/expected/schema_based_sharding.out
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_2');
t
(1 row)

-- verify that we don't allow creating tenant tables via CREATE SCHEMA command
CREATE SCHEMA schema_using_schema_elements CREATE TABLE test_table(a int, b text);
ERROR: cannot create tenant table in CREATE SCHEMA statement
HINT: Use CREATE TABLE statement to create tenant tables.
CREATE SCHEMA tenant_4;
CREATE TABLE tenant_4.tbl_1(a int, b text);
CREATE TABLE tenant_4.tbl_2(a int, b text);
Expand Down Expand Up @@ -350,7 +354,81 @@ SELECT EXISTS(
t
(1 row)

CREATE TABLE tenant_4.tbl_3(a int, b text);
-- verify that we don't allow creating tenant tables by using CREATE TABLE AS / SELECT INTO commands
CREATE TABLE tenant_4.tbl_3 AS SELECT 1 AS a, 'text' as b;
ERROR: cannot create a tenant table using CREATE TABLE AS or SELECT INTO statements
CREATE TABLE IF NOT EXISTS tenant_4.tbl_3 AS SELECT 1 as a, 'text' as b;
ERROR: cannot create a tenant table using CREATE TABLE AS or SELECT INTO statements
SELECT 1 as a, 'text' as b INTO tenant_4.tbl_3;
ERROR: cannot create a tenant table using CREATE TABLE AS or SELECT INTO statements
CREATE TYPE employee_type AS (name text, salary numeric);
-- verify that we don't allow creating tenant tables by using CREATE TABLE OF commands
CREATE TABLE tenant_4.employees OF employee_type (
PRIMARY KEY (name),
salary WITH OPTIONS DEFAULT 1000
);
ERROR: cannot create a tenant table by using CREATE TABLE OF syntax
-- verify that we act accordingly when if not exists is used
CREATE TABLE IF NOT EXISTS tenant_4.tbl_3(a int, b text);
CREATE TABLE IF NOT EXISTS tenant_4.tbl_3(a int, b text);
NOTICE: relation "tbl_3" already exists, skipping
CREATE TABLE regular_schema.local(a int, b text);
CREATE TABLE regular_schema.citus_local(a int, b text);
SELECT citus_add_local_table_to_metadata('regular_schema.citus_local');
citus_add_local_table_to_metadata
---------------------------------------------------------------------

(1 row)

CREATE TABLE regular_schema.dist(a int, b text);
SELECT create_distributed_table('regular_schema.dist', 'a');
create_distributed_table
---------------------------------------------------------------------

(1 row)

-- verify that we can create a table LIKE another table
CREATE TABLE tenant_5.test_table_like_1(LIKE tenant_5.tbl_1); -- using a table from the same schema
CREATE TABLE tenant_5.test_table_like_2(LIKE tenant_4.tbl_1); -- using a table from another schema
CREATE TABLE tenant_5.test_table_like_3(LIKE regular_schema.local); -- using a local table
CREATE TABLE tenant_5.test_table_like_4(LIKE regular_schema.citus_local); -- using a citus local table
CREATE TABLE tenant_5.test_table_like_5(LIKE regular_schema.dist); -- using a distributed table
-- verify that all of them are converted to tenant tables
SELECT COUNT(*) = 5
FROM pg_dist_partition
WHERE logicalrelid::text LIKE 'tenant_5.test_table_like_%' AND
partmethod = 'n' AND repmodel = 's' AND colocationid = (
SELECT colocationid FROM pg_dist_tenant_schema
WHERE schemaid::regnamespace::text = 'tenant_5'
);
?column?
---------------------------------------------------------------------
t
(1 row)

CREATE TABLE regular_schema.local_table_using_like(LIKE tenant_5.tbl_1);
-- verify that regular_schema.local_table_using_like is not a tenant table
SELECT COUNT(*) = 0 FROM pg_dist_partition
WHERE logicalrelid = 'regular_schema.local_table_using_like'::regclass;
?column?
---------------------------------------------------------------------
t
(1 row)

-- verify that INHERITS syntax is not supported when creating a tenant table
CREATE TABLE tenant_5.test_table_inherits_1(x int) INHERITS (tenant_5.tbl_1); -- using a table from the same schema
ERROR: tenant tables cannot inherit or be inherited
CREATE TABLE tenant_5.test_table_inherits_2(x int) INHERITS (tenant_4.tbl_1); -- using a table from another schema
ERROR: tenant tables cannot inherit or be inherited
CREATE TABLE tenant_5.test_table_inherits_3(x int) INHERITS (regular_schema.local); -- using a local table
ERROR: tenant tables cannot inherit or be inherited
CREATE TABLE tenant_5.test_table_inherits_4(x int) INHERITS (regular_schema.citus_local); -- using a citus local table
ERROR: tenant tables cannot inherit or be inherited
CREATE TABLE tenant_5.test_table_inherits_5(x int) INHERITS (regular_schema.dist); -- using a distributed table
ERROR: tenant tables cannot inherit or be inherited
-- verify that INHERITS syntax is not supported when creating a local table based on a tenant table
CREATE TABLE regular_schema.local_table_using_inherits(x int) INHERITS (tenant_5.tbl_1);
ERROR: tenant tables cannot inherit or be inherited
CREATE TABLE tenant_5.tbl_2(a int, b text);
CREATE SCHEMA "CiTuS.TeeN_108";
ALTER SCHEMA "CiTuS.TeeN_108" RENAME TO citus_teen_proper;
Expand Down
7 changes: 6 additions & 1 deletion src/test/regress/sql/auto_undist_citus_local.sql
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,11 @@ DROP TABLE IF EXISTS citus_local_table_1, citus_local_table_2, citus_local_table

-- this GUC will add the next three tables to metadata automatically
SET citus.use_citus_managed_tables TO ON;
CREATE TABLE citus_local_table_1(a INT UNIQUE);

-- try to create the table twice by using IF NOT EXISTS syntax
CREATE TABLE IF NOT EXISTS citus_local_table_1(a INT UNIQUE);
CREATE TABLE IF NOT EXISTS citus_local_table_1(a INT UNIQUE);

CREATE TABLE citus_local_table_2(a INT UNIQUE);
CREATE TABLE citus_local_table_3(a INT UNIQUE);
RESET citus.use_citus_managed_tables;
Expand Down Expand Up @@ -738,3 +742,4 @@ CALL drop_constraint_via_proc_exception();
SELECT logicalrelid, partmethod, repmodel FROM pg_dist_partition WHERE logicalrelid IN ('reference_table_1'::regclass, 'citus_local_table_1'::regclass) ORDER BY logicalrelid;

DROP SCHEMA drop_fkey_cascade CASCADE;
DROP USER another_user;
Loading

0 comments on commit 23687e7

Please sign in to comment.