diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index 7b310e5be..e34985834 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -634,17 +634,6 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, foreach(targetEntryCell, queryTree->targetList) { TargetEntry *targetEntry = (TargetEntry *) lfirst(targetEntryCell); - bool targetEntryPartitionColumn = false; - - /* reference tables do not have partition column */ - if (partitionColumn == NULL) - { - targetEntryPartitionColumn = false; - } - else if (targetEntry->resno == partitionColumn->varattno) - { - targetEntryPartitionColumn = true; - } /* skip resjunk entries: UPDATE adds some for ctid, etc. */ if (targetEntry->resjunk) @@ -652,6 +641,35 @@ ModifyPartialQuerySupported(Query *queryTree, bool multiShardQuery, continue; } + bool targetEntryPartitionColumn = false; + AttrNumber targetColumnAttrNumber = InvalidAttrNumber; + + /* reference tables do not have partition column */ + if (partitionColumn == NULL) + { + targetEntryPartitionColumn = false; + } + else + { + if (commandType == CMD_UPDATE) + { + /* + * Note that it is not possible to give an alias to + * UPDATE table SET ... + */ + if (targetEntry->resname) + { + targetColumnAttrNumber = get_attnum(resultRelationId, + targetEntry->resname); + if (targetColumnAttrNumber == partitionColumn->varattno) + { + targetEntryPartitionColumn = true; + } + } + } + } + + if (commandType == CMD_UPDATE && FindNodeMatchingCheckFunction((Node *) targetEntry->expr, CitusIsVolatileFunction)) @@ -1134,11 +1152,21 @@ ErrorIfOnConflictNotSupported(Query *queryTree) { setTargetEntryPartitionColumn = false; } - else if (setTargetEntry->resno == partitionColumn->varattno) + else { - setTargetEntryPartitionColumn = true; - } + Oid resultRelationId = ModifyQueryResultRelationId(queryTree); + AttrNumber targetColumnAttrNumber = InvalidAttrNumber; + if (setTargetEntry->resname) + { + targetColumnAttrNumber = get_attnum(resultRelationId, + setTargetEntry->resname); + if (targetColumnAttrNumber == partitionColumn->varattno) + { + setTargetEntryPartitionColumn = true; + } + } + } if (setTargetEntryPartitionColumn) { Expr *setExpr = setTargetEntry->expr; @@ -1522,12 +1550,7 @@ TargetEntryChangesValue(TargetEntry *targetEntry, Var *column, FromExpr *joinTre bool isColumnValueChanged = true; Expr *setExpr = targetEntry->expr; - if (targetEntry->resno != column->varattno) - { - /* target entry of the form SET some_other_col = */ - isColumnValueChanged = false; - } - else if (IsA(setExpr, Var)) + if (IsA(setExpr, Var)) { Var *newValue = (Var *) setExpr; if (newValue->varattno == column->varattno) diff --git a/src/test/regress/expected/modification_correctness.out b/src/test/regress/expected/modification_correctness.out new file mode 100644 index 000000000..d5380bcd9 --- /dev/null +++ b/src/test/regress/expected/modification_correctness.out @@ -0,0 +1,74 @@ +CREATE SCHEMA modification_correctness; +SET search_path to 'modification_correctness'; +CREATE TABLE test(k int, a int, b int, c int unique, d int, e int); +ALTER TABLE test DROP column k; +SELECT create_distributed_table('test', 'c'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +ALTER TABLE test DROP column b; +UPDATE test SET a = 5, c = 5; +ERROR: modifying the partition value of rows is not allowed +UPDATE test SET a = 5, c = d, d =3; +ERROR: modifying the partition value of rows is not allowed +UPDATE test SET c = d, d =3; +ERROR: modifying the partition value of rows is not allowed +UPDATE test SET d=c, c = d; +ERROR: modifying the partition value of rows is not allowed +UPDATE test SET e = c, d = 3; +UPDATE test SET c = c; +UPDATE test SET c = c, c = 3; +ERROR: multiple assignments to same column "c" +UPDATE test SET c = c, d = c; +UPDATE test SET c = c, d = 5, e = 3; +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=7; +ERROR: modifying the partition value of rows is not allowed +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET a=7; +INSERT INTO test (d,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=7; +ERROR: modifying the partition value of rows is not allowed +INSERT INTO test (d,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (a,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c) VALUES(3) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = 7; +-- make sure that without fast path planner, we don't get any unexpected errors. +SET citus.enable_fast_path_router_planner to false; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = 7; +UPDATE test SET e = c, d = 3; +UPDATE test SET c = c; +UPDATE test SET c = c, c = 3; +ERROR: multiple assignments to same column "c" +UPDATE test SET c = c, d = c; +UPDATE test SET c = c, d = 5, e = 3; +RESET citus.enable_fast_path_router_planner; +PREPARE foo(int,int) AS INSERT INTO test (c,a) VALUES($1,$2) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = $1; +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +PREPARE foo1(int, int) AS UPDATE test SET c = c, d = $1, e = $2; +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +-- we don't get an error because this is something like c = c +UPDATE test SET a = 5,d = 2, c = 5 FROM (SELECT * FROM test LIMIT 10) t2 WHERE t2.d = test.c and test.c = 5; +-- we should get an error because c gets 6 -> 5 +UPDATE test SET a = 5,d = 2, c = 5 FROM (SELECT * FROM test LIMIT 10) t2 WHERE t2.d = test.c and test.c = 6; +ERROR: modifying the partition value of rows is not allowed +DROP SCHEMA modification_correctness CASCADE; +NOTICE: drop cascades to table test diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 5ee743e87..87ce839c3 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -77,6 +77,7 @@ test: multi_agg_type_conversion multi_count_type_conversion recursive_relation_p test: multi_partition_pruning single_hash_repartition_join test: multi_join_pruning multi_hash_pruning intermediate_result_pruning test: multi_null_minmax_value_pruning cursors +test: modification_correctness test: multi_query_directory_cleanup test: multi_task_assignment_policy multi_cross_shard test: multi_utility_statements diff --git a/src/test/regress/sql/modification_correctness.sql b/src/test/regress/sql/modification_correctness.sql new file mode 100644 index 000000000..d28b28c63 --- /dev/null +++ b/src/test/regress/sql/modification_correctness.sql @@ -0,0 +1,71 @@ +CREATE SCHEMA modification_correctness; +SET search_path to 'modification_correctness'; + +CREATE TABLE test(k int, a int, b int, c int unique, d int, e int); +ALTER TABLE test DROP column k; +SELECT create_distributed_table('test', 'c'); +ALTER TABLE test DROP column b; + +UPDATE test SET a = 5, c = 5; +UPDATE test SET a = 5, c = d, d =3; +UPDATE test SET c = d, d =3; +UPDATE test SET d=c, c = d; +UPDATE test SET e = c, d = 3; +UPDATE test SET c = c; +UPDATE test SET c = c, c = 3; +UPDATE test SET c = c, d = c; +UPDATE test SET c = c, d = 5, e = 3; + + + +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=7; +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,d) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET a=7; +INSERT INTO test (d,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=7; +INSERT INTO test (d,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (a,c) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c) VALUES(3) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = 7; + +-- make sure that without fast path planner, we don't get any unexpected errors. +SET citus.enable_fast_path_router_planner to false; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET d=7; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = EXCLUDED.c; +INSERT INTO test (c,a) VALUES(3,4) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = 7; +UPDATE test SET e = c, d = 3; +UPDATE test SET c = c; +UPDATE test SET c = c, c = 3; +UPDATE test SET c = c, d = c; +UPDATE test SET c = c, d = 5, e = 3; +RESET citus.enable_fast_path_router_planner; + + +PREPARE foo(int,int) AS INSERT INTO test (c,a) VALUES($1,$2) ON CONFLICT(c) DO UPDATE SET c=EXCLUDED.c, d = EXCLUDED.c, e = $1; +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); +EXECUTE foo(1,2); + +PREPARE foo1(int, int) AS UPDATE test SET c = c, d = $1, e = $2; +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); +EXECUTE foo1(1,2); + +-- we don't get an error because this is something like c = c +UPDATE test SET a = 5,d = 2, c = 5 FROM (SELECT * FROM test LIMIT 10) t2 WHERE t2.d = test.c and test.c = 5; +-- we should get an error because c gets 6 -> 5 +UPDATE test SET a = 5,d = 2, c = 5 FROM (SELECT * FROM test LIMIT 10) t2 WHERE t2.d = test.c and test.c = 6; + + +DROP SCHEMA modification_correctness CASCADE;