diff --git a/src/backend/distributed/commands/create_distributed_table.c b/src/backend/distributed/commands/create_distributed_table.c index c639d836c..13f5f0692 100644 --- a/src/backend/distributed/commands/create_distributed_table.c +++ b/src/backend/distributed/commands/create_distributed_table.c @@ -31,6 +31,7 @@ #include "catalog/pg_opclass.h" #include "catalog/pg_proc.h" #include "catalog/pg_trigger.h" +#include "catalog/pg_type.h" #include "commands/defrem.h" #include "commands/extension.h" #include "commands/sequence.h" @@ -579,7 +580,7 @@ CreateDistributedTable(Oid relationId, char *distributionColumnName, * explicitly. */ void -EnsureSequenceTypeSupported(Oid seqOid, Oid seqTypId, Oid ownerRelationId) +EnsureSequenceTypeSupported(Oid seqOid, Oid attributeTypeId, Oid ownerRelationId) { List *citusTableIdList = CitusTableTypeIdList(ANY_CITUS_TABLE_TYPE); citusTableIdList = list_append_unique_oid(citusTableIdList, ownerRelationId); @@ -606,9 +607,9 @@ EnsureSequenceTypeSupported(Oid seqOid, Oid seqTypId, Oid ownerRelationId) */ if (currentSeqOid == seqOid) { - Oid currentSeqTypId = GetAttributeTypeOid(citusTableId, - currentAttnum); - if (seqTypId != currentSeqTypId) + Oid currentAttributeTypId = GetAttributeTypeOid(citusTableId, + currentAttnum); + if (attributeTypeId != currentAttributeTypId) { char *sequenceName = generate_qualified_relation_name( seqOid); @@ -685,17 +686,29 @@ EnsureDistributedSequencesHaveOneType(Oid relationId, List *dependentSequenceLis * We should make sure that the type of the column that uses * that sequence is supported */ - Oid seqTypId = GetAttributeTypeOid(relationId, attnum); - EnsureSequenceTypeSupported(sequenceOid, seqTypId, relationId); + Oid attributeTypeId = GetAttributeTypeOid(relationId, attnum); + EnsureSequenceTypeSupported(sequenceOid, attributeTypeId, relationId); /* * Alter the sequence's data type in the coordinator if needed. + * + * First, we should only change the sequence type if the column + * is a supported sequence type. For example, if a sequence is used + * in an expression which then becomes a text, we should not try to + * alter the sequence type to text. Postgres only supports int2, int4 + * and int8 as the sequence type. + * * A sequence's type is bigint by default and it doesn't change even if * it's used in an int column. We should change the type if needed, * and not allow future ALTER SEQUENCE ... TYPE ... commands for - * sequences used as defaults in distributed tables + * sequences used as defaults in distributed tables. */ - AlterSequenceType(sequenceOid, seqTypId); + if (attributeTypeId == INT2OID || + attributeTypeId == INT4OID || + attributeTypeId == INT8OID) + { + AlterSequenceType(sequenceOid, attributeTypeId); + } } } diff --git a/src/include/distributed/metadata_utility.h b/src/include/distributed/metadata_utility.h index c03b3abe7..0d9f125d8 100644 --- a/src/include/distributed/metadata_utility.h +++ b/src/include/distributed/metadata_utility.h @@ -290,7 +290,8 @@ extern bool GetNodeDiskSpaceStatsForConnection(MultiConnection *connection, uint64 *availableBytes, uint64 *totalBytes); extern void ExecuteQueryViaSPI(char *query, int SPIOK); -extern void EnsureSequenceTypeSupported(Oid seqOid, Oid seqTypId, Oid ownerRelationId); +extern void EnsureSequenceTypeSupported(Oid seqOid, Oid attributeTypeId, Oid + ownerRelationId); extern void AlterSequenceType(Oid seqOid, Oid typeOid); extern void EnsureRelationHasCompatibleSequenceTypes(Oid relationId); #endif /* METADATA_UTILITY_H */ diff --git a/src/test/regress/expected/sequences_with_different_types.out b/src/test/regress/expected/sequences_with_different_types.out new file mode 100644 index 000000000..3a26f6b7f --- /dev/null +++ b/src/test/regress/expected/sequences_with_different_types.out @@ -0,0 +1,102 @@ +CREATE SCHEMA sequences_with_different_types; +SET search_path TO sequences_with_different_types; +CREATE TYPE two_big_ints AS (a bigint, b bigint); +-- by default, sequences get bigint type +CREATE SEQUENCE bigint_sequence_1; +CREATE SEQUENCE bigint_sequence_2 START 10000; +CREATE SEQUENCE bigint_sequence_3 INCREMENT 10; +CREATE SEQUENCE bigint_sequence_4 MINVALUE 1000000; +CREATE SEQUENCE bigint_sequence_5; +CREATE SEQUENCE bigint_sequence_8; +CREATE TABLE table_1 +( + user_id bigint, + user_code_1 text DEFAULT (('CD'::text || lpad(nextval('bigint_sequence_1'::regclass)::text, 10, '0'::text))), + user_code_2 text DEFAULT nextval('bigint_sequence_2'::regclass)::text, + user_code_3 text DEFAULT (nextval('bigint_sequence_3'::regclass) + 1000)::text, + user_code_4 float DEFAULT nextval('bigint_sequence_4'::regclass), + user_code_5 two_big_ints DEFAULT (nextval('bigint_sequence_5'::regclass), nextval('bigint_sequence_5'::regclass)), + user_code_8 jsonb DEFAULT to_jsonb('test'::text) || to_jsonb(nextval('bigint_sequence_8'::regclass)) +); +SELECT create_distributed_table('table_1', 'user_id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO table_1 VALUES (1, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT), (2, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING *; + user_id | user_code_1 | user_code_2 | user_code_3 | user_code_4 | user_code_5 | user_code_8 +--------------------------------------------------------------------- + 1 | CD0000000001 | 10000 | 1001 | 1000000 | (1,2) | ["test", 1] + 2 | CD0000000002 | 10001 | 1011 | 1000001 | (3,4) | ["test", 2] +(2 rows) + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; +INSERT INTO table_1 VALUES (3, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT), (4, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING *; + user_id | user_code_1 | user_code_2 | user_code_3 | user_code_4 | user_code_5 | user_code_8 +--------------------------------------------------------------------- + 3 | CD3940649673 | 3940649673949185 | 3940649673950185 | 3.94064967394918e+15 | (3940649673949185,3940649673949186) | ["test", 3940649673949185] + 4 | CD3940649673 | 3940649673949186 | 3940649673950195 | 3.94064967394919e+15 | (3940649673949187,3940649673949188) | ["test", 3940649673949186] +(2 rows) + +\c - - - :master_port +SET search_path TO sequences_with_different_types; +CREATE SEQUENCE bigint_sequence_6; +CREATE TABLE table_2 +( + user_id bigint, + user_code OID DEFAULT nextval('bigint_sequence_6'::regclass) +); +SELECT create_distributed_table('table_2', 'user_id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- on the coordinator, the sequence starts from 0 +INSERT INTO table_2 VALUES (1, DEFAULT) RETURNING *; + user_id | user_code +--------------------------------------------------------------------- + 1 | 1 +(1 row) + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; +-- this fails because on the workers the start value of the sequence +-- is greater than the largest value of an oid +INSERT INTO table_2 VALUES (1, DEFAULT) RETURNING *; +ERROR: OID out of range +\c - - - :master_port +SET search_path TO sequences_with_different_types; +CREATE SEQUENCE bigint_sequence_7; +CREATE TABLE table_3 +( + user_id bigint, + user_code boolean DEFAULT ((nextval('bigint_sequence_7'::regclass)%2)::int)::boolean +); +SELECT create_distributed_table('table_3', 'user_id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO table_3 VALUES (1, DEFAULT), (2, DEFAULT) RETURNING *; + user_id | user_code +--------------------------------------------------------------------- + 1 | t + 2 | f +(2 rows) + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; +INSERT INTO table_3 VALUES (3, DEFAULT), (4, DEFAULT) RETURNING *; + user_id | user_code +--------------------------------------------------------------------- + 3 | t + 4 | f +(2 rows) + +\c - - - :master_port +SET client_min_messages TO WARNING; +DROP SCHEMA sequences_with_different_types CASCADE; diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 088a7b8bd..6a2785553 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -53,7 +53,7 @@ test: subqueries_deep subquery_view subquery_partitioning subqueries_not_support test: subquery_in_targetlist subquery_in_where subquery_complex_target_list subquery_append test: subquery_prepared_statements test: non_colocated_leaf_subquery_joins non_colocated_subquery_joins -test: cte_inline recursive_view_local_table values +test: cte_inline recursive_view_local_table values sequences_with_different_types test: pg13 pg12 # run pg14 sequentially as it syncs metadata test: pg14 diff --git a/src/test/regress/sql/sequences_with_different_types.sql b/src/test/regress/sql/sequences_with_different_types.sql new file mode 100644 index 000000000..d9e7e63f5 --- /dev/null +++ b/src/test/regress/sql/sequences_with_different_types.sql @@ -0,0 +1,74 @@ +CREATE SCHEMA sequences_with_different_types; +SET search_path TO sequences_with_different_types; + +CREATE TYPE two_big_ints AS (a bigint, b bigint); +-- by default, sequences get bigint type +CREATE SEQUENCE bigint_sequence_1; +CREATE SEQUENCE bigint_sequence_2 START 10000; +CREATE SEQUENCE bigint_sequence_3 INCREMENT 10; +CREATE SEQUENCE bigint_sequence_4 MINVALUE 1000000; +CREATE SEQUENCE bigint_sequence_5; +CREATE SEQUENCE bigint_sequence_8; + +CREATE TABLE table_1 +( + user_id bigint, + user_code_1 text DEFAULT (('CD'::text || lpad(nextval('bigint_sequence_1'::regclass)::text, 10, '0'::text))), + user_code_2 text DEFAULT nextval('bigint_sequence_2'::regclass)::text, + user_code_3 text DEFAULT (nextval('bigint_sequence_3'::regclass) + 1000)::text, + user_code_4 float DEFAULT nextval('bigint_sequence_4'::regclass), + user_code_5 two_big_ints DEFAULT (nextval('bigint_sequence_5'::regclass), nextval('bigint_sequence_5'::regclass)), + user_code_8 jsonb DEFAULT to_jsonb('test'::text) || to_jsonb(nextval('bigint_sequence_8'::regclass)) + +); +SELECT create_distributed_table('table_1', 'user_id'); + +INSERT INTO table_1 VALUES (1, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT), (2, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING *; + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; + +INSERT INTO table_1 VALUES (3, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT), (4, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT) RETURNING *; + + +\c - - - :master_port +SET search_path TO sequences_with_different_types; +CREATE SEQUENCE bigint_sequence_6; + +CREATE TABLE table_2 +( + user_id bigint, + user_code OID DEFAULT nextval('bigint_sequence_6'::regclass) +); +SELECT create_distributed_table('table_2', 'user_id'); + +-- on the coordinator, the sequence starts from 0 +INSERT INTO table_2 VALUES (1, DEFAULT) RETURNING *; + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; + +-- this fails because on the workers the start value of the sequence +-- is greater than the largest value of an oid +INSERT INTO table_2 VALUES (1, DEFAULT) RETURNING *; + +\c - - - :master_port +SET search_path TO sequences_with_different_types; +CREATE SEQUENCE bigint_sequence_7; + +CREATE TABLE table_3 +( + user_id bigint, + user_code boolean DEFAULT ((nextval('bigint_sequence_7'::regclass)%2)::int)::boolean +); +SELECT create_distributed_table('table_3', 'user_id'); + +INSERT INTO table_3 VALUES (1, DEFAULT), (2, DEFAULT) RETURNING *; + +\c - - - :worker_1_port +SET search_path TO sequences_with_different_types; +INSERT INTO table_3 VALUES (3, DEFAULT), (4, DEFAULT) RETURNING *; + +\c - - - :master_port +SET client_min_messages TO WARNING; +DROP SCHEMA sequences_with_different_types CASCADE;