From cb6d67a9a97d7b0ed1a3fba7e316c275b2a08f84 Mon Sep 17 00:00:00 2001 From: Burak Velioglu Date: Thu, 3 Mar 2022 19:13:15 +0300 Subject: [PATCH] Make sure that all dependencies of citus tables can be distributed --- .../citus_add_local_table_to_metadata.c | 2 + .../commands/create_distributed_table.c | 2 + src/backend/distributed/commands/function.c | 45 ------ src/backend/distributed/commands/table.c | 1 + src/backend/distributed/metadata/dependency.c | 136 ++++++++++++++++++ src/include/distributed/metadata/dependency.h | 2 + .../alter_table_set_access_method.out | 26 ++-- .../regress/expected/citus_local_tables.out | 4 + .../expected/coordinator_evaluation.out | 6 + .../regress/expected/distributed_types.out | 6 + .../regress/expected/function_propagation.out | 58 +++++++- src/test/regress/expected/multi_extension.out | 4 - .../regress/expected/multi_prepare_sql.out | 6 + src/test/regress/expected/pg13.out | 7 +- .../expected/prepared_statements_4.out | 19 +-- .../prepared_statements_create_load.out | 7 + src/test/regress/expected/single_node.out | 4 + src/test/regress/expected/tableam.out | 30 ++-- .../sql/alter_table_set_access_method.sql | 8 +- src/test/regress/sql/citus_local_tables.sql | 4 + .../regress/sql/coordinator_evaluation.sql | 7 + src/test/regress/sql/distributed_types.sql | 7 + src/test/regress/sql/function_propagation.sql | 45 ++++++ src/test/regress/sql/multi_prepare_sql.sql | 8 ++ .../regress/sql/prepared_statements_4.sql | 20 +-- .../sql/prepared_statements_create_load.sql | 10 +- src/test/regress/sql/single_node.sql | 4 + src/test/regress/sql/tableam.sql | 17 +-- 28 files changed, 368 insertions(+), 127 deletions(-) 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 f8c2a3042..6752a3ee3 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 @@ -31,6 +31,7 @@ #include "distributed/commands/sequence.h" #include "distributed/commands/utility_hook.h" #include "distributed/metadata/distobject.h" +#include "distributed/metadata/dependency.h" #include "distributed/foreign_key_relationship.h" #include "distributed/listutils.h" #include "distributed/local_executor.h" @@ -317,6 +318,7 @@ 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 b96882649..64734fff8 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -443,6 +443,8 @@ 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/function.c b/src/backend/distributed/commands/function.c index 46eb6a2a9..f990a225a 100644 --- a/src/backend/distributed/commands/function.c +++ b/src/backend/distributed/commands/function.c @@ -1347,51 +1347,6 @@ PostprocessCreateFunctionStmt(Node *node, const char *queryString) } -/* - * GetUndistributableDependency checks whether object has any non-distributable - * dependency. If any one found, it will be returned. - */ -ObjectAddress * -GetUndistributableDependency(ObjectAddress *objectAddress) -{ - List *dependencies = GetAllDependenciesForObject(objectAddress); - ObjectAddress *dependency = NULL; - foreach_ptr(dependency, dependencies) - { - if (IsObjectDistributed(dependency)) - { - continue; - } - - if (!SupportedDependencyByCitus(dependency)) - { - /* - * Since roles should be handled manually with Citus community, skip them. - */ - if (getObjectClass(dependency) != OCLASS_ROLE) - { - return dependency; - } - } - - if (getObjectClass(dependency) == OCLASS_CLASS) - { - /* - * Citus can only distribute dependent non-distributed sequence - * and composite types. - */ - char relKind = get_rel_relkind(dependency->objectId); - if (relKind != RELKIND_SEQUENCE && relKind != RELKIND_COMPOSITE_TYPE) - { - return dependency; - } - } - } - - return NULL; -} - - /* * CreateFunctionStmtObjectAddress returns the ObjectAddress for the subject of the * CREATE [OR REPLACE] FUNCTION statement. If missing_ok is false it will error with the diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index 4bf1ff373..fe1e21daf 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -1955,6 +1955,7 @@ 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 4b3595575..5994c5d51 100644 --- a/src/backend/distributed/metadata/dependency.c +++ b/src/backend/distributed/metadata/dependency.c @@ -741,6 +741,142 @@ SupportedDependencyByCitus(const ObjectAddress *address) } +/* + * EnsureRelationDependenciesCanBeDistributed ensures all dependencies of the relation + * can be distributed. + */ +void +EnsureRelationDependenciesCanBeDistributed(ObjectAddress *relationAddress) +{ + ObjectAddress *undistributableDependency = + GetUndistributableDependency(relationAddress); + + 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))); + } +} + + +/* + * GetUndistributableDependency checks whether object has any non-distributable + * dependency. If any one found, it will be returned. + */ +ObjectAddress * +GetUndistributableDependency(ObjectAddress *objectAddress) +{ + List *dependencies = GetAllDependenciesForObject(objectAddress); + ObjectAddress *dependency = NULL; + + /* + * Users can disable metadata sync by their own risk. If it is disabled, Citus + * doesn't propagate dependencies. So, if it is disabled, there is no undistributable + * dependency. + */ + if (!EnableMetadataSync) + { + return NULL; + } + + foreach_ptr(dependency, dependencies) + { + /* + * Objects with the id smaller than FirstNormalObjectId should be created within + * initdb. Citus needs to have such objects as distributed, so we can not add + * such check to dependency resolution logic. Though, Citus shouldn't error + * out if such dependency is not supported. So, skip them here. + */ + if (dependency->objectId < FirstNormalObjectId) + { + continue; + } + + /* + * If object is distributed already, ignore it. + */ + if (IsObjectDistributed(dependency)) + { + continue; + } + + /* + * If the dependency is not supported with Citus, return the dependency. + */ + if (!SupportedDependencyByCitus(dependency)) + { + /* + * Since roles should be handled manually with Citus community, skip them. + */ + if (getObjectClass(dependency) != OCLASS_ROLE) + { + return dependency; + } + } + + if (getObjectClass(dependency) == OCLASS_CLASS) + { + char relKind = get_rel_relkind(dependency->objectId); + + if (relKind == RELKIND_SEQUENCE || relKind == RELKIND_COMPOSITE_TYPE) + { + /* citus knows how to auto-distribute these dependencies */ + continue; + } + else if (relKind == RELKIND_INDEX || relKind == RELKIND_PARTITIONED_INDEX) + { + /* + * Indexes are only qualified for distributed objects for dependency + * tracking purposes, so we can ignore those. + */ + continue; + } + else + { + /* + * Citus doesn't know how to auto-distribute the rest of the RELKINDs + * via dependency resolution + */ + return dependency; + } + } + } + + return NULL; +} + + /* * IsTableOwnedByExtension returns whether the table with the given relation ID is * owned by an extension. diff --git a/src/include/distributed/metadata/dependency.h b/src/include/distributed/metadata/dependency.h index 141b2a628..92714f6cb 100644 --- a/src/include/distributed/metadata/dependency.h +++ b/src/include/distributed/metadata/dependency.h @@ -21,6 +21,8 @@ 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 List * OrderObjectAddressListInDependencyOrder(List *objectAddressList); extern bool SupportedDependencyByCitus(const ObjectAddress *address); extern List * GetPgDependTuplesForDependingObjects(Oid targetObjectClassId, diff --git a/src/test/regress/expected/alter_table_set_access_method.out b/src/test/regress/expected/alter_table_set_access_method.out index 209999667..75ddac37e 100644 --- a/src/test/regress/expected/alter_table_set_access_method.out +++ b/src/test/regress/expected/alter_table_set_access_method.out @@ -474,11 +474,8 @@ SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO ' table_type_ref | heap (4 rows) -SELECT alter_table_set_access_method('table_type_dist', 'fake_am'); +SELECT alter_table_set_access_method('table_type_dist', 'columnar'); NOTICE: creating a new table for alter_table_set_access_method.table_type_dist -WARNING: fake_scan_getnextslot -CONTEXT: SQL statement "SELECT TRUE FROM alter_table_set_access_method.table_type_dist_1533505599 LIMIT 1" -WARNING: fake_scan_getnextslot NOTICE: moving the data of alter_table_set_access_method.table_type_dist NOTICE: dropping the old alter_table_set_access_method.table_type_dist NOTICE: renaming the new table to alter_table_set_access_method.table_type_dist @@ -487,11 +484,8 @@ NOTICE: renaming the new table to alter_table_set_access_method.table_type_dist (1 row) -SELECT alter_table_set_access_method('table_type_ref', 'fake_am'); +SELECT alter_table_set_access_method('table_type_ref', 'columnar'); NOTICE: creating a new table for alter_table_set_access_method.table_type_ref -WARNING: fake_scan_getnextslot -CONTEXT: SQL statement "SELECT TRUE FROM alter_table_set_access_method.table_type_ref_1037855087 LIMIT 1" -WARNING: fake_scan_getnextslot NOTICE: moving the data of alter_table_set_access_method.table_type_ref NOTICE: dropping the old alter_table_set_access_method.table_type_ref NOTICE: renaming the new table to alter_table_set_access_method.table_type_ref @@ -500,7 +494,7 @@ NOTICE: renaming the new table to alter_table_set_access_method.table_type_ref (1 row) -SELECT alter_table_set_access_method('table_type_pg_local', 'fake_am'); +SELECT alter_table_set_access_method('table_type_pg_local', 'columnar'); NOTICE: creating a new table for alter_table_set_access_method.table_type_pg_local NOTICE: moving the data of alter_table_set_access_method.table_type_pg_local NOTICE: dropping the old alter_table_set_access_method.table_type_pg_local @@ -510,7 +504,7 @@ NOTICE: renaming the new table to alter_table_set_access_method.table_type_pg_l (1 row) -SELECT alter_table_set_access_method('table_type_citus_local', 'fake_am'); +SELECT alter_table_set_access_method('table_type_citus_local', 'columnar'); NOTICE: creating a new table for alter_table_set_access_method.table_type_citus_local NOTICE: moving the data of alter_table_set_access_method.table_type_citus_local NOTICE: dropping the old alter_table_set_access_method.table_type_citus_local @@ -523,17 +517,17 @@ NOTICE: renaming the new table to alter_table_set_access_method.table_type_citu SELECT table_name, citus_table_type, distribution_column, shard_count, access_method FROM public.citus_tables WHERE table_name::text LIKE 'table\_type%' ORDER BY 1; table_name | citus_table_type | distribution_column | shard_count | access_method --------------------------------------------------------------------- - table_type_dist | distributed | a | 4 | fake_am - table_type_ref | reference | | 1 | fake_am + table_type_dist | distributed | a | 4 | columnar + table_type_ref | reference | | 1 | columnar (2 rows) SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO 'table_type\D*' AND c.relnamespace = 'alter_table_set_access_method'::regnamespace AND c.relam = a.oid; relname | amname --------------------------------------------------------------------- - table_type_citus_local | fake_am - table_type_dist | fake_am - table_type_pg_local | fake_am - table_type_ref | fake_am + table_type_citus_local | columnar + table_type_dist | columnar + table_type_pg_local | columnar + table_type_ref | columnar (4 rows) -- test when the parent of a partition has foreign key to a reference table diff --git a/src/test/regress/expected/citus_local_tables.out b/src/test/regress/expected/citus_local_tables.out index 4e58021dd..9dac373cd 100644 --- a/src/test/regress/expected/citus_local_tables.out +++ b/src/test/regress/expected/citus_local_tables.out @@ -224,12 +224,16 @@ CREATE FOREIGN TABLE foreign_table ( ) SERVER fake_fdw_server OPTIONS (encoding 'utf-8', compression 'true'); -- observe that we do not create fdw server for shell table, both shard relation -- & shell relation points to the same same server object +-- Disable metadata sync since citus doesn't support distributing +-- foreign data wrappers for now. +SET citus.enable_metadata_sync TO OFF; SELECT citus_add_local_table_to_metadata('foreign_table'); citus_add_local_table_to_metadata --------------------------------------------------------------------- (1 row) +RESET citus.enable_metadata_sync; DROP FOREIGN TABLE foreign_table; NOTICE: executing the command locally: DROP FOREIGN TABLE IF EXISTS citus_local_tables_test_schema.foreign_table_xxxxx CASCADE -- drop them for next tests diff --git a/src/test/regress/expected/coordinator_evaluation.out b/src/test/regress/expected/coordinator_evaluation.out index f675c316f..47696a462 100644 --- a/src/test/regress/expected/coordinator_evaluation.out +++ b/src/test/regress/expected/coordinator_evaluation.out @@ -586,6 +586,8 @@ SELECT count(*) FROM coordinator_evaluation_table_2 WHERE key = 101; CREATE TYPE comptype_int as (int_a int); CREATE DOMAIN domain_comptype_int AS comptype_int CHECK ((VALUE).int_a > 0); -- citus does not propagate domain types +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers( $$ CREATE DOMAIN coordinator_evaluation.domain_comptype_int AS coordinator_evaluation.comptype_int CHECK ((VALUE).int_a > 0) @@ -597,12 +599,16 @@ $$); (2 rows) CREATE TABLE reference_table(column_a coordinator_evaluation.domain_comptype_int); +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_reference_table('reference_table'); create_reference_table --------------------------------------------------------------------- (1 row) +RESET citus.enable_metadata_sync; INSERT INTO reference_table (column_a) VALUES ('(1)'); INSERT INTO reference_table (column_a) VALUES ('(2)'), ('(3)'); INSERT INTO reference_table VALUES ('(4)'), ('(5)'); diff --git a/src/test/regress/expected/distributed_types.out b/src/test/regress/expected/distributed_types.out index 2e2ef9c1f..c20326820 100644 --- a/src/test/regress/expected/distributed_types.out +++ b/src/test/regress/expected/distributed_types.out @@ -415,6 +415,8 @@ HINT: Use the column name to insert or update the composite type as a single va CREATE TYPE two_ints as (if1 int, if2 int); CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); -- citus does not propagate domain objects +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers( $$ CREATE DOMAIN type_tests.domain AS type_tests.two_ints CHECK ((VALUE).if1 > 0); @@ -426,12 +428,16 @@ $$); (2 rows) CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array domain[]); +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('domain_indirection_test', 'f1'); create_distributed_table --------------------------------------------------------------------- (1 row) +RESET citus.enable_metadata_sync; -- not supported (field indirection to underlying composite type) INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); ERROR: inserting or modifying composite type fields is not supported diff --git a/src/test/regress/expected/function_propagation.out b/src/test/regress/expected/function_propagation.out index ac645c263..7aefbb697 100644 --- a/src/test/regress/expected/function_propagation.out +++ b/src/test/regress/expected/function_propagation.out @@ -435,8 +435,8 @@ $$; 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: type function_propagation_schema.non_dist_table does not exist -CONTEXT: while executing command on localhost:xxxxx +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. COMMIT; -- Adding a column with default value should propagate the function BEGIN; @@ -478,6 +478,28 @@ SELECT * FROM run_command_on_workers($$SELECT pg_identify_object_as_address(clas localhost | 57638 | t | (function,"{function_propagation_schema,func_in_transaction_4}",{}) (2 rows) +-- Adding a column with default function depending on non-distributable table should fail +BEGIN; + CREATE TABLE non_dist_table_for_function(id int); + CREATE OR REPLACE FUNCTION non_dist_func(col_1 non_dist_table_for_function) + RETURNS int + LANGUAGE plpgsql AS + $$ + BEGIN + return 1; + END; + $$; + CREATE TABLE table_to_dist(id int); + SELECT create_distributed_table('table_to_dist', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(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. +ROLLBACK; -- Adding multiple columns with default values should propagate the function BEGIN; CREATE OR REPLACE FUNCTION func_in_transaction_5() @@ -701,8 +723,8 @@ $$; 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: type function_propagation_schema.local_table_for_const does not exist -CONTEXT: while executing command on localhost:xxxxx +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. COMMIT; -- Show that function as a part of generated always is supporte BEGIN; @@ -1020,6 +1042,34 @@ SELECT * FROM run_command_on_workers($$SELECT pg_identify_object_as_address(clas localhost | 57638 | t | (function,"{function_propagation_schema,func_in_transaction_def_with_seq}",{bigint}) (2 rows) +-- Show that having a dependency on another dist table work out tx +CREATE TABLE loc_for_func_dist ( + product_no integer, + name text, + price numeric CONSTRAINT positive_price CHECK (price > 0)); +SELECT create_distributed_table('loc_for_func_dist', 'product_no'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CREATE OR REPLACE FUNCTION non_sense_func_for_default_val(loc_for_func_dist) +RETURNS int +LANGUAGE plpgsql IMMUTABLE AS +$$ +BEGIN +return 1; +END; +$$; +CREATE TABLE table_non_for_func_dist ( + a int, + b int DEFAULT non_sense_func_for_default_val(NULL::loc_for_func_dist)); +SELECT create_distributed_table('table_non_for_func_dist', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + RESET search_path; SET client_min_messages TO WARNING; DROP SCHEMA function_propagation_schema CASCADE; diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index f378d2635..aa9960bbe 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -77,8 +77,6 @@ END $func$ LANGUAGE plpgsql; CREATE SCHEMA test; :create_function_test_maintenance_worker -WARNING: Citus can't distribute functions having dependency on unsupported object of type "view" -DETAIL: Function will be created only locally -- check maintenance daemon is started SELECT datname, current_database(), usename, (SELECT extowner::regrole::text FROM pg_extension WHERE extname = 'citus') @@ -1222,8 +1220,6 @@ HINT: You can manually create a database and its extensions on workers. CREATE EXTENSION citus; CREATE SCHEMA test; :create_function_test_maintenance_worker -WARNING: Citus can't distribute functions having dependency on unsupported object of type "view" -DETAIL: Function will be created only locally -- see that the daemon started SELECT datname, current_database(), usename, (SELECT extowner::regrole::text FROM pg_extension WHERE extname = 'citus') diff --git a/src/test/regress/expected/multi_prepare_sql.out b/src/test/regress/expected/multi_prepare_sql.out index 6681f4112..4ff7bba42 100644 --- a/src/test/regress/expected/multi_prepare_sql.out +++ b/src/test/regress/expected/multi_prepare_sql.out @@ -1049,6 +1049,8 @@ SELECT key, value FROM text_partition_column_table ORDER BY key; DROP TABLE text_partition_column_table; -- Domain type columns can give issues CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers($$ CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$') $$); @@ -1062,12 +1064,16 @@ CREATE TABLE domain_partition_column_table ( key test_key NOT NULL, value int ); +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('domain_partition_column_table', 'key'); create_distributed_table --------------------------------------------------------------------- (1 row) +RESET citus.enable_metadata_sync; PREPARE prepared_coercion_to_domain_insert(text) AS INSERT INTO domain_partition_column_table VALUES ($1, 1); EXECUTE prepared_coercion_to_domain_insert('test-1'); diff --git a/src/test/regress/expected/pg13.out b/src/test/regress/expected/pg13.out index 2ba782aaf..44664fb83 100644 --- a/src/test/regress/expected/pg13.out +++ b/src/test/regress/expected/pg13.out @@ -164,8 +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: type "test_pg13.myvarchar" does not exist -CONTEXT: while executing command on localhost:xxxxx +ERROR: Relation "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 @@ -209,7 +208,7 @@ INSERT INTO test_wal VALUES(2,22); Tasks Shown: All -> Task Node: host=localhost port=xxxxx dbname=regression - -> Insert on test_wal_65013 (actual rows=0 loops=1) + -> Insert on test_wal_65011 (actual rows=0 loops=1) WAL: records=1 bytes=63 -> Result (actual rows=1 loops=1) (8 rows) @@ -227,7 +226,7 @@ INSERT INTO test_wal VALUES(3,33),(4,44),(5,55) RETURNING *; -> Task Tuple data received from node: 24 bytes Node: host=localhost port=xxxxx dbname=regression - -> Insert on test_wal_65012 citus_table_alias (actual rows=3 loops=1) + -> Insert on test_wal_65010 citus_table_alias (actual rows=3 loops=1) WAL: records=3 bytes=189 -> Values Scan on "*VALUES*" (actual rows=3 loops=1) (10 rows) diff --git a/src/test/regress/expected/prepared_statements_4.out b/src/test/regress/expected/prepared_statements_4.out index 0dba296e8..2d66e04eb 100644 --- a/src/test/regress/expected/prepared_statements_4.out +++ b/src/test/regress/expected/prepared_statements_4.out @@ -20,15 +20,16 @@ SELECT key, value FROM text_partition_column_table ORDER BY key; test | 1 (7 rows) -PREPARE prepared_coercion_to_domain_insert(text) AS - INSERT INTO domain_partition_column_table VALUES ($1, 1); -EXECUTE prepared_coercion_to_domain_insert('test-1'); -EXECUTE prepared_coercion_to_domain_insert('test-2'); -EXECUTE prepared_coercion_to_domain_insert('test-3'); -EXECUTE prepared_coercion_to_domain_insert('test-4'); -EXECUTE prepared_coercion_to_domain_insert('test-5'); -EXECUTE prepared_coercion_to_domain_insert('test-6'); -EXECUTE prepared_coercion_to_domain_insert('test-7'); +-- TODO: Uncomment tests below once domains are supported +-- PREPARE prepared_coercion_to_domain_insert(text) AS +-- INSERT INTO domain_partition_column_table VALUES ($1, 1); +-- EXECUTE prepared_coercion_to_domain_insert('test-1'); +-- EXECUTE prepared_coercion_to_domain_insert('test-2'); +-- EXECUTE prepared_coercion_to_domain_insert('test-3'); +-- EXECUTE prepared_coercion_to_domain_insert('test-4'); +-- EXECUTE prepared_coercion_to_domain_insert('test-5'); +-- EXECUTE prepared_coercion_to_domain_insert('test-6'); +-- EXECUTE prepared_coercion_to_domain_insert('test-7'); PREPARE FOO AS INSERT INTO http_request ( site_id, ingest_time, url, request_country, ip_address, status_code, response_time_msec diff --git a/src/test/regress/expected/prepared_statements_create_load.out b/src/test/regress/expected/prepared_statements_create_load.out index 91f653d08..68d9baeb6 100644 --- a/src/test/regress/expected/prepared_statements_create_load.out +++ b/src/test/regress/expected/prepared_statements_create_load.out @@ -65,6 +65,9 @@ SELECT create_distributed_table('text_partition_column_table', 'key'); -- Domain type columns can give issues -- and we use offset to prevent output diverging CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. Also uncomment related tests on +-- prepared_statements_4 test file. SELECT run_command_on_workers($$ CREATE DOMAIN "prepared statements".test_key AS text CHECK(VALUE ~ '^test-\d$') $$) OFFSET 10000; @@ -72,6 +75,9 @@ $$) OFFSET 10000; --------------------------------------------------------------------- (0 rows) +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; CREATE TABLE domain_partition_column_table ( key test_key NOT NULL, value int @@ -82,6 +88,7 @@ SELECT create_distributed_table('domain_partition_column_table', 'key'); (1 row) +RESET citus.enable_metadata_sync; -- verify we re-evaluate volatile functions every time CREATE TABLE http_request ( site_id INT, diff --git a/src/test/regress/expected/single_node.out b/src/test/regress/expected/single_node.out index 7e7e84a3a..c854ec48a 100644 --- a/src/test/regress/expected/single_node.out +++ b/src/test/regress/expected/single_node.out @@ -899,6 +899,9 @@ CREATE TABLE hpart0 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remaind CREATE TABLE hpart1 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remainder 1); CREATE TABLE hpart2 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remainder 2); CREATE TABLE hpart3 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remainder 3); +-- Disable metadata sync since citus doesn't support distributing +-- operator class for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('hash_parted ', 'a'); create_distributed_table --------------------------------------------------------------------- @@ -925,6 +928,7 @@ ALTER TABLE hash_parted DETACH PARTITION hpart0; ALTER TABLE hash_parted DETACH PARTITION hpart1; ALTER TABLE hash_parted DETACH PARTITION hpart2; ALTER TABLE hash_parted DETACH PARTITION hpart3; +RESET citus.enable_metadata_sync; -- test range partition without creating partitions and inserting with generate_series() -- should error out even in plain PG since no partition of relation "parent_tab" is found for row -- in Citus it errors out because it fails to evaluate partition key in insert diff --git a/src/test/regress/expected/tableam.out b/src/test/regress/expected/tableam.out index 7a11d7354..f82d0db90 100644 --- a/src/test/regress/expected/tableam.out +++ b/src/test/regress/expected/tableam.out @@ -23,6 +23,11 @@ $Q$); (1 row) +-- Since Citus assumes access methods are part of the extension, make fake_am +-- owned manually to be able to pass checks on Citus while distributing tables. +ALTER EXTENSION citus ADD ACCESS METHOD fake_am; +NOTICE: Citus does not propagate adding/dropping member objects +HINT: You can add/drop the member objects on the workers as well. -- -- Hash distributed table using a non-default table access method -- @@ -129,27 +134,6 @@ SELECT * FROM master_get_table_ddl_events('test_ref'); ALTER TABLE test_tableam.test_ref OWNER TO postgres (2 rows) --- replicate to coordinator -SET client_min_messages TO WARNING; -\set VERBOSIY terse -SELECT 1 FROM master_add_node('localhost', :master_port, groupid => 0); - ?column? ---------------------------------------------------------------------- - 1 -(1 row) - -RESET client_min_messages; -delete from test_ref; -WARNING: fake_scan_getnextslot -DETAIL: from localhost:xxxxx -ERROR: fake_tuple_delete not implemented -CONTEXT: while executing command on localhost:xxxxx -SELECT master_remove_node('localhost', :master_port); - master_remove_node ---------------------------------------------------------------------- - -(1 row) - -- -- Range partitioned table using a non-default table access method -- @@ -323,5 +307,7 @@ CREATE TABLE test_partitioned(id int, p int, val int) PARTITION BY RANGE (p) USING fake_am; ERROR: specifying a table access method is not supported on a partitioned table \set VERBOSITY terse +ALTER EXTENSION citus DROP ACCESS METHOD fake_am; +NOTICE: Citus does not propagate adding/dropping member objects drop schema test_tableam cascade; -NOTICE: drop cascades to 6 other objects +NOTICE: drop cascades to 5 other objects diff --git a/src/test/regress/sql/alter_table_set_access_method.sql b/src/test/regress/sql/alter_table_set_access_method.sql index 3d3ff2599..7ddadc531 100644 --- a/src/test/regress/sql/alter_table_set_access_method.sql +++ b/src/test/regress/sql/alter_table_set_access_method.sql @@ -173,10 +173,10 @@ CREATE TABLE table_type_pg_local (a INT); SELECT table_name, citus_table_type, distribution_column, shard_count, access_method FROM public.citus_tables WHERE table_name::text LIKE 'table\_type%' ORDER BY 1; SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO 'table_type\D*' AND c.relnamespace = 'alter_table_set_access_method'::regnamespace AND c.relam = a.oid; -SELECT alter_table_set_access_method('table_type_dist', 'fake_am'); -SELECT alter_table_set_access_method('table_type_ref', 'fake_am'); -SELECT alter_table_set_access_method('table_type_pg_local', 'fake_am'); -SELECT alter_table_set_access_method('table_type_citus_local', 'fake_am'); +SELECT alter_table_set_access_method('table_type_dist', 'columnar'); +SELECT alter_table_set_access_method('table_type_ref', 'columnar'); +SELECT alter_table_set_access_method('table_type_pg_local', 'columnar'); +SELECT alter_table_set_access_method('table_type_citus_local', 'columnar'); SELECT table_name, citus_table_type, distribution_column, shard_count, access_method FROM public.citus_tables WHERE table_name::text LIKE 'table\_type%' ORDER BY 1; SELECT c.relname, a.amname FROM pg_class c, pg_am a where c.relname SIMILAR TO 'table_type\D*' AND c.relnamespace = 'alter_table_set_access_method'::regnamespace AND c.relam = a.oid; diff --git a/src/test/regress/sql/citus_local_tables.sql b/src/test/regress/sql/citus_local_tables.sql index 3272bdfbd..ae3db0060 100644 --- a/src/test/regress/sql/citus_local_tables.sql +++ b/src/test/regress/sql/citus_local_tables.sql @@ -181,7 +181,11 @@ CREATE FOREIGN TABLE foreign_table ( -- observe that we do not create fdw server for shell table, both shard relation -- & shell relation points to the same same server object +-- Disable metadata sync since citus doesn't support distributing +-- foreign data wrappers for now. +SET citus.enable_metadata_sync TO OFF; SELECT citus_add_local_table_to_metadata('foreign_table'); +RESET citus.enable_metadata_sync; DROP FOREIGN TABLE foreign_table; diff --git a/src/test/regress/sql/coordinator_evaluation.sql b/src/test/regress/sql/coordinator_evaluation.sql index 1f313a76a..047d19c3f 100644 --- a/src/test/regress/sql/coordinator_evaluation.sql +++ b/src/test/regress/sql/coordinator_evaluation.sql @@ -215,13 +215,20 @@ SELECT count(*) FROM coordinator_evaluation_table_2 WHERE key = 101; CREATE TYPE comptype_int as (int_a int); CREATE DOMAIN domain_comptype_int AS comptype_int CHECK ((VALUE).int_a > 0); -- citus does not propagate domain types +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers( $$ CREATE DOMAIN coordinator_evaluation.domain_comptype_int AS coordinator_evaluation.comptype_int CHECK ((VALUE).int_a > 0) $$); CREATE TABLE reference_table(column_a coordinator_evaluation.domain_comptype_int); + +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_reference_table('reference_table'); +RESET citus.enable_metadata_sync; INSERT INTO reference_table (column_a) VALUES ('(1)'); INSERT INTO reference_table (column_a) VALUES ('(2)'), ('(3)'); diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index 23d84d26b..e22533101 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -262,12 +262,19 @@ UPDATE field_indirection_test_2 SET (ct2_col.text_1, ct1_col.int_2) = ('text2', CREATE TYPE two_ints as (if1 int, if2 int); CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); -- citus does not propagate domain objects +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers( $$ CREATE DOMAIN type_tests.domain AS type_tests.two_ints CHECK ((VALUE).if1 > 0); $$); CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array domain[]); + +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('domain_indirection_test', 'f1'); +RESET citus.enable_metadata_sync; -- not supported (field indirection to underlying composite type) INSERT INTO domain_indirection_test (f1,f3.if1, f3.if2) VALUES (0, 1, 2); diff --git a/src/test/regress/sql/function_propagation.sql b/src/test/regress/sql/function_propagation.sql index a9a6d04d8..cd915045f 100644 --- a/src/test/regress/sql/function_propagation.sql +++ b/src/test/regress/sql/function_propagation.sql @@ -299,6 +299,27 @@ COMMIT; SELECT * FROM run_command_on_workers($$SELECT pg_identify_object_as_address(classid, objid, objsubid) from citus.pg_dist_object where objid = 'function_propagation_schema.func_in_transaction_4'::regproc::oid;$$) ORDER BY 1,2; +-- Adding a column with default function depending on non-distributable table should fail +BEGIN; + CREATE TABLE non_dist_table_for_function(id int); + + CREATE OR REPLACE FUNCTION non_dist_func(col_1 non_dist_table_for_function) + RETURNS int + LANGUAGE plpgsql AS + $$ + BEGIN + return 1; + END; + $$; + + CREATE TABLE table_to_dist(id int); + SELECT create_distributed_table('table_to_dist', 'id'); + + ALTER TABLE table_to_dist ADD COLUMN col_1 int default function_propagation_schema.non_dist_func(NULL::non_dist_table_for_function); + +ROLLBACK; + + -- Adding multiple columns with default values should propagate the function BEGIN; CREATE OR REPLACE FUNCTION func_in_transaction_5() @@ -665,6 +686,30 @@ COMMIT; -- Function should be marked as distributed on the worker after committing changes SELECT * FROM run_command_on_workers($$SELECT pg_identify_object_as_address(classid, objid, objsubid) from citus.pg_dist_object where objid = 'function_propagation_schema.func_in_transaction_def_with_seq'::regproc::oid;$$) ORDER BY 1,2; + +-- Show that having a dependency on another dist table work out tx +CREATE TABLE loc_for_func_dist ( + product_no integer, + name text, + price numeric CONSTRAINT positive_price CHECK (price > 0)); + +SELECT create_distributed_table('loc_for_func_dist', 'product_no'); + +CREATE OR REPLACE FUNCTION non_sense_func_for_default_val(loc_for_func_dist) +RETURNS int +LANGUAGE plpgsql IMMUTABLE AS +$$ +BEGIN +return 1; +END; +$$; + +CREATE TABLE table_non_for_func_dist ( + a int, + b int DEFAULT non_sense_func_for_default_val(NULL::loc_for_func_dist)); + +SELECT create_distributed_table('table_non_for_func_dist', 'a'); + RESET search_path; SET client_min_messages TO WARNING; DROP SCHEMA function_propagation_schema CASCADE; diff --git a/src/test/regress/sql/multi_prepare_sql.sql b/src/test/regress/sql/multi_prepare_sql.sql index 54893d025..37a5f0690 100644 --- a/src/test/regress/sql/multi_prepare_sql.sql +++ b/src/test/regress/sql/multi_prepare_sql.sql @@ -546,6 +546,9 @@ DROP TABLE text_partition_column_table; -- Domain type columns can give issues CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); + +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. SELECT run_command_on_workers($$ CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$') $$); @@ -554,7 +557,12 @@ CREATE TABLE domain_partition_column_table ( key test_key NOT NULL, value int ); + +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('domain_partition_column_table', 'key'); +RESET citus.enable_metadata_sync; PREPARE prepared_coercion_to_domain_insert(text) AS INSERT INTO domain_partition_column_table VALUES ($1, 1); diff --git a/src/test/regress/sql/prepared_statements_4.sql b/src/test/regress/sql/prepared_statements_4.sql index e921a83cf..4aa79387c 100644 --- a/src/test/regress/sql/prepared_statements_4.sql +++ b/src/test/regress/sql/prepared_statements_4.sql @@ -16,17 +16,17 @@ SELECT key, value FROM text_partition_column_table ORDER BY key; +-- TODO: Uncomment tests below once domains are supported +-- PREPARE prepared_coercion_to_domain_insert(text) AS +-- INSERT INTO domain_partition_column_table VALUES ($1, 1); -PREPARE prepared_coercion_to_domain_insert(text) AS - INSERT INTO domain_partition_column_table VALUES ($1, 1); - -EXECUTE prepared_coercion_to_domain_insert('test-1'); -EXECUTE prepared_coercion_to_domain_insert('test-2'); -EXECUTE prepared_coercion_to_domain_insert('test-3'); -EXECUTE prepared_coercion_to_domain_insert('test-4'); -EXECUTE prepared_coercion_to_domain_insert('test-5'); -EXECUTE prepared_coercion_to_domain_insert('test-6'); -EXECUTE prepared_coercion_to_domain_insert('test-7'); +-- EXECUTE prepared_coercion_to_domain_insert('test-1'); +-- EXECUTE prepared_coercion_to_domain_insert('test-2'); +-- EXECUTE prepared_coercion_to_domain_insert('test-3'); +-- EXECUTE prepared_coercion_to_domain_insert('test-4'); +-- EXECUTE prepared_coercion_to_domain_insert('test-5'); +-- EXECUTE prepared_coercion_to_domain_insert('test-6'); +-- EXECUTE prepared_coercion_to_domain_insert('test-7'); diff --git a/src/test/regress/sql/prepared_statements_create_load.sql b/src/test/regress/sql/prepared_statements_create_load.sql index b2e5684c0..af7baa026 100644 --- a/src/test/regress/sql/prepared_statements_create_load.sql +++ b/src/test/regress/sql/prepared_statements_create_load.sql @@ -54,16 +54,24 @@ SELECT create_distributed_table('text_partition_column_table', 'key'); -- and we use offset to prevent output diverging CREATE DOMAIN test_key AS text CHECK(VALUE ~ '^test-\d$'); + +-- TODO: Once domains are supported, remove enable_metadata_sync off/on change +-- on dependent table distribution below. Also uncomment related tests on +-- prepared_statements_4 test file. SELECT run_command_on_workers($$ CREATE DOMAIN "prepared statements".test_key AS text CHECK(VALUE ~ '^test-\d$') $$) OFFSET 10000; +-- Disable metadata sync since citus doesn't support distributing +-- domains for now. +SET citus.enable_metadata_sync TO OFF; CREATE TABLE domain_partition_column_table ( key test_key NOT NULL, value int ); -SELECT create_distributed_table('domain_partition_column_table', 'key'); +SELECT create_distributed_table('domain_partition_column_table', 'key'); +RESET citus.enable_metadata_sync; -- verify we re-evaluate volatile functions every time CREATE TABLE http_request ( diff --git a/src/test/regress/sql/single_node.sql b/src/test/regress/sql/single_node.sql index fbd57c0b6..74c857d4e 100644 --- a/src/test/regress/sql/single_node.sql +++ b/src/test/regress/sql/single_node.sql @@ -538,6 +538,9 @@ CREATE TABLE hpart1 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remaind CREATE TABLE hpart2 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remainder 2); CREATE TABLE hpart3 PARTITION OF hash_parted FOR VALUES WITH (modulus 4, remainder 3); +-- Disable metadata sync since citus doesn't support distributing +-- operator class for now. +SET citus.enable_metadata_sync TO OFF; SELECT create_distributed_table('hash_parted ', 'a'); INSERT INTO hash_parted VALUES (1, generate_series(1, 10)); @@ -548,6 +551,7 @@ ALTER TABLE hash_parted DETACH PARTITION hpart0; ALTER TABLE hash_parted DETACH PARTITION hpart1; ALTER TABLE hash_parted DETACH PARTITION hpart2; ALTER TABLE hash_parted DETACH PARTITION hpart3; +RESET citus.enable_metadata_sync; -- test range partition without creating partitions and inserting with generate_series() -- should error out even in plain PG since no partition of relation "parent_tab" is found for row diff --git a/src/test/regress/sql/tableam.sql b/src/test/regress/sql/tableam.sql index ab2b640a6..3c7cb69f0 100644 --- a/src/test/regress/sql/tableam.sql +++ b/src/test/regress/sql/tableam.sql @@ -22,12 +22,17 @@ SELECT public.run_command_on_coordinator_and_workers($Q$ CREATE ACCESS METHOD fake_am TYPE TABLE HANDLER fake_am_handler; $Q$); +-- Since Citus assumes access methods are part of the extension, make fake_am +-- owned manually to be able to pass checks on Citus while distributing tables. +ALTER EXTENSION citus ADD ACCESS METHOD fake_am; + -- -- Hash distributed table using a non-default table access method -- create table test_hash_dist(id int, val int) using fake_am; insert into test_hash_dist values (1, 1); + select create_distributed_table('test_hash_dist','id'); select * from test_hash_dist; @@ -48,6 +53,7 @@ SELECT * FROM master_get_table_ddl_events('test_hash_dist'); create table test_ref(a int) using fake_am; insert into test_ref values (1); + select create_reference_table('test_ref'); select * from test_ref; @@ -62,20 +68,14 @@ RESET client_min_messages; -- ddl events should include "USING fake_am" SELECT * FROM master_get_table_ddl_events('test_ref'); --- replicate to coordinator -SET client_min_messages TO WARNING; -\set VERBOSIY terse -SELECT 1 FROM master_add_node('localhost', :master_port, groupid => 0); -RESET client_min_messages; -delete from test_ref; -SELECT master_remove_node('localhost', :master_port); - -- -- Range partitioned table using a non-default table access method -- CREATE TABLE test_range_dist(id int, val int) using fake_am; + SELECT create_distributed_table('test_range_dist', 'id', 'range'); + CALL public.create_range_partitioned_shards('test_range_dist', '{"0","25"}','{"24","49"}'); select * from test_range_dist; @@ -148,4 +148,5 @@ CREATE TABLE test_partitioned(id int, p int, val int) PARTITION BY RANGE (p) USING fake_am; \set VERBOSITY terse +ALTER EXTENSION citus DROP ACCESS METHOD fake_am; drop schema test_tableam cascade;