Disallow volatile functions on single shard update queries

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.

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.

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;

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

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

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