From 3c40ceef96da540314ec5306d4693fe010b0760e Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Mon, 7 Mar 2022 14:59:21 +0300 Subject: [PATCH] Update the places where we check circular dependency --- .../distributed/commands/dependencies.c | 42 ------------------- .../distributed/commands/utility_hook.c | 13 ++++++ src/backend/distributed/metadata/dependency.c | 34 +++++++++++++++ src/backend/distributed/metadata/distobject.c | 11 +++++ src/include/distributed/metadata/dependency.h | 1 + 5 files changed, 59 insertions(+), 42 deletions(-) diff --git a/src/backend/distributed/commands/dependencies.c b/src/backend/distributed/commands/dependencies.c index bc5724ad0..fe6e651fa 100644 --- a/src/backend/distributed/commands/dependencies.c +++ b/src/backend/distributed/commands/dependencies.c @@ -31,7 +31,6 @@ typedef bool (*AddressPredicate)(const ObjectAddress *); -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, @@ -57,13 +56,6 @@ EnsureDependenciesExistOnAllNodes(const ObjectAddress *target) List *dependenciesWithCommands = NIL; List *ddlCommands = NULL; - /* - * Having circular dependency between distributed objects prevents Citus from - * adding a new node. So, error out if circular dependency exists for the given - * target object. - */ - ErrorIfCircularDependencyExists(target); - /* collect all dependencies in creation order and get their ddl commands */ List *dependencies = GetDependenciesForObject(target); ObjectAddress *dependency = NULL; @@ -143,40 +135,6 @@ EnsureDependenciesExistOnAllNodes(const ObjectAddress *target) } -/* - * 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/utility_hook.c b/src/backend/distributed/commands/utility_hook.c index 91e02a8ff..a554af20a 100644 --- a/src/backend/distributed/commands/utility_hook.c +++ b/src/backend/distributed/commands/utility_hook.c @@ -58,6 +58,7 @@ #include "distributed/metadata_cache.h" #endif #include "distributed/metadata_sync.h" +#include "distributed/metadata/dependency.h" #include "distributed/metadata/distobject.h" #include "distributed/multi_executor.h" #include "distributed/multi_explain.h" @@ -656,6 +657,18 @@ ProcessUtilityInternal(PlannedStmt *pstmt, */ if (EnableDDLPropagation) { + /* + * Having circular dependency between distributed objects prevents Citus from + * adding a new node. So, error out if circular dependency exists for the given + * target object. + * + * We need to check circular dependency here in addition to checking it within + * MarkObjectDistributedLocally since altering already distributed object won't + * call MarkObjectDistributedLocally but will hit the check here. + */ + ObjectAddress targetObject = GetObjectAddressFromParseTree(parsetree, false); + ErrorIfCircularDependencyExists(&targetObject); + if (ops && ops->postprocess) { List *processJobs = ops->postprocess(parsetree, queryString); diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c index 5994c5d51..d257cc545 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -378,6 +378,40 @@ RecurseObjectDependencies(ObjectAddress target, expandFn expand, followFn follow } +/* + * ErrorIfCircularDependencyExists checks whether given object has circular dependency + * with itself via existing objects of pg_dist_object. + */ +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))); + } + } +} + + /* * DependencyDefinitionFromPgDepend loads all pg_depend records describing the * dependencies of target. diff --git a/src/backend/distributed/metadata/distobject.c b/src/backend/distributed/metadata/distobject.c index 41b3b372d..b4684d5de 100644 --- a/src/backend/distributed/metadata/distobject.c +++ b/src/backend/distributed/metadata/distobject.c @@ -30,6 +30,7 @@ #include "commands/extension.h" #include "distributed/colocation_utils.h" #include "distributed/commands/utility_hook.h" +#include "distributed/metadata/dependency.h" #include "distributed/metadata/distobject.h" #include "distributed/metadata/pg_dist_object.h" #include "distributed/metadata_cache.h" @@ -198,6 +199,16 @@ MarkObjectDistributedViaSuperUser(const ObjectAddress *distAddress) static void MarkObjectDistributedLocally(const ObjectAddress *distAddress) { + /* + * Having circular dependency between distributed objects prevents Citus from + * adding a new node. So, error out if circular dependency exists for the given + * target object. + * + * Similar check also added to postprocess phase of ProcessUtilityInternal to + * handle altering already distributed objects. + */ + ErrorIfCircularDependencyExists(distAddress); + int paramCount = 3; Oid paramTypes[3] = { OIDOID, diff --git a/src/include/distributed/metadata/dependency.h b/src/include/distributed/metadata/dependency.h index 92714f6cb..05462b181 100644 --- a/src/include/distributed/metadata/dependency.h +++ b/src/include/distributed/metadata/dependency.h @@ -24,6 +24,7 @@ extern List * GetAllDependenciesForObject(const ObjectAddress *target); extern void EnsureRelationDependenciesCanBeDistributed(ObjectAddress *relationAddress); extern ObjectAddress * GetUndistributableDependency(ObjectAddress *target); extern List * OrderObjectAddressListInDependencyOrder(List *objectAddressList); +extern void ErrorIfCircularDependencyExists(const ObjectAddress *objectAddress); extern bool SupportedDependencyByCitus(const ObjectAddress *address); extern List * GetPgDependTuplesForDependingObjects(Oid targetObjectClassId, Oid targetObjectId);