From bbe1b161252e1cd7d77540c87fe42e333b6cdefd Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Wed, 9 Mar 2022 14:43:05 +0300 Subject: [PATCH] Check whether the object has unsupported or circular dependency --- src/backend/distributed/commands/aggregate.c | 40 +----- .../citus_add_local_table_to_metadata.c | 1 - .../commands/create_distributed_table.c | 1 - .../distributed/commands/dependencies.c | 62 +++++++++ src/backend/distributed/commands/function.c | 45 +------ src/backend/distributed/commands/table.c | 1 - src/backend/distributed/metadata/dependency.c | 104 +++++++++------ src/include/distributed/commands.h | 1 - src/include/distributed/metadata/dependency.h | 6 +- .../regress/expected/aggregate_support.out | 12 +- .../regress/expected/distributed_types.out | 6 + .../regress/expected/function_propagation.out | 121 +++++++++++++++--- .../regress/expected/local_table_join.out | 7 + .../regress/expected/multi_create_fdw.out | 7 + src/test/regress/expected/pg13.out | 10 +- src/test/regress/sql/distributed_types.sql | 6 + src/test/regress/sql/function_propagation.sql | 76 +++++++++++ src/test/regress/sql/local_table_join.sql | 3 + src/test/regress/sql/multi_create_fdw.sql | 4 + 19 files changed, 363 insertions(+), 150 deletions(-) diff --git a/src/backend/distributed/commands/aggregate.c b/src/backend/distributed/commands/aggregate.c index 4c25f41bf..f6161df49 100644 --- a/src/backend/distributed/commands/aggregate.c +++ b/src/backend/distributed/commands/aggregate.c @@ -57,42 +57,12 @@ PostprocessDefineAggregateStmt(Node *node, const char *queryString) EnsureSequentialMode(OBJECT_AGGREGATE); - ObjectAddress *undistributableDependency = GetUndistributableDependency( - &address); - if (undistributableDependency != NULL) + /* If the aggregate has any unsupported dependency, create it locally */ + DeferredErrorMessage *depError = DeferErrorIfHasUnsupportedDependency(&address); + + if (depError != NULL) { - if (SupportedDependencyByCitus(undistributableDependency)) - { - /* - * Citus can't distribute some relations as dependency, although those - * types as supported by Citus. So we can use get_rel_name directly - */ - RangeVar *aggRangeVar = makeRangeVarFromNameList(stmt->defnames); - char *aggName = aggRangeVar->relname; - char *dependentRelationName = - get_rel_name(undistributableDependency->objectId); - - ereport(WARNING, (errmsg("Citus can't distribute aggregate \"%s\" having " - "dependency on non-distributed relation \"%s\"", - aggName, dependentRelationName), - errdetail("Aggregate will be created only locally"), - errhint("To distribute aggregate, distribute dependent " - "relations first. Then, re-create the aggregate"))); - } - else - { - char *objectType = NULL; - #if PG_VERSION_NUM >= PG_VERSION_14 - objectType = getObjectTypeDescription(undistributableDependency, false); - #else - objectType = getObjectTypeDescription(undistributableDependency); - #endif - ereport(WARNING, (errmsg("Citus can't distribute functions having " - "dependency on unsupported object of type \"%s\"", - objectType), - errdetail("Aggregate will be created only locally"))); - } - + RaiseDeferredError(depError, WARNING); return NIL; } diff --git a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c index 6752a3ee3..e447a2be1 100644 --- a/src/backend/distributed/commands/citus_add_local_table_to_metadata.c +++ b/src/backend/distributed/commands/citus_add_local_table_to_metadata.c @@ -318,7 +318,6 @@ CreateCitusLocalTable(Oid relationId, bool cascadeViaForeignKeys, bool autoConve * Ensure dependencies exist as we will create shell table on the other nodes * in the MX case. */ - EnsureRelationDependenciesCanBeDistributed(&tableAddress); EnsureDependenciesExistOnAllNodes(&tableAddress); /* diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index 64734fff8..c639d836c 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -444,7 +444,6 @@ CreateDistributedTable(Oid relationId, char *distributionColumnName, ObjectAddress tableAddress = { 0 }; ObjectAddressSet(tableAddress, RelationRelationId, relationId); - EnsureRelationDependenciesCanBeDistributed(&tableAddress); EnsureDependenciesExistOnAllNodes(&tableAddress); char replicationModel = DecideReplicationModel(distributionMethod, diff --git a/src/backend/distributed/commands/dependencies.c b/src/backend/distributed/commands/dependencies.c index fe6e651fa..01f561b65 100644 --- a/src/backend/distributed/commands/dependencies.c +++ b/src/backend/distributed/commands/dependencies.c @@ -31,6 +31,8 @@ typedef bool (*AddressPredicate)(const ObjectAddress *); +static void EnsureDependenciesCanBeDistributed(const ObjectAddress *relationAddress); +static void ErrorIfCircularDependencyExists(const ObjectAddress *objectAddress); static int ObjectAddressComparator(const void *a, const void *b); static List * GetDependencyCreateDDLCommands(const ObjectAddress *dependency); static List * FilterObjectAddressListByPredicate(List *objectAddressList, @@ -56,6 +58,12 @@ EnsureDependenciesExistOnAllNodes(const ObjectAddress *target) List *dependenciesWithCommands = NIL; List *ddlCommands = NULL; + /* + * If there is any unsupported dependency or circular dependency exists, Citus can + * not ensure dependencies will exist on all nodes. + */ + EnsureDependenciesCanBeDistributed(target); + /* collect all dependencies in creation order and get their ddl commands */ List *dependencies = GetDependenciesForObject(target); ObjectAddress *dependency = NULL; @@ -135,6 +143,60 @@ EnsureDependenciesExistOnAllNodes(const ObjectAddress *target) } +/* + * EnsureDependenciesCanBeDistributed ensures all dependencies of the given object + * can be distributed. + */ +static void +EnsureDependenciesCanBeDistributed(const ObjectAddress *objectAddress) +{ + /* If the object circularcly depends to itself, Citus can not handle it */ + ErrorIfCircularDependencyExists(objectAddress); + + /* If the object has any unsupported dependency, error out */ + DeferredErrorMessage *depError = DeferErrorIfHasUnsupportedDependency(objectAddress); + + if (depError != NULL) + { + RaiseDeferredError(depError, ERROR); + } +} + + +/* + * ErrorIfCircularDependencyExists checks whether given object has circular dependency + * with itself via existing objects of pg_dist_object. + */ +static void +ErrorIfCircularDependencyExists(const ObjectAddress *objectAddress) +{ + List *dependencies = GetAllSupportedDependenciesForObject(objectAddress); + + ObjectAddress *dependency = NULL; + foreach_ptr(dependency, dependencies) + { + if (dependency->classId == objectAddress->classId && + dependency->objectId == objectAddress->objectId && + dependency->objectSubId == objectAddress->objectSubId) + { + char *objectDescription = NULL; + + #if PG_VERSION_NUM >= PG_VERSION_14 + objectDescription = getObjectDescription(objectAddress, false); + #else + objectDescription = getObjectDescription(objectAddress); + #endif + + ereport(ERROR, (errmsg("Citus can not handle circular dependencies " + "between distributed objects"), + errdetail("\"%s\" circularly depends itself, resolve " + "circular dependency first", + objectDescription))); + } + } +} + + /* * Copied from PG object_address_comparator function to compare ObjectAddresses. */ diff --git a/src/backend/distributed/commands/function.c b/src/backend/distributed/commands/function.c index 11729a21f..2c0dfd6bb 100644 --- a/src/backend/distributed/commands/function.c +++ b/src/backend/distributed/commands/function.c @@ -1365,47 +1365,12 @@ PostprocessCreateFunctionStmt(Node *node, const char *queryString) return NIL; } - /* - * This check should have been valid for all objects not only for functions. Though, - * we do this limited check for now as functions are more likely to be used with - * such dependencies, and we want to scope it for now. - */ - ObjectAddress *undistributableDependency = GetUndistributableDependency( - &functionAddress); - if (undistributableDependency != NULL) + /* If the function has any unsupported dependency, create it locally */ + DeferredErrorMessage *errMsg = DeferErrorIfHasUnsupportedDependency(&functionAddress); + + if (errMsg != NULL) { - if (SupportedDependencyByCitus(undistributableDependency)) - { - /* - * Citus can't distribute some relations as dependency, although those - * types as supported by Citus. So we can use get_rel_name directly - */ - RangeVar *functionRangeVar = makeRangeVarFromNameList(stmt->funcname); - char *functionName = functionRangeVar->relname; - char *dependentRelationName = - get_rel_name(undistributableDependency->objectId); - - ereport(WARNING, (errmsg("Citus can't distribute function \"%s\" having " - "dependency on non-distributed relation \"%s\"", - functionName, dependentRelationName), - errdetail("Function will be created only locally"), - errhint("To distribute function, distribute dependent " - "relations first. Then, re-create the function"))); - } - else - { - char *objectType = NULL; - #if PG_VERSION_NUM >= PG_VERSION_14 - objectType = getObjectTypeDescription(undistributableDependency, false); - #else - objectType = getObjectTypeDescription(undistributableDependency); - #endif - ereport(WARNING, (errmsg("Citus can't distribute functions having " - "dependency on unsupported object of type \"%s\"", - objectType), - errdetail("Function will be created only locally"))); - } - + RaiseDeferredError(errMsg, WARNING); return NIL; } diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index fe1e21daf..4bf1ff373 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -1955,7 +1955,6 @@ PostprocessAlterTableStmt(AlterTableStmt *alterTableStatement) /* changing a relation could introduce new dependencies */ ObjectAddress tableAddress = { 0 }; ObjectAddressSet(tableAddress, RelationRelationId, relationId); - EnsureRelationDependenciesCanBeDistributed(&tableAddress); EnsureDependenciesExistOnAllNodes(&tableAddress); } diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c index 5994c5d51..73d41d811 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -136,6 +136,7 @@ static void CollectObjectAddress(ObjectAddressCollector *collector, const ObjectAddress *address); static bool IsObjectAddressCollected(ObjectAddress findAddress, ObjectAddressCollector *collector); +static ObjectAddress * GetUndistributableDependency(const ObjectAddress *objectAddress); static void MarkObjectVisited(ObjectAddressCollector *collector, ObjectAddress target); static bool TargetObjectVisited(ObjectAddressCollector *collector, @@ -742,51 +743,70 @@ SupportedDependencyByCitus(const ObjectAddress *address) /* - * EnsureRelationDependenciesCanBeDistributed ensures all dependencies of the relation - * can be distributed. + * DeferErrorIfHasUnsupportedDependency returns deferred error message if the given + * object has any undistributable dependency. */ -void -EnsureRelationDependenciesCanBeDistributed(ObjectAddress *relationAddress) +DeferredErrorMessage * +DeferErrorIfHasUnsupportedDependency(const ObjectAddress *objectAddress) { - ObjectAddress *undistributableDependency = - GetUndistributableDependency(relationAddress); + ObjectAddress *undistributableDependency = GetUndistributableDependency( + objectAddress); - if (undistributableDependency != NULL) + if (undistributableDependency == NULL) { - char *tableName = get_rel_name(relationAddress->objectId); - - if (SupportedDependencyByCitus(undistributableDependency)) - { - /* - * Citus can't distribute some relations as dependency, although those - * types as supported by Citus. So we can use get_rel_name directly - * - * For now the relations are the only type that is supported by Citus - * but can not be distributed as dependency, though we've added an - * explicit check below as well to not to break the logic here in case - * GetUndistributableDependency changes. - */ - if (getObjectClass(undistributableDependency) == OCLASS_CLASS) - { - char *dependentRelationName = get_rel_name( - undistributableDependency->objectId); - - ereport(ERROR, (errmsg("Relation \"%s\" has dependency to a table" - " \"%s\" that is not in Citus' metadata", - tableName, dependentRelationName), - errhint("Distribute dependent relation first."))); - } - } - - char *objectType = NULL; - #if PG_VERSION_NUM >= PG_VERSION_14 - objectType = getObjectDescription(undistributableDependency, false); - #else - objectType = getObjectDescription(undistributableDependency); - #endif - ereport(ERROR, (errmsg("Relation \"%s\" has dependency on unsupported " - "object \"%s\"", tableName, objectType))); + return NULL; } + + char *objectDescription = NULL; + char *dependencyDescription = NULL; + StringInfo errorInfo = makeStringInfo(); + StringInfo detailInfo = makeStringInfo(); + + #if PG_VERSION_NUM >= PG_VERSION_14 + objectDescription = getObjectDescription(objectAddress, false); + dependencyDescription = getObjectDescription(undistributableDependency, false); + #else + objectDescription = getObjectDescription(objectAddress); + dependencyDescription = getObjectDescription(undistributableDependency); + #endif + + /* + * If the given object is a procedure, we want to create it locally, + * so provide that information in the error detail. + */ + if (getObjectClass(objectAddress) == OCLASS_PROC) + { + appendStringInfo(detailInfo, "\"%s\" will be created only locally", + objectDescription); + } + + if (SupportedDependencyByCitus(undistributableDependency)) + { + StringInfo hintInfo = makeStringInfo(); + + appendStringInfo(errorInfo, "\"%s\" has dependency to \"%s\" that is not in " + "Citus' metadata", + objectDescription, + dependencyDescription); + + appendStringInfo(hintInfo, "Distribute \"%s\" first to distribute \"%s\"", + dependencyDescription, + objectDescription); + + return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, + errorInfo->data, + strlen(detailInfo->data) == 0 ? NULL : detailInfo->data, + hintInfo->data); + } + + appendStringInfo(errorInfo, "\"%s\" has dependency on unsupported " + "object \"%s\"", objectDescription, + dependencyDescription); + + return DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, + errorInfo->data, + strlen(detailInfo->data) == 0 ? NULL : detailInfo->data, + NULL); } @@ -794,8 +814,8 @@ EnsureRelationDependenciesCanBeDistributed(ObjectAddress *relationAddress) * GetUndistributableDependency checks whether object has any non-distributable * dependency. If any one found, it will be returned. */ -ObjectAddress * -GetUndistributableDependency(ObjectAddress *objectAddress) +static ObjectAddress * +GetUndistributableDependency(const ObjectAddress *objectAddress) { List *dependencies = GetAllDependenciesForObject(objectAddress); ObjectAddress *dependency = NULL; diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index 8ec3d9e8a..660220324 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -267,7 +267,6 @@ extern List * PreprocessCreateFunctionStmt(Node *stmt, const char *queryString, ProcessUtilityContext processUtilityContext); extern List * PostprocessCreateFunctionStmt(Node *stmt, const char *queryString); -extern ObjectAddress * GetUndistributableDependency(ObjectAddress *functionAddress); extern ObjectAddress CreateFunctionStmtObjectAddress(Node *stmt, bool missing_ok); extern ObjectAddress DefineAggregateStmtObjectAddress(Node *stmt, diff --git a/src/include/distributed/metadata/dependency.h b/src/include/distributed/metadata/dependency.h index 92714f6cb..af11c5f2a 100644 --- a/src/include/distributed/metadata/dependency.h +++ b/src/include/distributed/metadata/dependency.h @@ -15,14 +15,16 @@ #include "postgres.h" #include "catalog/objectaddress.h" +#include "distributed/errormessage.h" #include "nodes/pg_list.h" extern List * GetUniqueDependenciesList(List *objectAddressesList); extern List * GetDependenciesForObject(const ObjectAddress *target); extern List * GetAllSupportedDependenciesForObject(const ObjectAddress *target); extern List * GetAllDependenciesForObject(const ObjectAddress *target); -extern void EnsureRelationDependenciesCanBeDistributed(ObjectAddress *relationAddress); -extern ObjectAddress * GetUndistributableDependency(ObjectAddress *target); +extern DeferredErrorMessage * DeferErrorIfHasUnsupportedDependency(const + ObjectAddress * + objectAddress); extern List * OrderObjectAddressListInDependencyOrder(List *objectAddressList); extern bool SupportedDependencyByCitus(const ObjectAddress *address); extern List * GetPgDependTuplesForDependingObjects(Oid targetObjectClassId, diff --git a/src/test/regress/expected/aggregate_support.out b/src/test/regress/expected/aggregate_support.out index 80ac1bb7a..1459c139b 100644 --- a/src/test/regress/expected/aggregate_support.out +++ b/src/test/regress/expected/aggregate_support.out @@ -1089,14 +1089,14 @@ LEFT JOIN ref_table ON TRUE; create table dummy_tbl (a int); create function dummy_fnc(a dummy_tbl, d double precision) RETURNS dummy_tbl AS $$SELECT 1;$$ LANGUAGE sql; -WARNING: Citus can't distribute function "dummy_fnc" having dependency on non-distributed relation "dummy_tbl" -DETAIL: Function will be created only locally -HINT: To distribute function, distribute dependent relations first. Then, re-create the function +WARNING: "function dummy_fnc(dummy_tbl,double precision)" has dependency to "table dummy_tbl" that is not in Citus' metadata +DETAIL: "function dummy_fnc(dummy_tbl,double precision)" will be created only locally +HINT: Distribute "table dummy_tbl" first to distribute "function dummy_fnc(dummy_tbl,double precision)" -- should give warning and create aggregate local only create aggregate dependent_agg (float8) (stype=dummy_tbl, sfunc=dummy_fnc); -WARNING: Citus can't distribute aggregate "dependent_agg" having dependency on non-distributed relation "dummy_tbl" -DETAIL: Aggregate will be created only locally -HINT: To distribute aggregate, distribute dependent relations first. Then, re-create the aggregate +WARNING: "function dependent_agg(double precision)" has dependency to "table dummy_tbl" that is not in Citus' metadata +DETAIL: "function dependent_agg(double precision)" will be created only locally +HINT: Distribute "table dummy_tbl" first to distribute "function dependent_agg(double precision)" -- clear and try again with distributed table DROP TABLE dummy_tbl CASCADE; NOTICE: drop cascades to 2 other objects diff --git a/src/test/regress/expected/distributed_types.out b/src/test/regress/expected/distributed_types.out index c20326820..69684c702 100644 --- a/src/test/regress/expected/distributed_types.out +++ b/src/test/regress/expected/distributed_types.out @@ -560,6 +560,12 @@ SHOW citus.multi_shard_modify_mode; (1 row) COMMIT; +-- Show that PG does not allow adding a circular dependency btw types +-- We added here to make sure we can catch it if PG changes it's behaviour +CREATE TYPE circ_type1 AS (a int); +CREATE TYPE circ_type2 AS (a int, b circ_type1); +ALTER TYPE circ_type1 ADD ATTRIBUTE b circ_type2; +ERROR: composite type circ_type1 cannot be made a member of itself -- clear objects SET client_min_messages TO error; -- suppress cascading objects dropping DROP SCHEMA type_tests CASCADE; diff --git a/src/test/regress/expected/function_propagation.out b/src/test/regress/expected/function_propagation.out index cf87f43fd..dc6cf5d69 100644 --- a/src/test/regress/expected/function_propagation.out +++ b/src/test/regress/expected/function_propagation.out @@ -134,9 +134,9 @@ BEGIN return 1; END; $$; -WARNING: Citus can't distribute function "func_4" having dependency on non-distributed relation "function_prop_table" -DETAIL: Function will be created only locally -HINT: To distribute function, distribute dependent relations first. Then, re-create the function +WARNING: "function func_4(function_prop_table)" has dependency to "table function_prop_table" that is not in Citus' metadata +DETAIL: "function func_4(function_prop_table)" will be created only locally +HINT: Distribute "table function_prop_table" first to distribute "function func_4(function_prop_table)" CREATE OR REPLACE FUNCTION func_5(param_1 int) RETURNS function_prop_table LANGUAGE plpgsql AS @@ -145,9 +145,9 @@ BEGIN return 1; END; $$; -WARNING: Citus can't distribute function "func_5" having dependency on non-distributed relation "function_prop_table" -DETAIL: Function will be created only locally -HINT: To distribute function, distribute dependent relations first. Then, re-create the function +WARNING: "function func_5(integer)" has dependency to "table function_prop_table" that is not in Citus' metadata +DETAIL: "function func_5(integer)" will be created only locally +HINT: Distribute "table function_prop_table" first to distribute "function func_5(integer)" -- Functions can be created with distributed table dependency SELECT create_distributed_table('function_prop_table', 'a'); create_distributed_table @@ -186,8 +186,8 @@ BEGIN return 1; END; $$; -WARNING: Citus can't distribute functions having dependency on unsupported object of type "view" -DETAIL: Function will be created only locally +WARNING: "function func_7(function_prop_view)" has dependency on unsupported object "view function_prop_view" +DETAIL: "function func_7(function_prop_view)" will be created only locally CREATE OR REPLACE FUNCTION func_8(param_1 int) RETURNS function_prop_view LANGUAGE plpgsql AS @@ -196,8 +196,8 @@ BEGIN return 1; END; $$; -WARNING: Citus can't distribute functions having dependency on unsupported object of type "view" -DETAIL: Function will be created only locally +WARNING: "function func_8(integer)" has dependency on unsupported object "view function_prop_view" +DETAIL: "function func_8(integer)" will be created only locally -- Check within transaction BEGIN; CREATE TYPE type_in_transaction AS (a int, b int); @@ -435,8 +435,8 @@ BEGIN; CREATE TABLE table_to_prop_func_3(id int, col_1 int default func_in_transaction_3(NULL::non_dist_table)); -- It should error out as there is a non-distributed table dependency SELECT create_distributed_table('table_to_prop_func_3','id'); -ERROR: Relation "table_to_prop_func_3" has dependency to a table "non_dist_table" that is not in Citus' metadata -HINT: Distribute dependent relation first. +ERROR: "table table_to_prop_func_3" has dependency to "table non_dist_table" that is not in Citus' metadata +HINT: Distribute "table non_dist_table" first to distribute "table table_to_prop_func_3" COMMIT; -- Adding a column with default value should propagate the function BEGIN; @@ -497,8 +497,8 @@ BEGIN; (1 row) ALTER TABLE table_to_dist ADD COLUMN col_1 int default function_propagation_schema.non_dist_func(NULL::non_dist_table_for_function); -ERROR: Relation "table_to_dist" has dependency to a table "non_dist_table_for_function" that is not in Citus' metadata -HINT: Distribute dependent relation first. +ERROR: "table table_to_dist" has dependency to "table non_dist_table_for_function" that is not in Citus' metadata +HINT: Distribute "table non_dist_table_for_function" first to distribute "table table_to_dist" ROLLBACK; -- Adding multiple columns with default values should propagate the function BEGIN; @@ -723,8 +723,8 @@ BEGIN; CREATE TABLE table_to_prop_func_9(id int, col_1 int check (func_in_transaction_11(col_1, NULL::local_table_for_const))); -- It should error out since there is non-distributed table dependency exists SELECT create_distributed_table('table_to_prop_func_9', 'id'); -ERROR: Relation "table_to_prop_func_9" has dependency to a table "local_table_for_const" that is not in Citus' metadata -HINT: Distribute dependent relation first. +ERROR: "table table_to_prop_func_9" has dependency to "table local_table_for_const" that is not in Citus' metadata +HINT: Distribute "table local_table_for_const" first to distribute "table table_to_prop_func_9" COMMIT; -- Show that function as a part of generated always is supporte BEGIN; @@ -1188,6 +1188,95 @@ SELECT distribution_argument_index, colocationid, force_delegation FROM pg_catal | | (1 row) +-- Show that causing circular dependency via functions and default values are not allowed +CREATE TABLE table_1_for_circ_dep(id int); +select create_distributed_table('table_1_for_circ_dep','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION func_1_for_circ_dep(col_1 table_1_for_circ_dep) +RETURNS int +LANGUAGE plpgsql AS +$$ +BEGIN +return 1; +END; +$$; +CREATE TABLE table_2_for_circ_dep(id int, col_2 int default func_1_for_circ_dep(NULL::table_1_for_circ_dep)); +select create_distributed_table('table_2_for_circ_dep','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION func_2_for_circ_dep(col_3 table_2_for_circ_dep) +RETURNS int +LANGUAGE plpgsql AS +$$ +BEGIN +return 1; +END; +$$; +-- It should error out due to circular dependency +ALTER TABLE table_1_for_circ_dep ADD COLUMN col_2 int default func_2_for_circ_dep(NULL::table_2_for_circ_dep); +ERROR: Citus can not handle circular dependencies between distributed objects +DETAIL: "table table_1_for_circ_dep" circularly depends itself, resolve circular dependency first +-- Show that causing circular dependency via functions and constraints are not allowed +CREATE TABLE table_1_for_circ_dep_2(id int, col_1 int); +select create_distributed_table('table_1_for_circ_dep_2','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION func_1_for_circ_dep_2(param_1 int, param_2 table_1_for_circ_dep_2) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; +CREATE TABLE table_2_for_circ_dep_2(id int, col_1 int check (func_1_for_circ_dep_2(col_1, NULL::table_1_for_circ_dep_2))); +select create_distributed_table('table_2_for_circ_dep_2','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION func_2_for_circ_dep_2(param_1 int, param_2 table_2_for_circ_dep_2) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; +-- It should error out due to circular dependency +ALTER TABLE table_1_for_circ_dep_2 ADD CONSTRAINT check_const_for_circ check (func_2_for_circ_dep_2(col_1, NULL::table_2_for_circ_dep_2)); +ERROR: Citus can not handle circular dependencies between distributed objects +DETAIL: "table table_1_for_circ_dep_2" circularly depends itself, resolve circular dependency first +-- Show that causing circular dependency via functions and create_distributed_table are not allowed +CREATE TABLE table_1_for_circ_dep_3(id int, col_1 int); +CREATE OR REPLACE FUNCTION func_for_circ_dep_3(param_1 int, param_2 table_1_for_circ_dep_3) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; +WARNING: "function func_for_circ_dep_3(integer,table_1_for_circ_dep_3)" has dependency to "table table_1_for_circ_dep_3" that is not in Citus' metadata +DETAIL: "function func_for_circ_dep_3(integer,table_1_for_circ_dep_3)" will be created only locally +HINT: Distribute "table table_1_for_circ_dep_3" first to distribute "function func_for_circ_dep_3(integer,table_1_for_circ_dep_3)" +CREATE TABLE table_2_for_circ_dep_3(id int, col_1 int check (func_for_circ_dep_3(col_1, NULL::table_1_for_circ_dep_3))); +ALTER TABLE table_1_for_circ_dep_3 ADD COLUMN col_2 table_2_for_circ_dep_3; +-- It should error out due to circular dependency +SELECT create_distributed_table('table_1_for_circ_dep_3','id'); +ERROR: Citus can not handle circular dependencies between distributed objects +DETAIL: "table table_1_for_circ_dep_3" circularly depends itself, resolve circular dependency first RESET search_path; SET client_min_messages TO WARNING; DROP SCHEMA function_propagation_schema CASCADE; diff --git a/src/test/regress/expected/local_table_join.out b/src/test/regress/expected/local_table_join.out index d71aee3cf..f2abb1066 100644 --- a/src/test/regress/expected/local_table_join.out +++ b/src/test/regress/expected/local_table_join.out @@ -71,7 +71,14 @@ $$); (localhost,57638,t,"CREATE FOREIGN DATA WRAPPER") (2 rows) +-- Since we are assuming fdw should be part of the extension, add it manually. +ALTER EXTENSION citus ADD FOREIGN DATA WRAPPER fake_fdw_1; +NOTICE: Citus does not propagate adding/dropping member objects +HINT: You can add/drop the member objects on the workers as well. CREATE SERVER fake_fdw_server_1 FOREIGN DATA WRAPPER fake_fdw_1; +ALTER EXTENSION citus DROP FOREIGN DATA WRAPPER fake_fdw_1; +NOTICE: Citus does not propagate adding/dropping member objects +HINT: You can add/drop the member objects on the workers as well. CREATE FOREIGN TABLE foreign_table ( key int, value text diff --git a/src/test/regress/expected/multi_create_fdw.out b/src/test/regress/expected/multi_create_fdw.out index 453dda1b9..e15d17546 100644 --- a/src/test/regress/expected/multi_create_fdw.out +++ b/src/test/regress/expected/multi_create_fdw.out @@ -8,4 +8,11 @@ RETURNS fdw_handler AS 'citus' LANGUAGE C STRICT; CREATE FOREIGN DATA WRAPPER fake_fdw HANDLER fake_fdw_handler; +-- Since we are assuming fdw should be part of the extension, add and drop it manually. +ALTER EXTENSION citus ADD FOREIGN DATA WRAPPER fake_fdw; +NOTICE: Citus does not propagate adding/dropping member objects +HINT: You can add/drop the member objects on the workers as well. CREATE SERVER fake_fdw_server FOREIGN DATA WRAPPER fake_fdw; +ALTER EXTENSION citus DROP FOREIGN DATA WRAPPER fake_fdw; +NOTICE: Citus does not propagate adding/dropping member objects +HINT: You can add/drop the member objects on the workers as well. diff --git a/src/test/regress/expected/pg13.out b/src/test/regress/expected/pg13.out index 44664fb83..cceab4418 100644 --- a/src/test/regress/expected/pg13.out +++ b/src/test/regress/expected/pg13.out @@ -145,13 +145,13 @@ CREATE TYPE myvarchar; CREATE FUNCTION myvarcharin(cstring, oid, integer) RETURNS myvarchar LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharin'; NOTICE: return type myvarchar is only a shell -WARNING: Citus can't distribute functions having dependency on unsupported object of type "type" -DETAIL: Function will be created only locally +WARNING: "function myvarcharin(cstring,oid,integer)" has dependency on unsupported object "type myvarchar" +DETAIL: "function myvarcharin(cstring,oid,integer)" will be created only locally CREATE FUNCTION myvarcharout(myvarchar) RETURNS cstring LANGUAGE internal IMMUTABLE PARALLEL SAFE STRICT AS 'varcharout'; NOTICE: argument type myvarchar is only a shell -WARNING: Citus can't distribute functions having dependency on unsupported object of type "type" -DETAIL: Function will be created only locally +WARNING: "function myvarcharout(myvarchar)" has dependency on unsupported object "type myvarchar" +DETAIL: "function myvarcharout(myvarchar)" will be created only locally CREATE TYPE myvarchar ( input = myvarcharin, output = myvarcharout, @@ -164,7 +164,7 @@ CREATE TABLE my_table (a int, b myvarchar); -- """Add ALTER TYPE options useful for extensions, -- like TOAST and I/O functions control (Tomas Vondra, Tom Lane)""" SELECT create_distributed_table('my_table', 'a'); -ERROR: Relation "my_table" has dependency on unsupported object "type myvarchar" +ERROR: "table my_table" has dependency on unsupported object "type myvarchar" CREATE TABLE test_table(a int, b tsvector); SELECT create_distributed_table('test_table', 'a'); create_distributed_table diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index e22533101..944bc9702 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -329,6 +329,12 @@ SELECT create_distributed_table('immediate_table', 'a'); SHOW citus.multi_shard_modify_mode; COMMIT; +-- Show that PG does not allow adding a circular dependency btw types +-- We added here to make sure we can catch it if PG changes it's behaviour +CREATE TYPE circ_type1 AS (a int); +CREATE TYPE circ_type2 AS (a int, b circ_type1); +ALTER TYPE circ_type1 ADD ATTRIBUTE b circ_type2; + -- clear objects SET client_min_messages TO error; -- suppress cascading objects dropping DROP SCHEMA type_tests CASCADE; diff --git a/src/test/regress/sql/function_propagation.sql b/src/test/regress/sql/function_propagation.sql index 18137a67d..0d7151e3f 100644 --- a/src/test/regress/sql/function_propagation.sql +++ b/src/test/regress/sql/function_propagation.sql @@ -756,6 +756,82 @@ SELECT create_distributed_function('func_to_colocate(int)'); -- show that the pg_dist_object fields are gone SELECT distribution_argument_index, colocationid, force_delegation FROM pg_catalog.pg_dist_object WHERE objid = 'func_to_colocate'::regproc; + +-- Show that causing circular dependency via functions and default values are not allowed +CREATE TABLE table_1_for_circ_dep(id int); +select create_distributed_table('table_1_for_circ_dep','id'); + +CREATE OR REPLACE FUNCTION func_1_for_circ_dep(col_1 table_1_for_circ_dep) +RETURNS int +LANGUAGE plpgsql AS +$$ +BEGIN +return 1; +END; +$$; + +CREATE TABLE table_2_for_circ_dep(id int, col_2 int default func_1_for_circ_dep(NULL::table_1_for_circ_dep)); +select create_distributed_table('table_2_for_circ_dep','id'); +CREATE OR REPLACE FUNCTION func_2_for_circ_dep(col_3 table_2_for_circ_dep) +RETURNS int +LANGUAGE plpgsql AS +$$ +BEGIN +return 1; +END; +$$; + +-- It should error out due to circular dependency +ALTER TABLE table_1_for_circ_dep ADD COLUMN col_2 int default func_2_for_circ_dep(NULL::table_2_for_circ_dep); + + +-- Show that causing circular dependency via functions and constraints are not allowed +CREATE TABLE table_1_for_circ_dep_2(id int, col_1 int); +select create_distributed_table('table_1_for_circ_dep_2','id'); + +CREATE OR REPLACE FUNCTION func_1_for_circ_dep_2(param_1 int, param_2 table_1_for_circ_dep_2) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; + +CREATE TABLE table_2_for_circ_dep_2(id int, col_1 int check (func_1_for_circ_dep_2(col_1, NULL::table_1_for_circ_dep_2))); + +select create_distributed_table('table_2_for_circ_dep_2','id'); +CREATE OR REPLACE FUNCTION func_2_for_circ_dep_2(param_1 int, param_2 table_2_for_circ_dep_2) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; + +-- It should error out due to circular dependency +ALTER TABLE table_1_for_circ_dep_2 ADD CONSTRAINT check_const_for_circ check (func_2_for_circ_dep_2(col_1, NULL::table_2_for_circ_dep_2)); + + +-- Show that causing circular dependency via functions and create_distributed_table are not allowed +CREATE TABLE table_1_for_circ_dep_3(id int, col_1 int); + +CREATE OR REPLACE FUNCTION func_for_circ_dep_3(param_1 int, param_2 table_1_for_circ_dep_3) +RETURNS boolean +LANGUAGE plpgsql AS +$$ +BEGIN + return param_1 > 5; +END; +$$; + +CREATE TABLE table_2_for_circ_dep_3(id int, col_1 int check (func_for_circ_dep_3(col_1, NULL::table_1_for_circ_dep_3))); +ALTER TABLE table_1_for_circ_dep_3 ADD COLUMN col_2 table_2_for_circ_dep_3; + +-- It should error out due to circular dependency +SELECT create_distributed_table('table_1_for_circ_dep_3','id'); + RESET search_path; SET client_min_messages TO WARNING; DROP SCHEMA function_propagation_schema CASCADE; diff --git a/src/test/regress/sql/local_table_join.sql b/src/test/regress/sql/local_table_join.sql index f9c05789a..5eb9ece3f 100644 --- a/src/test/regress/sql/local_table_join.sql +++ b/src/test/regress/sql/local_table_join.sql @@ -43,7 +43,10 @@ CREATE FOREIGN DATA WRAPPER fake_fdw_1 HANDLER fake_fdw_handler; SELECT run_command_on_workers($$ CREATE FOREIGN DATA WRAPPER fake_fdw_1 HANDLER fake_fdw_handler; $$); +-- Since we are assuming fdw should be part of the extension, add it manually. +ALTER EXTENSION citus ADD FOREIGN DATA WRAPPER fake_fdw_1; CREATE SERVER fake_fdw_server_1 FOREIGN DATA WRAPPER fake_fdw_1; +ALTER EXTENSION citus DROP FOREIGN DATA WRAPPER fake_fdw_1; CREATE FOREIGN TABLE foreign_table ( key int, diff --git a/src/test/regress/sql/multi_create_fdw.sql b/src/test/regress/sql/multi_create_fdw.sql index 3f83ae227..ea9333781 100644 --- a/src/test/regress/sql/multi_create_fdw.sql +++ b/src/test/regress/sql/multi_create_fdw.sql @@ -13,4 +13,8 @@ AS 'citus' LANGUAGE C STRICT; CREATE FOREIGN DATA WRAPPER fake_fdw HANDLER fake_fdw_handler; + +-- Since we are assuming fdw should be part of the extension, add and drop it manually. +ALTER EXTENSION citus ADD FOREIGN DATA WRAPPER fake_fdw; CREATE SERVER fake_fdw_server FOREIGN DATA WRAPPER fake_fdw; +ALTER EXTENSION citus DROP FOREIGN DATA WRAPPER fake_fdw;