From aa047bda165d5b0d2f8b58b1d3adb15e6591d3da Mon Sep 17 00:00:00 2001 From: Nitish Upreti Date: Wed, 22 Jun 2022 16:42:27 -0700 Subject: [PATCH] Negative tests --- .../citus_split_shard_by_split_points.c | 5 +- .../distributed/operations/shard_split.c | 4 +- src/backend/distributed/utils/array_type.c | 34 +---- src/include/distributed/utils/array_type.h | 2 +- ...s_split_shard_by_split_points_negative.out | 140 ++++++++++++++++++ src/test/regress/split_schedule | 1 + ...s_split_shard_by_split_points_negative.sql | 117 +++++++++++++++ 7 files changed, 266 insertions(+), 37 deletions(-) create mode 100644 src/test/regress/expected/citus_split_shard_by_split_points_negative.out create mode 100644 src/test/regress/sql/citus_split_shard_by_split_points_negative.sql diff --git a/src/backend/distributed/operations/citus_split_shard_by_split_points.c b/src/backend/distributed/operations/citus_split_shard_by_split_points.c index 751539485..1392f40fc 100644 --- a/src/backend/distributed/operations/citus_split_shard_by_split_points.c +++ b/src/backend/distributed/operations/citus_split_shard_by_split_points.c @@ -12,10 +12,10 @@ #include "postgres.h" #include "catalog/pg_type.h" #include "nodes/pg_list.h" -#include "distributed/utils/array_type.h" #include "lib/stringinfo.h" #include "utils/builtins.h" #include "utils/lsyscache.h" +#include "distributed/utils/array_type.h" #include "distributed/colocation_utils.h" #include "distributed/metadata_cache.h" #include "distributed/shardinterval_utils.h" @@ -46,8 +46,7 @@ citus_split_shard_by_split_points(PG_FUNCTION_ARGS) uint64 shardIdToSplit = DatumGetUInt64(PG_GETARG_DATUM(0)); ArrayType *splitPointsArrayObject = PG_GETARG_ARRAYTYPE_P(1); - List *shardSplitPointsList = TextArrayTypeToIntegerList(splitPointsArrayObject, - INT4OID); + List *shardSplitPointsList = TextArrayTypeToIntegerList(splitPointsArrayObject); ArrayType *nodeIdsArrayObject = PG_GETARG_ARRAYTYPE_P(2); List *nodeIdsForPlacementList = IntegerArrayTypeToList(nodeIdsArrayObject); diff --git a/src/backend/distributed/operations/shard_split.c b/src/backend/distributed/operations/shard_split.c index dc03c35b1..4484cdee5 100644 --- a/src/backend/distributed/operations/shard_split.c +++ b/src/backend/distributed/operations/shard_split.c @@ -256,8 +256,10 @@ ErrorIfCannotSplitShardExtended(SplitOperation splitOperation, ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg( - "Split point %d is outside the min/max range for shard id %lu.", + "Split point %d is outside the min/max range(%d, %d) for shard id %lu.", shardSplitPointValue, + DatumGetInt32(shardIntervalToSplit->minValue), + DatumGetInt32(shardIntervalToSplit->maxValue), shardIntervalToSplit->shardId))); } diff --git a/src/backend/distributed/utils/array_type.c b/src/backend/distributed/utils/array_type.c index ca84caf44..62ed45776 100644 --- a/src/backend/distributed/utils/array_type.c +++ b/src/backend/distributed/utils/array_type.c @@ -126,7 +126,7 @@ IntegerArrayTypeToList(ArrayType *arrayObject) * Converts Text ArrayType to Integer List. */ extern List * -TextArrayTypeToIntegerList(ArrayType *arrayObject, Oid datumTypeId) +TextArrayTypeToIntegerList(ArrayType *arrayObject) { List *list = NULL; Datum *datumObjectArray = DeconstructArrayObject(arrayObject); @@ -135,37 +135,7 @@ TextArrayTypeToIntegerList(ArrayType *arrayObject, Oid datumTypeId) for (int index = 0; index < arrayObjectCount; index++) { char *intAsStr = text_to_cstring(DatumGetTextP(datumObjectArray[index])); - - switch (datumTypeId) - { - case INT2OID: - { - int16_t *int16Value = palloc0(sizeof(int16_t)); - *int16Value = pg_strtoint16(intAsStr); - list = lappend(list, (void *) int16Value); - break; - } - - case INT4OID: - { - int32_t *int32Value = palloc0(sizeof(int32_t)); - *int32Value = pg_strtoint32(intAsStr); - list = lappend(list, (void *) int32Value); - break; - } - - case INT8OID: - { - int64_t *int64Value = palloc0(sizeof(int64_t)); - *int64Value = pg_strtoint64(intAsStr); - list = lappend(list, (void *) int64Value); - break; - } - - default: - ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("Unsupported datum type for array."))); - } + list = lappend_int(list, pg_strtoint32(intAsStr)); } return list; diff --git a/src/include/distributed/utils/array_type.h b/src/include/distributed/utils/array_type.h index 9d0ccbdd4..4599b8a9f 100644 --- a/src/include/distributed/utils/array_type.h +++ b/src/include/distributed/utils/array_type.h @@ -21,6 +21,6 @@ extern int32 ArrayObjectCount(ArrayType *arrayObject); extern ArrayType * DatumArrayToArrayType(Datum *datumArray, int datumCount, Oid datumTypeId); extern List * IntegerArrayTypeToList(ArrayType *arrayObject); -extern List * TextArrayTypeToIntegerList(ArrayType *arrayObject, Oid datumTypeId); +extern List * TextArrayTypeToIntegerList(ArrayType *arrayObject); #endif /* CITUS_ARRAY_TYPE_H */ diff --git a/src/test/regress/expected/citus_split_shard_by_split_points_negative.out b/src/test/regress/expected/citus_split_shard_by_split_points_negative.out new file mode 100644 index 000000000..788baaf46 --- /dev/null +++ b/src/test/regress/expected/citus_split_shard_by_split_points_negative.out @@ -0,0 +1,140 @@ +-- Negative test cases for citus_split_shard_by_split_points UDF. +CREATE SCHEMA citus_split_shard_by_split_points_negative; +SET search_path TO citus_split_shard_by_split_points_negative; +SET citus.shard_count TO 4; +SET citus.shard_replication_factor TO 1; +SET citus.next_shard_id TO 60761300; +CREATE TABLE range_paritioned_table_to_split(rid bigserial PRIMARY KEY, value char); +SELECT create_distributed_table('range_paritioned_table_to_split', 'rid', 'range'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Shards are not created automatically for range distributed table. +SELECT master_create_empty_shard('range_paritioned_table_to_split'); + master_create_empty_shard +--------------------------------------------------------------------- + 60761300 +(1 row) + +SET citus.next_shard_id TO 49761300; +CREATE TABLE table_to_split (id bigserial PRIMARY KEY, value char); +-- Shard1 | -2147483648 | -1073741825 +-- Shard2 | -1073741824 | -1 +-- Shard3 | 0 | 1073741823 +-- Shard4 | 1073741824 | 2147483647 +SELECT create_distributed_table('table_to_split','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SELECT nodeid AS worker_1_node FROM pg_dist_node WHERE nodeport=:worker_1_port \gset +SELECT nodeid AS worker_2_node FROM pg_dist_node WHERE nodeport=:worker_2_port \gset +-- UDF fails for range partitioned tables. +SELECT citus_split_shard_by_split_points( + 60761300, + ARRAY[-1073741826], + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails if number of placement node list does not exceed split points by one. +-- Example: One split point defines two way split (2 worker nodes needed). +SELECT citus_split_shard_by_split_points( + 49761300, + -- 2 split points defined making it a 3 way split but we only specify 2 placement lists. + ARRAY[-1073741826, -107374182], + ARRAY[:worker_1_node, :worker_2_node]); -- 2 worker nodes. +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails if split ranges specified are not within the shard id to split. +SELECT citus_split_shard_by_split_points( + 49761300, -- Shard range is from (-2147483648, -1073741825) + ARRAY[0], -- The range we specified is 0 which is not in the range. + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails if split points are not strictly increasing. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY[50, 35], + ARRAY[:worker_1_node, :worker_2_node, :worker_1_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY[50, 50], + ARRAY[:worker_1_node, :worker_2_node, :worker_1_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails if nodeIds are < 1 or Invalid. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY[50], + ARRAY[0, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY[50], + ARRAY[101, 201]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails if split point specified is equal to the max value in the range. +-- Example: ShardId 81060002 range is from (-2147483648, -1073741825) +-- '-1073741825' as split point is invalid. +-- '-1073741826' is valid and will split to: (-2147483648, -1073741826) and (-1073741825, -1073741825) +SELECT citus_split_shard_by_split_points( + 49761300, -- Shard range is from (-2147483648, -1073741825) + ARRAY[-1073741825], -- Split point equals shard's max value. + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- UDF fails where source shard cannot be split further i.e min and max range is equal. +-- Create a Shard where range cannot be split further +SELECT isolate_tenant_to_new_shard('table_to_split', 1); + isolate_tenant_to_new_shard +--------------------------------------------------------------------- + 49761305 +(1 row) + +SELECT citus_split_shard_by_split_points( + 49761305, + ARRAY[-1073741826], + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- Create distributed table with replication factor > 1 +SET citus.shard_replication_factor TO 2; +SET citus.next_shard_id TO 51261400; +CREATE TABLE table_to_split_replication_factor_2 (id bigserial PRIMARY KEY, value char); +SELECT create_distributed_table('table_to_split_replication_factor_2','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- UDF fails for replication factor > 1 +SELECT citus_split_shard_by_split_points( + 51261400, + ARRAY[-1073741826], + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +-- Create distributed table with columnar type. +SET citus.next_shard_id TO 51271400; +CREATE TABLE table_to_split_columnar (id bigserial PRIMARY KEY, value char) USING columnar; +SELECT create_distributed_table('table_to_split_columnar','id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- UDF fails for columnar table. +SELECT citus_split_shard_by_split_points( + 51271400, + ARRAY[-1073741826], + ARRAY[:worker_1_node, :worker_2_node]); +ERROR: function citus_split_shard_by_split_points(integer, integer[], integer[]) does not exist +HINT: No function matches the given name and argument types. You might need to add explicit type casts. diff --git a/src/test/regress/split_schedule b/src/test/regress/split_schedule index 344b805b9..cba127195 100644 --- a/src/test/regress/split_schedule +++ b/src/test/regress/split_schedule @@ -7,3 +7,4 @@ test: tablespace # Split tests go here. test: worker_split_binary_copy_test test: worker_split_text_copy_test +test: citus_split_shard_by_split_points_negative diff --git a/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql b/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql new file mode 100644 index 000000000..bdaf32682 --- /dev/null +++ b/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql @@ -0,0 +1,117 @@ +-- Negative test cases for citus_split_shard_by_split_points UDF. + +CREATE SCHEMA citus_split_shard_by_split_points_negative; +SET search_path TO citus_split_shard_by_split_points_negative; +SET citus.shard_count TO 4; +SET citus.shard_replication_factor TO 1; +SET citus.next_shard_id TO 60761300; + +CREATE TABLE range_paritioned_table_to_split(rid bigserial PRIMARY KEY, value char); +SELECT create_distributed_table('range_paritioned_table_to_split', 'rid', 'range'); +-- Shards are not created automatically for range distributed table. +SELECT master_create_empty_shard('range_paritioned_table_to_split'); + +SET citus.next_shard_id TO 49761300; +CREATE TABLE table_to_split (id bigserial PRIMARY KEY, value char); + +-- Shard1 | -2147483648 | -1073741825 +-- Shard2 | -1073741824 | -1 +-- Shard3 | 0 | 1073741823 +-- Shard4 | 1073741824 | 2147483647 +SELECT create_distributed_table('table_to_split','id'); + +SELECT nodeid AS worker_1_node FROM pg_dist_node WHERE nodeport=:worker_1_port \gset +SELECT nodeid AS worker_2_node FROM pg_dist_node WHERE nodeport=:worker_2_port \gset + +-- UDF fails for range partitioned tables. +SELECT citus_split_shard_by_split_points( + 60761300, + ARRAY['-1073741826'], + ARRAY[:worker_1_node, :worker_2_node]); + +-- UDF fails if number of placement node list does not exceed split points by one. +-- Example: One split point defines two way split (2 worker nodes needed). +SELECT citus_split_shard_by_split_points( + 49761300, + -- 2 split points defined making it a 3 way split but we only specify 2 placement lists. + ARRAY['-1073741826', '-107374182'], + ARRAY[:worker_1_node, :worker_2_node]); -- 2 worker nodes. + +-- UDF fails if split ranges specified are not within the shard id to split. +SELECT citus_split_shard_by_split_points( + 49761300, -- Shard range is from (-2147483648, -1073741825) + ARRAY['0'], -- The range we specified is 0 which is not in the range. + ARRAY[:worker_1_node, :worker_2_node]); + +-- UDF fails if split points are not strictly increasing. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50', '35'], + ARRAY[:worker_1_node, :worker_2_node, :worker_1_node]); + +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50', '50'], + ARRAY[:worker_1_node, :worker_2_node, :worker_1_node]); + +-- UDF fails if nodeIds are < 1 or Invalid. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[0, :worker_2_node]); + +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201]); + +-- UDF fails if split point specified is equal to the max value in the range. +-- Example: ShardId 81060002 range is from (-2147483648, -1073741825) +-- '-1073741825' as split point is invalid. +-- '-1073741826' is valid and will split to: (-2147483648, -1073741826) and (-1073741825, -1073741825) +SELECT citus_split_shard_by_split_points( + 49761300, -- Shard range is from (-2147483648, -1073741825) + ARRAY['-1073741825'], -- Split point equals shard's max value. + ARRAY[:worker_1_node, :worker_2_node]); + +-- UDF fails where source shard cannot be split further i.e min and max range is equal. +-- Create a Shard where range cannot be split further +SELECT isolate_tenant_to_new_shard('table_to_split', 1); +SELECT citus_split_shard_by_split_points( + 49761305, + ARRAY['-1073741826'], + ARRAY[:worker_1_node, :worker_2_node]); + +-- Create distributed table with replication factor > 1 +SET citus.shard_replication_factor TO 2; +SET citus.next_shard_id TO 51261400; +CREATE TABLE table_to_split_replication_factor_2 (id bigserial PRIMARY KEY, value char); +SELECT create_distributed_table('table_to_split_replication_factor_2','id'); + +-- UDF fails for replication factor > 1 +SELECT citus_split_shard_by_split_points( + 51261400, + ARRAY['-1073741826'], + ARRAY[:worker_1_node, :worker_2_node]); + +-- Create distributed table with columnar type. +SET citus.next_shard_id TO 51271400; +CREATE TABLE table_to_split_columnar (id bigserial PRIMARY KEY, value char) USING columnar; +SELECT create_distributed_table('table_to_split_columnar','id'); + +-- UDF fails for columnar table. +SELECT citus_split_shard_by_split_points( + 51271400, + ARRAY['-1073741826'], + ARRAY[:worker_1_node, :worker_2_node]); + +-- Create distributed table which is partitioned. +SET citus.next_shard_id TO 51271900; +CREATE TABLE table_to_split_partitioned(id integer, dt date) PARTITION BY RANGE(dt); +SELECT create_distributed_table('table_to_split_partitioned','id'); + +-- UDF fails for partitioned table. +SELECT citus_split_shard_by_split_points( + 51271900, + ARRAY['-1073741826'], + ARRAY[:worker_1_node, :worker_2_node]);