diff --git a/src/backend/distributed/commands/type.c b/src/backend/distributed/commands/type.c index 86d32979b..b57f8efef 100644 --- a/src/backend/distributed/commands/type.c +++ b/src/backend/distributed/commands/type.c @@ -355,7 +355,12 @@ PlanAlterEnumStmt(AlterEnumStmt *stmt, const char *queryString) QualifyTreeNode((Node *) stmt); alterEnumStmtSql = DeparseTreeNode((Node *) stmt); - /* TODO this is not needed anymore for pg12, alter enum can actually run in a xact */ + /* + * Before pg12 ALTER ENUM ... ADD VALUE could not be within a xact block. Instead of + * creating a DDLTaksList we won't return anything here. During the processing phase + * we directly connect to workers and execute the commands remotely. + */ +#if PG_VERSION_NUM < 120000 if (AlterEnumIsAddValue(stmt)) { /* @@ -364,6 +369,7 @@ PlanAlterEnumStmt(AlterEnumStmt *stmt, const char *queryString) */ return NIL; } +#endif commands = list_make3(DISABLE_DDL_PROPAGATION, (void *) alterEnumStmtSql, @@ -400,7 +406,15 @@ ProcessAlterEnumStmt(AlterEnumStmt *stmt, const char *queryString) return; } - /* TODO this is not needed anymore for pg12, alter enum can actually run in a xact */ + /* + * Before pg12 ALTER ENUM ... ADD VALUE could not be within a xact block. Normally we + * would propagate the statements in a xact block to perform 2pc on changes via ddl. + * Instead we need to connect directly to the workers here and execute the command. + * + * From pg12 and up we use the normal infrastructure and create the ddl jobs during + * planning. + */ +#if PG_VERSION_NUM < 120000 if (AlterEnumIsAddValue(stmt)) { /* @@ -437,6 +451,7 @@ ProcessAlterEnumStmt(AlterEnumStmt *stmt, const char *queryString) "all workers"))); } } +#endif } @@ -832,7 +847,7 @@ EnumValsList(Oid typeOid) enum_rel = heap_open(EnumRelationId, AccessShareLock); enum_scan = systable_beginscan(enum_rel, - EnumTypIdLabelIndexId, + EnumTypIdSortOrderIndexId, true, NULL, 1, &skey); diff --git a/src/test/regress/expected/distributed_types.out b/src/test/regress/expected/distributed_types.out index 94c5e4d2f..571e40645 100644 --- a/src/test/regress/expected/distributed_types.out +++ b/src/test/regress/expected/distributed_types.out @@ -100,6 +100,20 @@ SELECT * FROM t4; -- ALTER TYPE ... ADD VALUE does not work in transactions COMMIT; +-- verify order of enum labels +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype; + string_agg +------------ + yes,no +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype;$$); + run_command_on_workers +------------------------------ + (localhost,57637,t,"yes,no") + (localhost,57638,t,"yes,no") +(2 rows) + -- test some combination of types without ddl propagation, this will prevent the workers -- from having those types created. They are created just-in-time on table distribution SET citus.enable_ddl_propagation TO off; diff --git a/src/test/regress/expected/distributed_types_0.out b/src/test/regress/expected/distributed_types_0.out index 8acfd99c8..d27e5d686 100644 --- a/src/test/regress/expected/distributed_types_0.out +++ b/src/test/regress/expected/distributed_types_0.out @@ -100,6 +100,20 @@ SELECT * FROM t4; -- ALTER TYPE ... ADD VALUE does not work in transactions COMMIT; +-- verify order of enum labels +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype; + string_agg +------------ + yes,no +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype;$$); + run_command_on_workers +------------------------------ + (localhost,57637,t,"yes,no") + (localhost,57638,t,"yes,no") +(2 rows) + -- test some combination of types without ddl propagation, this will prevent the workers -- from having those types created. They are created just-in-time on table distribution SET citus.enable_ddl_propagation TO off; diff --git a/src/test/regress/expected/distributed_types_xact_add_enum_value.out b/src/test/regress/expected/distributed_types_xact_add_enum_value.out new file mode 100644 index 000000000..c5e818c36 --- /dev/null +++ b/src/test/regress/expected/distributed_types_xact_add_enum_value.out @@ -0,0 +1,73 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 11 AS version_above_eleven; + version_above_eleven +---------------------- + t +(1 row) + +SET citus.next_shard_id TO 20040000; +CREATE SCHEMA xact_enum_type; +SET search_path TO xact_enum_type; +SET citus.shard_count TO 4; +-- transaction block with simple type +BEGIN; +CREATE TYPE xact_enum_edit AS ENUM ('yes', 'no'); +CREATE TABLE t1 (a int PRIMARY KEY, b xact_enum_edit); +SELECT create_distributed_table('t1','a'); + create_distributed_table +-------------------------- + +(1 row) + +INSERT INTO t1 VALUES (1, 'yes'); +SELECT * FROM t1; + a | b +---+----- + 1 | yes +(1 row) + +COMMIT; +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +ABORT; +-- maybe should not be on the workers +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; + string_agg +------------ + yes,no +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + run_command_on_workers +------------------------------ + (localhost,57637,t,"yes,no") + (localhost,57638,t,"yes,no") +(2 rows) + +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +COMMIT; +-- maybe should be on the workers (pg12 and above) +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; + string_agg +-------------- + yes,no,maybe +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + run_command_on_workers +------------------------------------ + (localhost,57637,t,"yes,no,maybe") + (localhost,57638,t,"yes,no,maybe") +(2 rows) + +-- clear objects +SET client_min_messages TO error; -- suppress cascading objects dropping +DROP SCHEMA xact_enum_type CASCADE; +SELECT run_command_on_workers($$DROP SCHEMA xact_enum_type CASCADE;$$); + run_command_on_workers +----------------------------------- + (localhost,57637,t,"DROP SCHEMA") + (localhost,57638,t,"DROP SCHEMA") +(2 rows) + diff --git a/src/test/regress/expected/distributed_types_xact_add_enum_value_0.out b/src/test/regress/expected/distributed_types_xact_add_enum_value_0.out new file mode 100644 index 000000000..934dcaf06 --- /dev/null +++ b/src/test/regress/expected/distributed_types_xact_add_enum_value_0.out @@ -0,0 +1,75 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 11 AS version_above_eleven; + version_above_eleven +---------------------- + f +(1 row) + +SET citus.next_shard_id TO 20040000; +CREATE SCHEMA xact_enum_type; +SET search_path TO xact_enum_type; +SET citus.shard_count TO 4; +-- transaction block with simple type +BEGIN; +CREATE TYPE xact_enum_edit AS ENUM ('yes', 'no'); +CREATE TABLE t1 (a int PRIMARY KEY, b xact_enum_edit); +SELECT create_distributed_table('t1','a'); + create_distributed_table +-------------------------- + +(1 row) + +INSERT INTO t1 VALUES (1, 'yes'); +SELECT * FROM t1; + a | b +---+----- + 1 | yes +(1 row) + +COMMIT; +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +ERROR: ALTER TYPE ... ADD cannot run inside a transaction block +ABORT; +-- maybe should not be on the workers +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; + string_agg +------------ + yes,no +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + run_command_on_workers +------------------------------ + (localhost,57637,t,"yes,no") + (localhost,57638,t,"yes,no") +(2 rows) + +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +ERROR: ALTER TYPE ... ADD cannot run inside a transaction block +COMMIT; +-- maybe should be on the workers (pg12 and above) +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; + string_agg +------------ + yes,no +(1 row) + +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + run_command_on_workers +------------------------------ + (localhost,57637,t,"yes,no") + (localhost,57638,t,"yes,no") +(2 rows) + +-- clear objects +SET client_min_messages TO error; -- suppress cascading objects dropping +DROP SCHEMA xact_enum_type CASCADE; +SELECT run_command_on_workers($$DROP SCHEMA xact_enum_type CASCADE;$$); + run_command_on_workers +----------------------------------- + (localhost,57637,t,"DROP SCHEMA") + (localhost,57638,t,"DROP SCHEMA") +(2 rows) + diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index af1bf504e..61ebe7612 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -280,7 +280,7 @@ test: ssl_by_default # --------- # object distribution tests # --------- -test: distributed_types distributed_types_conflict disable_object_propagation +test: distributed_types distributed_types_conflict disable_object_propagation distributed_types_xact_add_enum_value test: distributed_functions test: distributed_procedure diff --git a/src/test/regress/sql/distributed_types.sql b/src/test/regress/sql/distributed_types.sql index 6c56dcbac..2258475d9 100644 --- a/src/test/regress/sql/distributed_types.sql +++ b/src/test/regress/sql/distributed_types.sql @@ -57,6 +57,10 @@ SELECT * FROM t4; -- ALTER TYPE ... ADD VALUE does not work in transactions COMMIT; +-- verify order of enum labels +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype; +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'type_tests.te2'::regtype;$$); + -- test some combination of types without ddl propagation, this will prevent the workers -- from having those types created. They are created just-in-time on table distribution SET citus.enable_ddl_propagation TO off; diff --git a/src/test/regress/sql/distributed_types_xact_add_enum_value.sql b/src/test/regress/sql/distributed_types_xact_add_enum_value.sql new file mode 100644 index 000000000..e9020f140 --- /dev/null +++ b/src/test/regress/sql/distributed_types_xact_add_enum_value.sql @@ -0,0 +1,35 @@ +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int > 11 AS version_above_eleven; +SET citus.next_shard_id TO 20040000; + +CREATE SCHEMA xact_enum_type; +SET search_path TO xact_enum_type; +SET citus.shard_count TO 4; + +-- transaction block with simple type +BEGIN; +CREATE TYPE xact_enum_edit AS ENUM ('yes', 'no'); +CREATE TABLE t1 (a int PRIMARY KEY, b xact_enum_edit); +SELECT create_distributed_table('t1','a'); +INSERT INTO t1 VALUES (1, 'yes'); +SELECT * FROM t1; +COMMIT; + +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +ABORT; +-- maybe should not be on the workers +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + +BEGIN; +ALTER TYPE xact_enum_edit ADD VALUE 'maybe'; +COMMIT; +-- maybe should be on the workers (pg12 and above) +SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype; +SELECT run_command_on_workers($$SELECT string_agg(enumlabel, ',' ORDER BY enumsortorder ASC) FROM pg_enum WHERE enumtypid = 'xact_enum_type.xact_enum_edit'::regtype;$$); + +-- clear objects +SET client_min_messages TO error; -- suppress cascading objects dropping +DROP SCHEMA xact_enum_type CASCADE; +SELECT run_command_on_workers($$DROP SCHEMA xact_enum_type CASCADE;$$);