From 3ddc0896510db59d339e7404094af588fb670edf Mon Sep 17 00:00:00 2001 From: aykut-bozkurt <51649454+aykut-bozkurt@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:34:40 +0300 Subject: [PATCH] stop distributing views with no distributed dependency if GUC DistributeLocalViews is set false. (#6083) --- src/backend/distributed/commands/view.c | 69 ++++++++++++++++ src/backend/distributed/metadata/dependency.c | 5 +- src/backend/distributed/shared_library_init.c | 11 +++ src/include/distributed/commands.h | 2 + .../regress/expected/view_propagation.out | 79 +++++++++++++++++++ src/test/regress/pg_regress_multi.pl | 3 + src/test/regress/sql/view_propagation.sql | 44 +++++++++++ 7 files changed, 212 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/commands/view.c b/src/backend/distributed/commands/view.c index 8fc9c7dc0..8219a2907 100644 --- a/src/backend/distributed/commands/view.c +++ b/src/backend/distributed/commands/view.c @@ -35,11 +35,50 @@ #include "utils/lsyscache.h" #include "utils/syscache.h" +/* + * GUC controls some restrictions for local objects. For example, + * if it is disabled, a local view with no distributed relation dependency + * will be created even if it has circular dependency and will not + * log error or warning. Citus will normally restrict that behaviour for the + * local views. e.g CREATE TABLE local_t (a int); + * CREATE VIEW vv1 as SELECT * FROM local_t; + * CREATE OR REPLACE VIEW vv2 as SELECT * FROM vv1; + * + * When the GUC is disabled, citus also wont distribute the local + * view which has no citus relation dependency to workers. Otherwise, it distributes + * them by default. e.g create view v as select 1; + */ +bool EnforceLocalObjectRestrictions = true; + static List * FilterNameListForDistributedViews(List *viewNamesList, bool missing_ok); static void AppendQualifiedViewNameToCreateViewCommand(StringInfo buf, Oid viewOid); static void AppendViewDefinitionToCreateViewCommand(StringInfo buf, Oid viewOid); static void AppendAliasesToCreateViewCommand(StringInfo createViewCommand, Oid viewOid); static void AppendOptionsToCreateViewCommand(StringInfo createViewCommand, Oid viewOid); +static bool ViewHasDistributedRelationDependency(ObjectAddress *viewObjectAddress); + +/* + * ViewHasDistributedRelationDependency returns true if given view at address has + * any distributed relation dependency. + */ +static bool +ViewHasDistributedRelationDependency(ObjectAddress *viewObjectAddress) +{ + List *dependencies = GetAllDependenciesForObject(viewObjectAddress); + ObjectAddress *dependency = NULL; + + foreach_ptr(dependency, dependencies) + { + if (dependency->classId == RelationRelationId && IsAnyObjectDistributed( + list_make1(dependency))) + { + return true; + } + } + + return false; +} + /* * PreprocessViewStmt is called during the planning phase for CREATE OR REPLACE VIEW @@ -110,6 +149,36 @@ PostprocessViewStmt(Node *node, const char *queryString) return NIL; } + /* + * If it is disabled, a local view with no distributed relation dependency + * will be created even if it has circular dependency and will not + * log error or warning. Citus will normally restrict that behaviour for the + * local views. e.g CREATE TABLE local_t (a int); + * CREATE VIEW vv1 as SELECT * FROM local_t; + * CREATE OR REPLACE VIEW vv2 as SELECT * FROM vv1; + * + * When the GUC is disabled, citus also wont distribute the local + * view which has no citus relation dependency to workers. Otherwise, it distributes + * them by default. e.g create view v as select 1; + * + * We disable local object restrictions during pg vanilla tests to not diverge + * from Postgres in terms of error messages. + */ + if (!EnforceLocalObjectRestrictions) + { + /* we asserted that we have only one address. */ + ObjectAddress *viewAddress = linitial(viewAddresses); + + if (!ViewHasDistributedRelationDependency(viewAddress)) + { + /* + * The local view has no distributed relation dependency, so we can allow + * it to be created even if it has circular dependency. + */ + return NIL; + } + } + EnsureAllObjectDependenciesExistOnAllNodes(viewAddresses); /* We have already asserted that we have exactly 1 address in the addresses. */ diff --git a/src/backend/distributed/metadata/dependency.c b/src/backend/distributed/metadata/dependency.c index a9900bd87..9f14b9564 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -843,7 +843,10 @@ ErrorOrWarnIfObjectHasUnsupportedDependency(const ObjectAddress *objectAddress) } else { - RaiseDeferredError(errMsg, WARNING); + if (EnableUnsupportedFeatureMessages) + { + RaiseDeferredError(errMsg, WARNING); + } } return true; diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index 6a9d1fa51..a426639c4 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -1204,6 +1204,17 @@ RegisterCitusConfigVariables(void) GUC_NO_SHOW_ALL, NULL, NULL, NULL); + DefineCustomBoolVariable( + "citus.enforce_object_restrictions_for_local_objects", + gettext_noop( + "Controls some restrictions for local objects."), + NULL, + &EnforceLocalObjectRestrictions, + true, + PGC_USERSET, + GUC_NO_SHOW_ALL, + NULL, NULL, NULL); + DefineCustomIntVariable( "citus.executor_slow_start_interval", gettext_noop("Time to wait between opening connections to the same worker node"), diff --git a/src/include/distributed/commands.h b/src/include/distributed/commands.h index e1d38ed3c..776365666 100644 --- a/src/include/distributed/commands.h +++ b/src/include/distributed/commands.h @@ -31,6 +31,8 @@ extern bool EnableUnsafeTriggers; extern int MaxMatViewSizeToAutoRecreate; +extern bool EnforceLocalObjectRestrictions; + extern void SwitchToSequentialAndLocalExecutionIfRelationNameTooLong(Oid relationId, char * finalRelationName); diff --git a/src/test/regress/expected/view_propagation.out b/src/test/regress/expected/view_propagation.out index 245c9a155..faa5623a3 100644 --- a/src/test/regress/expected/view_propagation.out +++ b/src/test/regress/expected/view_propagation.out @@ -855,6 +855,85 @@ HINT: Use DROP MATERIALIZED VIEW to remove a materialized view. DROP MATERIALIZED VIEW IF EXISTS axx.temp_view_to_drop; DROP MATERIALIZED VIEW IF EXISTS axx.temp_view_to_drop; NOTICE: materialized view "temp_view_to_drop" does not exist, skipping +-- Test the GUC citus.enforce_object_restrictions_for_local_objects +SET search_path to view_prop_schema; +CREATE TABLE local_t (a int); +-- tests when citus.enforce_object_restrictions_for_local_objects=true +SET citus.enforce_object_restrictions_for_local_objects TO true; +-- views will be created locally with warnings +CREATE VIEW vv1 as SELECT * FROM local_t; +WARNING: "view vv1" has dependency to "table local_t" that is not in Citus' metadata +DETAIL: "view vv1" will be created only locally +HINT: Distribute "table local_t" first to distribute "view vv1" +CREATE OR REPLACE VIEW vv2 as SELECT * FROM vv1; +WARNING: "view vv2" has dependency to "table local_t" that is not in Citus' metadata +DETAIL: "view vv2" will be created only locally +HINT: Distribute "table local_t" first to distribute "view vv2" +-- view will fail due to local circular dependency +CREATE OR REPLACE VIEW vv1 as SELECT * FROM vv2; +ERROR: Citus can not handle circular dependencies between distributed objects +DETAIL: "view vv1" circularly depends itself, resolve circular dependency first +-- local view with no citus relation dependency will be distributed +CREATE VIEW v_dist AS SELECT 1; +-- show that the view is in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid = 'v_dist'::regclass; + count +--------------------------------------------------------------------- + 1 +(1 row) + +-- tests when citus.enforce_object_restrictions_for_local_objects=false +SET citus.enforce_object_restrictions_for_local_objects TO false; +SET citus.enable_unsupported_feature_messages TO false; +-- views will be created locally without errors OR warnings +CREATE VIEW vv3 as SELECT * FROM local_t; +CREATE OR REPLACE VIEW vv4 as SELECT * FROM vv3; +CREATE OR REPLACE VIEW vv3 as SELECT * FROM vv4; +-- show that views are NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv3'::regclass, 'vv4'::regclass); + count +--------------------------------------------------------------------- + 0 +(1 row) + +-- local view with no citus relation dependency will NOT be distributed +CREATE VIEW v_local_only AS SELECT 1; +-- show that the view is NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid = 'v_local_only'::regclass; + count +--------------------------------------------------------------------- + 0 +(1 row) + +-- distribute the local table and check the distribution of dependent views +SELECT create_distributed_table('local_t', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- show that views with no circular dependency are in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv1'::regclass, 'vv2'::regclass); + count +--------------------------------------------------------------------- + 2 +(1 row) + +-- show that views with circular dependency are NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv3'::regclass, 'vv4'::regclass); + count +--------------------------------------------------------------------- + 0 +(1 row) + +-- show that we cannot re-create the circular views ever +CREATE OR REPLACE VIEW vv3 as SELECT * FROM local_t; +CREATE OR REPLACE VIEW vv4 as SELECT * FROM vv3; +CREATE OR REPLACE VIEW vv3 as SELECT * FROM vv4; +ERROR: Citus can not handle circular dependencies between distributed objects +DETAIL: "view vv3" circularly depends itself, resolve circular dependency first +RESET citus.enable_unsupported_feature_messages; +RESET citus.enforce_object_restrictions_for_local_objects; SET client_min_messages TO ERROR; DROP SCHEMA view_prop_schema_inner CASCADE; DROP SCHEMA view_prop_schema, axx CASCADE; diff --git a/src/test/regress/pg_regress_multi.pl b/src/test/regress/pg_regress_multi.pl index 4799ad3c8..23b5d8033 100755 --- a/src/test/regress/pg_regress_multi.pl +++ b/src/test/regress/pg_regress_multi.pl @@ -503,6 +503,9 @@ if(!$vanillaDev && $vanillatest) # we disable citus related unwanted messages to not break postgres vanilla test behaviour. push(@pgOptions, "citus.enable_unsupported_feature_messages=false"); + + # we disable some restrictions for local objects like local views to not break postgres vanilla test behaviour. + push(@pgOptions, "citus.enforce_object_restrictions_for_local_objects=false"); } if ($useMitmproxy) diff --git a/src/test/regress/sql/view_propagation.sql b/src/test/regress/sql/view_propagation.sql index d43e33f6f..97e314375 100644 --- a/src/test/regress/sql/view_propagation.sql +++ b/src/test/regress/sql/view_propagation.sql @@ -497,6 +497,50 @@ DROP VIEW IF EXISTS axx.temp_view_to_drop; DROP MATERIALIZED VIEW IF EXISTS axx.temp_view_to_drop; DROP MATERIALIZED VIEW IF EXISTS axx.temp_view_to_drop; +-- Test the GUC citus.enforce_object_restrictions_for_local_objects +SET search_path to view_prop_schema; +CREATE TABLE local_t (a int); + +-- tests when citus.enforce_object_restrictions_for_local_objects=true +SET citus.enforce_object_restrictions_for_local_objects TO true; +-- views will be created locally with warnings +CREATE VIEW vv1 as SELECT * FROM local_t; +CREATE OR REPLACE VIEW vv2 as SELECT * FROM vv1; +-- view will fail due to local circular dependency +CREATE OR REPLACE VIEW vv1 as SELECT * FROM vv2; +-- local view with no citus relation dependency will be distributed +CREATE VIEW v_dist AS SELECT 1; +-- show that the view is in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid = 'v_dist'::regclass; + +-- tests when citus.enforce_object_restrictions_for_local_objects=false +SET citus.enforce_object_restrictions_for_local_objects TO false; +SET citus.enable_unsupported_feature_messages TO false; +-- views will be created locally without errors OR warnings +CREATE VIEW vv3 as SELECT * FROM local_t; +CREATE OR REPLACE VIEW vv4 as SELECT * FROM vv3; +CREATE OR REPLACE VIEW vv3 as SELECT * FROM vv4; +-- show that views are NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv3'::regclass, 'vv4'::regclass); +-- local view with no citus relation dependency will NOT be distributed +CREATE VIEW v_local_only AS SELECT 1; +-- show that the view is NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid = 'v_local_only'::regclass; + +-- distribute the local table and check the distribution of dependent views +SELECT create_distributed_table('local_t', 'a'); +-- show that views with no circular dependency are in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv1'::regclass, 'vv2'::regclass); +-- show that views with circular dependency are NOT in pg_dist_object +SELECT COUNT(*) FROM pg_dist_object WHERE objid IN ('vv3'::regclass, 'vv4'::regclass); + +-- show that we cannot re-create the circular views ever +CREATE OR REPLACE VIEW vv3 as SELECT * FROM local_t; +CREATE OR REPLACE VIEW vv4 as SELECT * FROM vv3; +CREATE OR REPLACE VIEW vv3 as SELECT * FROM vv4; + +RESET citus.enable_unsupported_feature_messages; +RESET citus.enforce_object_restrictions_for_local_objects; SET client_min_messages TO ERROR; DROP SCHEMA view_prop_schema_inner CASCADE; DROP SCHEMA view_prop_schema, axx CASCADE;