From e26b29d3bb03d3f971eca3952bd575eed2363aa0 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Wed, 16 Jun 2021 13:58:49 +0300 Subject: [PATCH] Fix nextval('seq_name'::text) bug, and schema for seq tests (#5046) --- .../distributed/deparser/citus_ruleutils.c | 14 ++- .../expected/multi_sequence_default.out | 119 ++++++++++++++---- .../regress/sql/multi_sequence_default.sql | 53 ++++++-- 3 files changed, 151 insertions(+), 35 deletions(-) diff --git a/src/backend/distributed/deparser/citus_ruleutils.c b/src/backend/distributed/deparser/citus_ruleutils.c index da5246659..28d83918c 100644 --- a/src/backend/distributed/deparser/citus_ruleutils.c +++ b/src/backend/distributed/deparser/citus_ruleutils.c @@ -515,9 +515,21 @@ EnsureSequenceTypeSupported(Oid relationId, AttrNumber attnum, Oid seqTypId) /* retrieve the sequence id of the sequence found in nextval('seq') */ List *sequencesFromAttrDef = GetSequencesFromAttrDef(attrdefOid); - /* to simplify and eliminate cases like "DEFAULT nextval('..') - nextval('..')" */ + if (list_length(sequencesFromAttrDef) == 0) + { + /* + * We need this check because sometimes there are cases where the + * dependency between the table and the sequence is not formed + * One example is when the default is defined by + * DEFAULT nextval('seq_name'::text) (not by DEFAULT nextval('seq_name')) + * In these cases, sequencesFromAttrDef with be empty. + */ + return; + } + if (list_length(sequencesFromAttrDef) > 1) { + /* to simplify and eliminate cases like "DEFAULT nextval('..') - nextval('..')" */ ereport(ERROR, (errmsg( "More than one sequence in a column default" " is not supported for distribution"))); diff --git a/src/test/regress/expected/multi_sequence_default.out b/src/test/regress/expected/multi_sequence_default.out index 8201d3aaa..378f003b4 100644 --- a/src/test/regress/expected/multi_sequence_default.out +++ b/src/test/regress/expected/multi_sequence_default.out @@ -6,6 +6,8 @@ SET citus.next_shard_id TO 890000; SET citus.shard_count TO 4; SET citus.shard_replication_factor TO 1; +CREATE SCHEMA sequence_default; +SET search_path = sequence_default, public; -- Cannot add a column involving DEFAULT nextval('..') because the table is not empty CREATE SEQUENCE seq_0; CREATE TABLE seq_test_0 (x int, y int); @@ -38,7 +40,7 @@ SELECT * FROM seq_test_0 ORDER BY 1, 2 LIMIT 5; (5 rows) \d seq_test_0 - Table "public.seq_test_0" + Table "sequence_default.seq_test_0" Column | Type | Collation | Nullable | Default --------------------------------------------------------------------- x | integer | | | @@ -83,7 +85,7 @@ CREATE SEQUENCE seq_4; ALTER TABLE seq_test_4 ADD COLUMN b int DEFAULT nextval('seq_4'); -- on worker it should generate high sequence number \c - - - :worker_1_port -INSERT INTO seq_test_4 VALUES (1,2) RETURNING *; +INSERT INTO sequence_default.seq_test_4 VALUES (1,2) RETURNING *; x | y | a | b --------------------------------------------------------------------- 1 | 2 | | 268435457 @@ -91,6 +93,7 @@ INSERT INTO seq_test_4 VALUES (1,2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -101,7 +104,7 @@ SELECT start_metadata_sync_to_node('localhost', :worker_1_port); CREATE SEQUENCE seq_1; -- type is bigint by default \d seq_1 - Sequence "public.seq_1" + Sequence "sequence_default.seq_1" Type | Start | Minimum | Maximum | Increment | Cycles? | Cache --------------------------------------------------------------------- bigint | 1 | 1 | 9223372036854775807 | 1 | no | 1 @@ -116,14 +119,14 @@ SELECT create_distributed_table('seq_test_1','x'); ALTER TABLE seq_test_1 ADD COLUMN z int DEFAULT nextval('seq_1'); -- type is changed to int \d seq_1 - Sequence "public.seq_1" + Sequence "sequence_default.seq_1" Type | Start | Minimum | Maximum | Increment | Cycles? | Cache --------------------------------------------------------------------- integer | 1 | 1 | 2147483647 | 1 | no | 1 -- check insertion is within int bounds in the worker \c - - - :worker_1_port -INSERT INTO seq_test_1 values (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_1 values (1, 2) RETURNING *; x | y | z --------------------------------------------------------------------- 1 | 2 | 268435457 @@ -131,6 +134,7 @@ INSERT INTO seq_test_1 values (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -163,7 +167,7 @@ SELECT create_distributed_table('seq_test_2','x'); CREATE TABLE seq_test_2_0(x int, y smallint DEFAULT nextval('seq_2')); -- shouldn't work SELECT create_distributed_table('seq_test_2_0','x'); -ERROR: The sequence public.seq_2 is already used for a different type in column 2 of the table public.seq_test_2 +ERROR: The sequence sequence_default.seq_2 is already used for a different type in column 2 of the table sequence_default.seq_test_2 DROP TABLE seq_test_2; DROP TABLE seq_test_2_0; -- should work @@ -178,19 +182,20 @@ DROP TABLE seq_test_2; CREATE TABLE seq_test_2 (x int, y int DEFAULT nextval('seq_2'), z bigint DEFAULT nextval('seq_2')); -- shouldn't work SELECT create_distributed_table('seq_test_2','x'); -ERROR: The sequence public.seq_2 is already used for a different type in column 3 of the table public.seq_test_2 +ERROR: The sequence sequence_default.seq_2 is already used for a different type in column 3 of the table sequence_default.seq_test_2 -- check rename is propagated properly ALTER SEQUENCE seq_2 RENAME TO sequence_2; -- check in the worker \c - - - :worker_1_port -\d sequence_2 - Sequence "public.sequence_2" +\d sequence_default.sequence_2 + Sequence "sequence_default.sequence_2" Type | Start | Minimum | Maximum | Increment | Cycles? | Cache --------------------------------------------------------------------- bigint | 281474976710657 | 281474976710657 | 562949953421313 | 1 | no | 1 \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -200,9 +205,8 @@ SELECT start_metadata_sync_to_node('localhost', :worker_1_port); -- check rename with another schema -- we notice that schema is also propagated as one of the sequence's dependencies CREATE SCHEMA sequence_default_0; -SET search_path TO public, sequence_default_0; CREATE SEQUENCE sequence_default_0.seq_3; -CREATE TABLE seq_test_3 (x int, y bigint DEFAULT nextval('seq_3')); +CREATE TABLE seq_test_3 (x int, y bigint DEFAULT nextval('sequence_default_0.seq_3')); SELECT create_distributed_table('seq_test_3', 'x'); create_distributed_table --------------------------------------------------------------------- @@ -220,6 +224,7 @@ ALTER SEQUENCE sequence_default_0.seq_3 RENAME TO sequence_3; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -253,7 +258,7 @@ INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; -- but is still present on worker \c - - - :worker_1_port -INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_5 VALUES (1, 2) RETURNING *; x | y | a --------------------------------------------------------------------- 1 | 2 | 268435457 @@ -261,6 +266,7 @@ INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -277,7 +283,7 @@ SELECT run_command_on_workers('DROP SCHEMA sequence_default_1 CASCADE'); -- now the sequence is gone from the worker as well \c - - - :worker_1_port -INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_5 VALUES (1, 2) RETURNING *; x | y | a --------------------------------------------------------------------- 1 | 2 | @@ -285,6 +291,7 @@ INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -319,20 +326,21 @@ CREATE TABLE seq_test_7_par (x text, s bigint DEFAULT nextval('seq_7_par'), t ti ALTER TABLE seq_test_7 ATTACH PARTITION seq_test_7_par FOR VALUES FROM ('2021-05-31') TO ('2021-06-01'); -- check that both sequences are in worker \c - - - :worker_1_port -\d seq_7 - Sequence "public.seq_7" +\d sequence_default.seq_7 + Sequence "sequence_default.seq_7" Type | Start | Minimum | Maximum | Increment | Cycles? | Cache --------------------------------------------------------------------- bigint | 281474976710657 | 281474976710657 | 562949953421313 | 1 | no | 1 -\d seq_7_par - Sequence "public.seq_7_par" +\d sequence_default.seq_7_par + Sequence "sequence_default.seq_7_par" Type | Start | Minimum | Maximum | Increment | Cycles? | Cache --------------------------------------------------------------------- bigint | 281474976710657 | 281474976710657 | 562949953421313 | 1 | no | 1 \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); start_metadata_sync_to_node --------------------------------------------------------------------- @@ -345,7 +353,7 @@ CREATE SEQUENCE seq_8; CREATE SCHEMA sequence_default_8; -- can change schema in a sequence not yet distributed ALTER SEQUENCE seq_8 SET SCHEMA sequence_default_8; -ALTER SEQUENCE sequence_default_8.seq_8 SET SCHEMA public; +ALTER SEQUENCE sequence_default_8.seq_8 SET SCHEMA sequence_default; CREATE TABLE seq_test_8 (x int, y int DEFAULT nextval('seq_8')); SELECT create_distributed_table('seq_test_8', 'x'); create_distributed_table @@ -378,12 +386,81 @@ CREATE SEQUENCE seq_10; CREATE TABLE seq_test_9 (x int, y int DEFAULT nextval('seq_9') - nextval('seq_10')); SELECT create_distributed_table('seq_test_9', 'x'); ERROR: More than one sequence in a column default is not supported for distribution --- clean up -DROP TABLE seq_test_0, seq_test_1, seq_test_2, seq_test_3, seq_test_4, seq_test_5, seq_test_6, seq_test_7, seq_test_8, seq_test_9; -DROP SEQUENCE seq_0, seq_1, sequence_2, seq_4, seq_6, seq_7, seq_7_par, seq_8, seq_9, seq_10; +-- Check some cases when default is defined by +-- DEFAULT nextval('seq_name'::text) (not by DEFAULT nextval('seq_name')) SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); stop_metadata_sync_to_node --------------------------------------------------------------------- (1 row) +CREATE SEQUENCE seq_11; +CREATE TABLE seq_test_10 (col0 int, col1 int DEFAULT nextval('seq_11'::text)); +SELECT create_reference_table('seq_test_10'); + create_reference_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO seq_test_10 VALUES (0); +CREATE TABLE seq_test_11 (col0 int, col1 bigint DEFAULT nextval('seq_11'::text)); +-- works but doesn't create seq_11 in the workers +SELECT start_metadata_sync_to_node('localhost', :worker_1_port); + start_metadata_sync_to_node +--------------------------------------------------------------------- + +(1 row) + +-- works because there is no dependency created between seq_11 and seq_test_10 +SELECT create_distributed_table('seq_test_11', 'col1'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- insertion from workers fails +\c - - - :worker_1_port +INSERT INTO sequence_default.seq_test_10 VALUES (1); +ERROR: relation "seq_11" does not exist +\c - - - :master_port +-- clean up +DROP TABLE sequence_default.seq_test_7_par; +DROP SCHEMA sequence_default CASCADE; +NOTICE: drop cascades to 23 other objects +DETAIL: drop cascades to sequence sequence_default.seq_0 +drop cascades to table sequence_default.seq_test_0 +drop cascades to table sequence_default.seq_test_4 +drop cascades to sequence sequence_default.seq_4 +drop cascades to sequence sequence_default.seq_1 +drop cascades to table sequence_default.seq_test_1 +drop cascades to sequence sequence_default.sequence_2 +drop cascades to table sequence_default.seq_test_2 +drop cascades to table sequence_default.seq_test_3 +drop cascades to table sequence_default.seq_test_5 +drop cascades to sequence sequence_default.seq_6 +drop cascades to table sequence_default.seq_test_6 +drop cascades to sequence sequence_default.seq_7 +drop cascades to table sequence_default.seq_test_7 +drop cascades to sequence sequence_default.seq_7_par +drop cascades to sequence sequence_default.seq_8 +drop cascades to table sequence_default.seq_test_8 +drop cascades to sequence sequence_default.seq_9 +drop cascades to sequence sequence_default.seq_10 +drop cascades to table sequence_default.seq_test_9 +drop cascades to sequence sequence_default.seq_11 +drop cascades to table sequence_default.seq_test_10 +drop cascades to table sequence_default.seq_test_11 +SELECT run_command_on_workers('DROP SCHEMA IF EXISTS sequence_default CASCADE'); + run_command_on_workers +--------------------------------------------------------------------- + (localhost,57637,t,"DROP SCHEMA") + (localhost,57638,t,"DROP SCHEMA") +(2 rows) + +SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); + stop_metadata_sync_to_node +--------------------------------------------------------------------- + +(1 row) + +SET search_path TO public; diff --git a/src/test/regress/sql/multi_sequence_default.sql b/src/test/regress/sql/multi_sequence_default.sql index 61636c5fe..a986e5ec8 100644 --- a/src/test/regress/sql/multi_sequence_default.sql +++ b/src/test/regress/sql/multi_sequence_default.sql @@ -7,6 +7,8 @@ SET citus.next_shard_id TO 890000; SET citus.shard_count TO 4; SET citus.shard_replication_factor TO 1; +CREATE SCHEMA sequence_default; +SET search_path = sequence_default, public; -- Cannot add a column involving DEFAULT nextval('..') because the table is not empty @@ -52,9 +54,10 @@ CREATE SEQUENCE seq_4; ALTER TABLE seq_test_4 ADD COLUMN b int DEFAULT nextval('seq_4'); -- on worker it should generate high sequence number \c - - - :worker_1_port -INSERT INTO seq_test_4 VALUES (1,2) RETURNING *; +INSERT INTO sequence_default.seq_test_4 VALUES (1,2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); @@ -69,9 +72,10 @@ ALTER TABLE seq_test_1 ADD COLUMN z int DEFAULT nextval('seq_1'); \d seq_1 -- check insertion is within int bounds in the worker \c - - - :worker_1_port -INSERT INTO seq_test_1 values (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_1 values (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); @@ -107,16 +111,16 @@ SELECT create_distributed_table('seq_test_2','x'); ALTER SEQUENCE seq_2 RENAME TO sequence_2; -- check in the worker \c - - - :worker_1_port -\d sequence_2 +\d sequence_default.sequence_2 \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); -- check rename with another schema -- we notice that schema is also propagated as one of the sequence's dependencies CREATE SCHEMA sequence_default_0; -SET search_path TO public, sequence_default_0; CREATE SEQUENCE sequence_default_0.seq_3; -CREATE TABLE seq_test_3 (x int, y bigint DEFAULT nextval('seq_3')); +CREATE TABLE seq_test_3 (x int, y bigint DEFAULT nextval('sequence_default_0.seq_3')); SELECT create_distributed_table('seq_test_3', 'x'); ALTER SEQUENCE sequence_default_0.seq_3 RENAME TO sequence_3; -- check in the worker @@ -124,6 +128,7 @@ ALTER SEQUENCE sequence_default_0.seq_3 RENAME TO sequence_3; \d sequence_default_0.sequence_3 \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); DROP SEQUENCE sequence_default_0.sequence_3 CASCADE; DROP SCHEMA sequence_default_0; @@ -140,17 +145,19 @@ DROP SCHEMA sequence_default_1 CASCADE; INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; -- but is still present on worker \c - - - :worker_1_port -INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_5 VALUES (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); -- apply workaround SELECT run_command_on_workers('DROP SCHEMA sequence_default_1 CASCADE'); -- now the sequence is gone from the worker as well \c - - - :worker_1_port -INSERT INTO seq_test_5 VALUES (1, 2) RETURNING *; +INSERT INTO sequence_default.seq_test_5 VALUES (1, 2) RETURNING *; \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); @@ -173,10 +180,11 @@ CREATE TABLE seq_test_7_par (x text, s bigint DEFAULT nextval('seq_7_par'), t ti ALTER TABLE seq_test_7 ATTACH PARTITION seq_test_7_par FOR VALUES FROM ('2021-05-31') TO ('2021-06-01'); -- check that both sequences are in worker \c - - - :worker_1_port -\d seq_7 -\d seq_7_par +\d sequence_default.seq_7 +\d sequence_default.seq_7_par \c - - - :master_port SET citus.shard_replication_factor TO 1; +SET search_path = sequence_default, public; SELECT start_metadata_sync_to_node('localhost', :worker_1_port); @@ -186,7 +194,7 @@ CREATE SEQUENCE seq_8; CREATE SCHEMA sequence_default_8; -- can change schema in a sequence not yet distributed ALTER SEQUENCE seq_8 SET SCHEMA sequence_default_8; -ALTER SEQUENCE sequence_default_8.seq_8 SET SCHEMA public; +ALTER SEQUENCE sequence_default_8.seq_8 SET SCHEMA sequence_default; CREATE TABLE seq_test_8 (x int, y int DEFAULT nextval('seq_8')); SELECT create_distributed_table('seq_test_8', 'x'); -- cannot change sequence specifications @@ -209,7 +217,26 @@ CREATE TABLE seq_test_9 (x int, y int DEFAULT nextval('seq_9') - nextval('seq_10 SELECT create_distributed_table('seq_test_9', 'x'); --- clean up -DROP TABLE seq_test_0, seq_test_1, seq_test_2, seq_test_3, seq_test_4, seq_test_5, seq_test_6, seq_test_7, seq_test_8, seq_test_9; -DROP SEQUENCE seq_0, seq_1, sequence_2, seq_4, seq_6, seq_7, seq_7_par, seq_8, seq_9, seq_10; +-- Check some cases when default is defined by +-- DEFAULT nextval('seq_name'::text) (not by DEFAULT nextval('seq_name')) SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); +CREATE SEQUENCE seq_11; +CREATE TABLE seq_test_10 (col0 int, col1 int DEFAULT nextval('seq_11'::text)); +SELECT create_reference_table('seq_test_10'); +INSERT INTO seq_test_10 VALUES (0); +CREATE TABLE seq_test_11 (col0 int, col1 bigint DEFAULT nextval('seq_11'::text)); +-- works but doesn't create seq_11 in the workers +SELECT start_metadata_sync_to_node('localhost', :worker_1_port); +-- works because there is no dependency created between seq_11 and seq_test_10 +SELECT create_distributed_table('seq_test_11', 'col1'); +-- insertion from workers fails +\c - - - :worker_1_port +INSERT INTO sequence_default.seq_test_10 VALUES (1); +\c - - - :master_port + +-- clean up +DROP TABLE sequence_default.seq_test_7_par; +DROP SCHEMA sequence_default CASCADE; +SELECT run_command_on_workers('DROP SCHEMA IF EXISTS sequence_default CASCADE'); +SELECT stop_metadata_sync_to_node('localhost', :worker_1_port); +SET search_path TO public;