From 4d9a733c2f39e5602ab98be42ee83a3ddbe91317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Tue, 14 Jan 2020 23:57:06 +0000 Subject: [PATCH] Fix inserting multiple values with row expression partition column causing the insert to be ignored Raise an error instead of silently inserting nothing if we hit this condition in the future --- .../planner/multi_router_planner.c | 5 +- src/backend/distributed/utils/citus_clauses.c | 1 + .../regress/expected/multi_row_insert.out | 50 +++++++++++++++++++ src/test/regress/multi_schedule | 1 + src/test/regress/sql/multi_row_insert.sql | 29 +++++++++++ 5 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 src/test/regress/expected/multi_row_insert.out create mode 100644 src/test/regress/sql/multi_row_insert.sql diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index fb6441db9..59c6325dd 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -2559,8 +2559,9 @@ BuildRoutesForInsert(Query *query, DeferredErrorMessage **planningError) if (!IsA(insertValues->partitionValueExpr, Const)) { - /* shard pruning not possible right now */ - return NIL; + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("failed to evaluate partition key in insert"), + errhint("try using constant values for partition column"))); } Const *partitionValueConst = (Const *) insertValues->partitionValueExpr; diff --git a/src/backend/distributed/utils/citus_clauses.c b/src/backend/distributed/utils/citus_clauses.c index 2f18be9c7..304a22024 100644 --- a/src/backend/distributed/utils/citus_clauses.c +++ b/src/backend/distributed/utils/citus_clauses.c @@ -92,6 +92,7 @@ PartiallyEvaluateExpression(Node *expression, PlanState *planState) case T_CoerceViaIO: case T_ArrayCoerceExpr: case T_ScalarArrayOpExpr: + case T_RowExpr: case T_RowCompareExpr: case T_RelabelType: case T_CoerceToDomain: diff --git a/src/test/regress/expected/multi_row_insert.out b/src/test/regress/expected/multi_row_insert.out new file mode 100644 index 000000000..6f7c3087a --- /dev/null +++ b/src/test/regress/expected/multi_row_insert.out @@ -0,0 +1,50 @@ +-- Test multi row insert with composite type as partition key +-- https://github.com/citusdata/citus/issues/3357 +SET citus.next_shard_id TO 4213581; +SET citus.shard_replication_factor TO 1; +CREATE TYPE composite_key_type AS (f1 int, f2 text); +CREATE TABLE source_table_xyz(key composite_key_type, value int, mapped_key composite_key_type); +SELECT create_distributed_table('source_table_xyz', 'key', 'range'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +CALL public.create_range_partitioned_shards('source_table_xyz', '{"(0,a)","(25,z)"}','{"(24,a)","(49,z)"}'); +SELECT * FROM pg_dist_shard WHERE logicalrelid='source_table_xyz'::regclass::oid; + logicalrelid | shardid | shardstorage | shardminvalue | shardmaxvalue +--------------------------------------------------------------------- + source_table_xyz | 4213581 | t | (0,a) | (24,a) + source_table_xyz | 4213582 | t | (25,z) | (49,z) +(2 rows) + +SELECT shardid, nodename, nodeport FROM pg_dist_shard_placement WHERE EXISTS(SELECT shardid FROM pg_dist_shard WHERE shardid=pg_dist_shard_placement.shardid AND logicalrelid='source_table_xyz'::regclass::oid); + shardid | nodename | nodeport +--------------------------------------------------------------------- + 4213581 | localhost | 57637 + 4213582 | localhost | 57638 +(2 rows) + +INSERT INTO source_table_xyz VALUES ((0, 'a'), 1, (0, 'a')), + ((1, 'b'), 2, (26, 'b')), + ((2, 'c'), 3, (3, 'c')), + ('(4,d)', 4, (27, 'd')), + ((30, 'e'), 5, (30, 'e')), + ((31, 'f'), 6, (31, 'f')), + ((32, 'g'), 7, (8, 'g')); +INSERT INTO source_table_xyz VALUES ((27, 'h'), 8, (26, 'b')); +SELECT * FROM source_table_xyz ORDER BY 1; + key | value | mapped_key +--------------------------------------------------------------------- + (0,a) | 1 | (0,a) + (1,b) | 2 | (26,b) + (2,c) | 3 | (3,c) + (4,d) | 4 | (27,d) + (27,h) | 8 | (26,b) + (30,e) | 5 | (30,e) + (31,f) | 6 | (31,f) + (32,g) | 7 | (8,g) +(8 rows) + +DROP TABLE source_table_xyz; +DROP TYPE composite_key_type; diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index b813aec5a..2ef3781b4 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -44,6 +44,7 @@ test: multi_create_table_constraints multi_master_protocol multi_load_data multi test: multi_behavioral_analytics_basics multi_behavioral_analytics_single_shard_queries multi_insert_select_non_pushable_queries multi_insert_select test: multi_insert_select_window multi_shard_update_delete window_functions dml_recursive recursive_dml_with_different_planners_executors test: multi_insert_select_conflict +test: multi_row_insert # --------- # at the end of the regression tests regaring recursively planned modifications diff --git a/src/test/regress/sql/multi_row_insert.sql b/src/test/regress/sql/multi_row_insert.sql new file mode 100644 index 000000000..aecf4f499 --- /dev/null +++ b/src/test/regress/sql/multi_row_insert.sql @@ -0,0 +1,29 @@ +-- Test multi row insert with composite type as partition key +-- https://github.com/citusdata/citus/issues/3357 + +SET citus.next_shard_id TO 4213581; +SET citus.shard_replication_factor TO 1; + +CREATE TYPE composite_key_type AS (f1 int, f2 text); +CREATE TABLE source_table_xyz(key composite_key_type, value int, mapped_key composite_key_type); +SELECT create_distributed_table('source_table_xyz', 'key', 'range'); +CALL public.create_range_partitioned_shards('source_table_xyz', '{"(0,a)","(25,z)"}','{"(24,a)","(49,z)"}'); + +SELECT * FROM pg_dist_shard WHERE logicalrelid='source_table_xyz'::regclass::oid; +SELECT shardid, nodename, nodeport FROM pg_dist_shard_placement WHERE EXISTS(SELECT shardid FROM pg_dist_shard WHERE shardid=pg_dist_shard_placement.shardid AND logicalrelid='source_table_xyz'::regclass::oid); + +INSERT INTO source_table_xyz VALUES ((0, 'a'), 1, (0, 'a')), + ((1, 'b'), 2, (26, 'b')), + ((2, 'c'), 3, (3, 'c')), + ('(4,d)', 4, (27, 'd')), + ((30, 'e'), 5, (30, 'e')), + ((31, 'f'), 6, (31, 'f')), + ((32, 'g'), 7, (8, 'g')); + +INSERT INTO source_table_xyz VALUES ((27, 'h'), 8, (26, 'b')); + + +SELECT * FROM source_table_xyz ORDER BY 1; + +DROP TABLE source_table_xyz; +DROP TYPE composite_key_type;