From f4b2494d0c251718c7676532079e1c70fcf786a6 Mon Sep 17 00:00:00 2001 From: ahmet gedemenli Date: Fri, 2 Jun 2023 14:05:15 +0300 Subject: [PATCH 1/5] Disable update_distributed_table_colocation for tenant tables --- .../distributed/commands/schema_based_sharding.c | 15 +++++++++++++++ src/backend/distributed/utils/colocation_utils.c | 2 ++ src/include/distributed/commands.h | 1 + .../regress/expected/schema_based_sharding.out | 16 ++++++++++++++-- src/test/regress/sql/schema_based_sharding.sql | 7 ++++++- 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/backend/distributed/commands/schema_based_sharding.c b/src/backend/distributed/commands/schema_based_sharding.c index 678835821..ce33b51a1 100644 --- a/src/backend/distributed/commands/schema_based_sharding.c +++ b/src/backend/distributed/commands/schema_based_sharding.c @@ -242,3 +242,18 @@ citus_internal_unregister_tenant_schema_globally(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + + +/* + * ErrorIfTenantTable errors out with the given operation name, + * if the given relation is a tenant table. + */ +void +ErrorIfTenantTable(Oid relationId, char *operationName) +{ + if (IsTenantSchema(get_rel_namespace(relationId))) + { + ereport(ERROR, (errmsg("%s is not allowed for %s because it is a tenant table", + get_rel_name(relationId), operationName))); + } +} diff --git a/src/backend/distributed/utils/colocation_utils.c b/src/backend/distributed/utils/colocation_utils.c index 8f8dade6b..dba791ba4 100644 --- a/src/backend/distributed/utils/colocation_utils.c +++ b/src/backend/distributed/utils/colocation_utils.c @@ -116,6 +116,7 @@ update_distributed_table_colocation(PG_FUNCTION_ARGS) text *colocateWithTableNameText = PG_GETARG_TEXT_P(1); EnsureTableOwner(targetRelationId); + ErrorIfTenantTable(targetRelationId, "update_distributed_table_colocation"); char *colocateWithTableName = text_to_cstring(colocateWithTableNameText); if (IsColocateWithNone(colocateWithTableName)) @@ -126,6 +127,7 @@ update_distributed_table_colocation(PG_FUNCTION_ARGS) else { Oid colocateWithTableId = ResolveRelationId(colocateWithTableNameText, false); + ErrorIfTenantTable(colocateWithTableId, "colocate_with"); EnsureTableOwner(colocateWithTableId); MarkTablesColocated(colocateWithTableId, targetRelationId); } diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index fd67da301..3064f83cd 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -789,5 +789,6 @@ extern bool IsTenantSchema(Oid schemaId); extern void ErrorIfIllegalPartitioningInTenantSchema(Oid parentRelationId, Oid partitionRelationId); extern void CreateTenantSchemaTable(Oid relationId); +extern void ErrorIfTenantTable(Oid relationId, char *operationName); #endif /*CITUS_COMMANDS_H */ diff --git a/src/test/regress/expected/schema_based_sharding.out b/src/test/regress/expected/schema_based_sharding.out index 6beb5ddd7..32ad6b1a2 100644 --- a/src/test/regress/expected/schema_based_sharding.out +++ b/src/test/regress/expected/schema_based_sharding.out @@ -34,6 +34,12 @@ SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); (1 row) CREATE TABLE regular_schema.test_table(a int, b text); +SELECT create_distributed_table('regular_schema.test_table', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + SET citus.enable_schema_based_sharding TO ON; -- show that regular_schema doesn't show up in pg_dist_tenant_schema SELECT COUNT(*)=0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text = 'regular_schema'; @@ -68,6 +74,12 @@ SELECT create_reference_table('tenant_2.test_table'); ERROR: table "test_table" is already distributed SELECT citus_add_local_table_to_metadata('tenant_2.test_table'); ERROR: table "test_table" is already distributed +-- verify we don't allow update_distributed_table_colocation for tenant tables +SELECT update_distributed_table_colocation('tenant_2.test_table', colocate_with => 'none'); +ERROR: test_table is not allowed for update_distributed_table_colocation because it is a tenant table +-- verify we also don't allow colocate_with a tenant table +SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); +ERROR: test_table is not allowed for colocate_with because it is a tenant table -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_3'); @@ -235,8 +247,8 @@ SELECT EXISTS( (1 row) INSERT INTO tenant_4.another_partitioned_table VALUES (1, 'a'); -ERROR: insert or update on table "another_partitioned_table_child_1920008" violates foreign key constraint "another_partitioned_table_a_fkey_1920007" -DETAIL: Key (a)=(1) is not present in table "partitioned_table_1920005". +ERROR: insert or update on table "another_partitioned_table_child_1920040" violates foreign key constraint "another_partitioned_table_a_fkey_1920039" +DETAIL: Key (a)=(1) is not present in table "partitioned_table_1920037". CONTEXT: while executing command on localhost:xxxxx INSERT INTO tenant_4.partitioned_table VALUES (1, 'a'); INSERT INTO tenant_4.another_partitioned_table VALUES (1, 'a'); diff --git a/src/test/regress/sql/schema_based_sharding.sql b/src/test/regress/sql/schema_based_sharding.sql index 12dec9f0b..fc75ca4cc 100644 --- a/src/test/regress/sql/schema_based_sharding.sql +++ b/src/test/regress/sql/schema_based_sharding.sql @@ -25,7 +25,7 @@ SELECT citus_internal_unregister_tenant_schema_globally('regular_schema'::regnam SELECT 1 FROM citus_remove_node('localhost', :worker_2_port); CREATE TABLE regular_schema.test_table(a int, b text); - +SELECT create_distributed_table('regular_schema.test_table', 'a'); SET citus.enable_schema_based_sharding TO ON; -- show that regular_schema doesn't show up in pg_dist_tenant_schema @@ -55,6 +55,11 @@ SELECT create_distributed_table('tenant_2.test_table', 'a'); SELECT create_reference_table('tenant_2.test_table'); SELECT citus_add_local_table_to_metadata('tenant_2.test_table'); +-- verify we don't allow update_distributed_table_colocation for tenant tables +SELECT update_distributed_table_colocation('tenant_2.test_table', colocate_with => 'none'); +-- verify we also don't allow colocate_with a tenant table +SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); + -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_3'); From 4b67e398b11676e6e8d518a16cfa584b0d019d52 Mon Sep 17 00:00:00 2001 From: ahmet gedemenli Date: Fri, 2 Jun 2023 14:08:03 +0300 Subject: [PATCH 2/5] Disable undistribute_table for tenant tables --- src/backend/distributed/commands/alter_table.c | 2 ++ src/test/regress/expected/schema_based_sharding.out | 3 +++ src/test/regress/sql/schema_based_sharding.sql | 2 ++ 3 files changed, 7 insertions(+) diff --git a/src/backend/distributed/commands/alter_table.c b/src/backend/distributed/commands/alter_table.c index a0359d335..3728e7470 100644 --- a/src/backend/distributed/commands/alter_table.c +++ b/src/backend/distributed/commands/alter_table.c @@ -380,6 +380,8 @@ UndistributeTable(TableConversionParameters *params) "because the table is not distributed"))); } + ErrorIfTenantTable(params->relationId, "undistribute_table"); + if (!params->cascadeViaForeignKeys) { EnsureTableNotReferencing(params->relationId, UNDISTRIBUTE_TABLE); diff --git a/src/test/regress/expected/schema_based_sharding.out b/src/test/regress/expected/schema_based_sharding.out index 32ad6b1a2..0a529398a 100644 --- a/src/test/regress/expected/schema_based_sharding.out +++ b/src/test/regress/expected/schema_based_sharding.out @@ -80,6 +80,9 @@ ERROR: test_table is not allowed for update_distributed_table_colocation becaus -- verify we also don't allow colocate_with a tenant table SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); ERROR: test_table is not allowed for colocate_with because it is a tenant table +-- verify we don't allow undistribute_table for tenant tables +SELECT undistribute_table('tenant_2.test_table'); +ERROR: test_table is not allowed for undistribute_table because it is a tenant table -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_3'); diff --git a/src/test/regress/sql/schema_based_sharding.sql b/src/test/regress/sql/schema_based_sharding.sql index fc75ca4cc..469145a91 100644 --- a/src/test/regress/sql/schema_based_sharding.sql +++ b/src/test/regress/sql/schema_based_sharding.sql @@ -59,6 +59,8 @@ SELECT citus_add_local_table_to_metadata('tenant_2.test_table'); SELECT update_distributed_table_colocation('tenant_2.test_table', colocate_with => 'none'); -- verify we also don't allow colocate_with a tenant table SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); +-- verify we don't allow undistribute_table for tenant tables +SELECT undistribute_table('tenant_2.test_table'); -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema From f68ea2000932b97d9b05ffe80024d6b32306aff5 Mon Sep 17 00:00:00 2001 From: ahmet gedemenli Date: Fri, 2 Jun 2023 14:24:14 +0300 Subject: [PATCH 3/5] Disable alter_distributed_table for tenant tables --- .../distributed/commands/alter_table.c | 22 +++++++++++++++++++ .../expected/schema_based_sharding.out | 6 +++++ .../regress/sql/schema_based_sharding.sql | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/src/backend/distributed/commands/alter_table.c b/src/backend/distributed/commands/alter_table.c index 3728e7470..40324cfcb 100644 --- a/src/backend/distributed/commands/alter_table.c +++ b/src/backend/distributed/commands/alter_table.c @@ -196,6 +196,7 @@ static void EnsureTableNotReferencing(Oid relationId, char conversionType); static void EnsureTableNotReferenced(Oid relationId, char conversionType); static void EnsureTableNotForeign(Oid relationId); static void EnsureTableNotPartition(Oid relationId); +static void ErrorIfColocateWithTenantTable(char *colocateWith); static TableConversionState * CreateTableConversion(TableConversionParameters *params); static void CreateDistributedTableLike(TableConversionState *con); static void CreateCitusTableLike(TableConversionState *con); @@ -437,6 +438,9 @@ AlterDistributedTable(TableConversionParameters *params) "is not distributed"))); } + ErrorIfTenantTable(params->relationId, "alter_distributed_table"); + ErrorIfColocateWithTenantTable(params->colocateWith); + EnsureTableNotForeign(params->relationId); EnsureTableNotPartition(params->relationId); EnsureHashDistributedTable(params->relationId); @@ -1182,6 +1186,24 @@ EnsureTableNotPartition(Oid relationId) } +/* + * ErrorIfColocateWithTenantTable errors out if given colocateWith text refers to + * a tenant table. + */ +void +ErrorIfColocateWithTenantTable(char *colocateWith) +{ + if (colocateWith != NULL && + !IsColocateWithDefault(colocateWith) && + !IsColocateWithNone(colocateWith)) + { + text *colocateWithTableNameText = cstring_to_text(colocateWith); + Oid colocateWithTableId = ResolveRelationId(colocateWithTableNameText, false); + ErrorIfTenantTable(colocateWithTableId, "colocate_with"); + } +} + + TableConversionState * CreateTableConversion(TableConversionParameters *params) { diff --git a/src/test/regress/expected/schema_based_sharding.out b/src/test/regress/expected/schema_based_sharding.out index 0a529398a..84ca562e3 100644 --- a/src/test/regress/expected/schema_based_sharding.out +++ b/src/test/regress/expected/schema_based_sharding.out @@ -83,6 +83,12 @@ ERROR: test_table is not allowed for colocate_with because it is a tenant table -- verify we don't allow undistribute_table for tenant tables SELECT undistribute_table('tenant_2.test_table'); ERROR: test_table is not allowed for undistribute_table because it is a tenant table +-- verify we don't allow alter_distributed_table for tenant tables +SELECT alter_distributed_table('tenant_2.test_table', colocate_with => 'none'); +ERROR: test_table is not allowed for alter_distributed_table because it is a tenant table +-- verify we also don't allow colocate_with a tenant table +SELECT alter_distributed_table('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); +ERROR: test_table is not allowed for colocate_with because it is a tenant table -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_3'); diff --git a/src/test/regress/sql/schema_based_sharding.sql b/src/test/regress/sql/schema_based_sharding.sql index 469145a91..f24628731 100644 --- a/src/test/regress/sql/schema_based_sharding.sql +++ b/src/test/regress/sql/schema_based_sharding.sql @@ -61,6 +61,10 @@ SELECT update_distributed_table_colocation('tenant_2.test_table', colocate_with SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); -- verify we don't allow undistribute_table for tenant tables SELECT undistribute_table('tenant_2.test_table'); +-- verify we don't allow alter_distributed_table for tenant tables +SELECT alter_distributed_table('tenant_2.test_table', colocate_with => 'none'); +-- verify we also don't allow colocate_with a tenant table +SELECT alter_distributed_table('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema From fccfee08b6b29fba2a87127cc1cc9f85295a45a0 Mon Sep 17 00:00:00 2001 From: ahmet gedemenli Date: Fri, 2 Jun 2023 14:28:58 +0300 Subject: [PATCH 4/5] Style --- src/backend/distributed/commands/schema_based_sharding.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/distributed/commands/schema_based_sharding.c b/src/backend/distributed/commands/schema_based_sharding.c index ce33b51a1..f33debb34 100644 --- a/src/backend/distributed/commands/schema_based_sharding.c +++ b/src/backend/distributed/commands/schema_based_sharding.c @@ -254,6 +254,6 @@ ErrorIfTenantTable(Oid relationId, char *operationName) if (IsTenantSchema(get_rel_namespace(relationId))) { ereport(ERROR, (errmsg("%s is not allowed for %s because it is a tenant table", - get_rel_name(relationId), operationName))); + get_rel_name(relationId), operationName))); } } From 2bd6ff0e93c09f32fe534a40554bbdac23d3f390 Mon Sep 17 00:00:00 2001 From: ahmet gedemenli Date: Fri, 2 Jun 2023 15:25:14 +0300 Subject: [PATCH 5/5] Use schema name in the error msg --- .../distributed/commands/schema_based_sharding.c | 3 ++- src/test/regress/expected/schema_based_sharding.out | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/backend/distributed/commands/schema_based_sharding.c b/src/backend/distributed/commands/schema_based_sharding.c index f33debb34..b7f1ba70a 100644 --- a/src/backend/distributed/commands/schema_based_sharding.c +++ b/src/backend/distributed/commands/schema_based_sharding.c @@ -254,6 +254,7 @@ ErrorIfTenantTable(Oid relationId, char *operationName) if (IsTenantSchema(get_rel_namespace(relationId))) { ereport(ERROR, (errmsg("%s is not allowed for %s because it is a tenant table", - get_rel_name(relationId), operationName))); + generate_qualified_relation_name(relationId), + operationName))); } } diff --git a/src/test/regress/expected/schema_based_sharding.out b/src/test/regress/expected/schema_based_sharding.out index 84ca562e3..e55b1cd2b 100644 --- a/src/test/regress/expected/schema_based_sharding.out +++ b/src/test/regress/expected/schema_based_sharding.out @@ -76,19 +76,19 @@ SELECT citus_add_local_table_to_metadata('tenant_2.test_table'); ERROR: table "test_table" is already distributed -- verify we don't allow update_distributed_table_colocation for tenant tables SELECT update_distributed_table_colocation('tenant_2.test_table', colocate_with => 'none'); -ERROR: test_table is not allowed for update_distributed_table_colocation because it is a tenant table +ERROR: tenant_2.test_table is not allowed for update_distributed_table_colocation because it is a tenant table -- verify we also don't allow colocate_with a tenant table SELECT update_distributed_table_colocation('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); -ERROR: test_table is not allowed for colocate_with because it is a tenant table +ERROR: tenant_2.test_table is not allowed for colocate_with because it is a tenant table -- verify we don't allow undistribute_table for tenant tables SELECT undistribute_table('tenant_2.test_table'); -ERROR: test_table is not allowed for undistribute_table because it is a tenant table +ERROR: tenant_2.test_table is not allowed for undistribute_table because it is a tenant table -- verify we don't allow alter_distributed_table for tenant tables SELECT alter_distributed_table('tenant_2.test_table', colocate_with => 'none'); -ERROR: test_table is not allowed for alter_distributed_table because it is a tenant table +ERROR: tenant_2.test_table is not allowed for alter_distributed_table because it is a tenant table -- verify we also don't allow colocate_with a tenant table SELECT alter_distributed_table('regular_schema.test_table', colocate_with => 'tenant_2.test_table'); -ERROR: test_table is not allowed for colocate_with because it is a tenant table +ERROR: tenant_2.test_table is not allowed for colocate_with because it is a tenant table -- (on coordinator) verify that colocation id is set for empty tenants too SELECT colocationid > 0 FROM pg_dist_tenant_schema WHERE schemaid::regnamespace::text IN ('tenant_1', 'tenant_3');