diff --git a/src/backend/distributed/planner/multi_router_planner.c b/src/backend/distributed/planner/multi_router_planner.c index 745f1449b..1d666b358 100644 --- a/src/backend/distributed/planner/multi_router_planner.c +++ b/src/backend/distributed/planner/multi_router_planner.c @@ -136,9 +136,12 @@ static DeferredErrorMessage * DeferErrorIfUnsupportedModifyQueryWithCitusLocalTa RTEListProperties *rteListProperties, Oid targetRelationId); static DeferredErrorMessage * DeferErrorIfUnsupportedModifyQueryWithPostgresLocalTable( RTEListProperties *rteListProperties, Oid targetRelationId); -static DeferredErrorMessage * MultiShardModifyQuerySupported(Query *originalQuery, - PlannerRestrictionContext * - plannerRestrictionContext); +static DeferredErrorMessage * MultiShardUpdateDeleteSupported(Query *originalQuery, + PlannerRestrictionContext * + plannerRestrictionContext); +static DeferredErrorMessage * SingleShardUpdateDeleteSupported(Query *originalQuery, + PlannerRestrictionContext * + plannerRestrictionContext); static bool HasDangerousJoinUsing(List *rtableList, Node *jtnode); static bool MasterIrreducibleExpression(Node *expression, bool *varArgument, bool *badCoalesce); @@ -1061,10 +1064,20 @@ ModifyQuerySupported(Query *queryTree, Query *originalQuery, bool multiShardQuer } } - if (commandType != CMD_INSERT && multiShardQuery) + if (commandType != CMD_INSERT) { - DeferredErrorMessage *errorMessage = MultiShardModifyQuerySupported( - originalQuery, plannerRestrictionContext); + DeferredErrorMessage *errorMessage = NULL; + + if (multiShardQuery) + { + errorMessage = MultiShardUpdateDeleteSupported(originalQuery, + plannerRestrictionContext); + } + else + { + errorMessage = SingleShardUpdateDeleteSupported(originalQuery, + plannerRestrictionContext); + } if (errorMessage != NULL) { @@ -1217,12 +1230,12 @@ ErrorIfOnConflictNotSupported(Query *queryTree) /* - * MultiShardModifyQuerySupported returns the error message if the modify query is + * MultiShardUpdateDeleteSupported returns the error message if the update/delete is * not pushdownable, otherwise it returns NULL. */ static DeferredErrorMessage * -MultiShardModifyQuerySupported(Query *originalQuery, - PlannerRestrictionContext *plannerRestrictionContext) +MultiShardUpdateDeleteSupported(Query *originalQuery, + PlannerRestrictionContext *plannerRestrictionContext) { DeferredErrorMessage *errorMessage = NULL; RangeTblEntry *resultRangeTable = ExtractResultRelationRTE(originalQuery); @@ -1261,6 +1274,35 @@ MultiShardModifyQuerySupported(Query *originalQuery, } +/* + * SingleShardUpdateDeleteSupported returns the error message if the update/delete query is + * not routable, otherwise it returns NULL. + */ +static DeferredErrorMessage * +SingleShardUpdateDeleteSupported(Query *originalQuery, + PlannerRestrictionContext *plannerRestrictionContext) +{ + DeferredErrorMessage *errorMessage = NULL; + + /* + * We currently do not support volatile functions in update/delete statements because + * the function evaluation logic does not know how to distinguish volatile functions + * (that need to be evaluated per row) from stable functions (that need to be evaluated per query), + * and it is also not safe to push the volatile functions down on replicated tables. + */ + if (FindNodeMatchingCheckFunction((Node *) originalQuery, + CitusIsVolatileFunction)) + { + errorMessage = DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, + "functions used in UPDATE queries on distributed " + "tables must not be VOLATILE", + NULL, NULL); + } + + return errorMessage; +} + + /* * HasDangerousJoinUsing search jointree for unnamed JOIN USING. Check the * implementation of has_dangerous_join_using in ruleutils. diff --git a/src/test/regress/expected/multi_router_planner.out b/src/test/regress/expected/multi_router_planner.out index d04677c8c..73d2f5495 100644 --- a/src/test/regress/expected/multi_router_planner.out +++ b/src/test/regress/expected/multi_router_planner.out @@ -2314,11 +2314,6 @@ DEBUG: query has a single distribution column value: 1 52 (7 rows) --- https://github.com/citusdata/citus/issues/3624 -UPDATE articles_hash SET id = id -WHERE author_id = 1 AND title IN (SELECT name FROM authors_reference WHERE random() > 0.5); -DEBUG: Creating router plan -DEBUG: query has a single distribution column value: 1 SET client_min_messages to 'NOTICE'; -- test that a connection failure marks placements invalid SET citus.shard_replication_factor TO 2; diff --git a/src/test/regress/expected/multi_shard_update_delete.out b/src/test/regress/expected/multi_shard_update_delete.out index 614b28887..f5d845597 100644 --- a/src/test/regress/expected/multi_shard_update_delete.out +++ b/src/test/regress/expected/multi_shard_update_delete.out @@ -704,6 +704,12 @@ SET value_2 = 5 * random() FROM events_test_table WHERE users_test_table.user_id = events_test_table.user_id; ERROR: functions used in UPDATE queries on distributed tables must not be VOLATILE +UPDATE users_test_table +SET value_1 = 3 +WHERE user_id = 1 AND value_1 IN (SELECT value_1 + FROM users_test_table + WHERE user_id = 1 AND value_2 > random()); +ERROR: functions used in UPDATE queries on distributed tables must not be VOLATILE -- Recursive modify planner does not take care of following test because the query -- is fully pushdownable, yet not allowed because it would lead to inconsistent replicas. UPDATE users_test_table diff --git a/src/test/regress/sql/multi_router_planner.sql b/src/test/regress/sql/multi_router_planner.sql index c55d02aab..0c507becf 100644 --- a/src/test/regress/sql/multi_router_planner.sql +++ b/src/test/regress/sql/multi_router_planner.sql @@ -1126,10 +1126,6 @@ SELECT id WHERE author_id = 1 ORDER BY 1; --- https://github.com/citusdata/citus/issues/3624 -UPDATE articles_hash SET id = id -WHERE author_id = 1 AND title IN (SELECT name FROM authors_reference WHERE random() > 0.5); - SET client_min_messages to 'NOTICE'; -- test that a connection failure marks placements invalid diff --git a/src/test/regress/sql/multi_shard_update_delete.sql b/src/test/regress/sql/multi_shard_update_delete.sql index cbf2e7408..c0cbdd791 100644 --- a/src/test/regress/sql/multi_shard_update_delete.sql +++ b/src/test/regress/sql/multi_shard_update_delete.sql @@ -579,6 +579,12 @@ SET value_2 = 5 * random() FROM events_test_table WHERE users_test_table.user_id = events_test_table.user_id; +UPDATE users_test_table +SET value_1 = 3 +WHERE user_id = 1 AND value_1 IN (SELECT value_1 + FROM users_test_table + WHERE user_id = 1 AND value_2 > random()); + -- Recursive modify planner does not take care of following test because the query -- is fully pushdownable, yet not allowed because it would lead to inconsistent replicas. UPDATE users_test_table