Merge pull request #1309 from citusdata/fix_sql_function_returns_wrong_result

Error out on parameterized SQL functions

src/backend/distributed/planner/multi_planner.c

@@ -126,6 +126,22 @@ multi_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
 	/* remove the context from the context list */
 	PopPlannerRestrictionContext();
 
+	/*
+	 * In some cases, for example; parameterized SQL functions, we may miss that
+	 * there is a need for distributed planning. Such cases only become clear after
+	 * standart_planner performs some modifications on parse tree. In such cases
+	 * we will simply error out.
+	 */
+	if (!needsDistributedPlanning && NeedsDistributedPlanning(parse))
+	{
+		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("cannot perform distributed planning on this "
+							   "query because parameterized queries for SQL "
+							   "functions referencing distributed tables are "
+							   "not supported"),
+						errhint("Consider using PL/pgSQL functions instead.")));
+	}
+
 	return result;
 }
 
@@ -334,7 +350,7 @@ CreateDistributedPlan(PlannedStmt *localPlan, Query *originalQuery, Query *query
 						  "could not create distributed plan",
 						  "Possibly this is caused by the use of parameters in SQL "
 						  "functions, which is not supported in Citus.",
-						  "Consider using PLPGSQL functions instead.");
+						  "Consider using PL/pgSQL functions instead.");
 	}
 
 	/*

src/test/regress/expected/multi_sql_function.out

@@ -34,15 +34,6 @@ CREATE FUNCTION sql_test_no_4() RETURNS bigint AS '
 		o_custkey = c_custkey AND
 		o_orderkey = l_orderkey;
 ' LANGUAGE SQL;
-CREATE FUNCTION sql_test_no_6(integer) RETURNS bigint AS  $$
-	SELECT
-		count(*)
-	FROM
-		orders, lineitem
-	WHERE
-		o_orderkey = l_orderkey AND
-		l_suppkey > $1;
-$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
 SET citus.task_executor_type TO 'task-tracker';
 SET client_min_messages TO INFO;
 -- now, run plain SQL functions
@@ -86,10 +77,6 @@ SELECT sql_test_no_2();
          11998
 (1 row)
 
--- plain SQL functions with parameters cannot be executed
--- FIXME: temporarily disabled, bad error message - waiting for proper
--- parametrized query support
--- SELECT sql_test_no_6(155);
 -- test router executor parameterized sql functions
 CREATE TABLE temp_table (
 	key int,
@@ -319,14 +306,38 @@ SELECT * FROM prepare_table ORDER BY key, value;
    6 |      
 (8 rows)
 
+-- test running parameterized SQL function
+CREATE TABLE test_parameterized_sql(id integer, org_id integer);
+select create_distributed_table('test_parameterized_sql','org_id');
+ create_distributed_table 
+--------------------------
+ 
+(1 row)
+
+CREATE OR REPLACE FUNCTION test_parameterized_sql_function(org_id_val integer)
+RETURNS TABLE (a bigint)
+AS $$
+    SELECT count(*) AS count_val from test_parameterized_sql where org_id = org_id_val;
+$$ LANGUAGE SQL STABLE;
+INSERT INTO test_parameterized_sql VALUES(1, 1);
+-- both of them should fail
+SELECT * FROM test_parameterized_sql_function(1);
+ERROR:  cannot perform distributed planning on this query because parameterized queries for SQL functions referencing distributed tables are not supported
+HINT:  Consider using PL/pgSQL functions instead.
+SELECT test_parameterized_sql_function(1);
+ERROR:  could not create distributed plan
+DETAIL:  Possibly this is caused by the use of parameters in SQL functions, which is not supported in Citus.
+HINT:  Consider using PL/pgSQL functions instead.
+CONTEXT:  SQL function "test_parameterized_sql_function" statement 1
 DROP TABLE temp_table;
+DROP TABLE test_parameterized_sql;
 -- clean-up functions
 DROP FUNCTION sql_test_no_1();
 DROP FUNCTION sql_test_no_2();
 DROP FUNCTION sql_test_no_3();
 DROP FUNCTION sql_test_no_4();
-DROP FUNCTION sql_test_no_6(int);
 DROP FUNCTION no_parameter_insert_sql();
 DROP FUNCTION non_partition_parameter_insert_sql(int);
 DROP FUNCTION non_partition_parameter_update_sql(int, int);
 DROP FUNCTION non_partition_parameter_delete_sql(int);
+DROP FUNCTION test_parameterized_sql_function(int);

src/test/regress/sql/multi_sql_function.sql

@@ -41,16 +41,6 @@ CREATE FUNCTION sql_test_no_4() RETURNS bigint AS '
 		o_orderkey = l_orderkey;
 ' LANGUAGE SQL;
 
-CREATE FUNCTION sql_test_no_6(integer) RETURNS bigint AS  $$
-	SELECT
-		count(*)
-	FROM
-		orders, lineitem
-	WHERE
-		o_orderkey = l_orderkey AND
-		l_suppkey > $1;
-$$ LANGUAGE SQL RETURNS NULL ON NULL INPUT;
-
 SET citus.task_executor_type TO 'task-tracker';
 SET client_min_messages TO INFO;
 
@@ -68,11 +58,6 @@ SET citus.task_executor_type TO 'real-time';
 SELECT sql_test_no_1();
 SELECT sql_test_no_2();
 
--- plain SQL functions with parameters cannot be executed
--- FIXME: temporarily disabled, bad error message - waiting for proper
--- parametrized query support
--- SELECT sql_test_no_6(155);
-
 -- test router executor parameterized sql functions
 CREATE TABLE temp_table (
 	key int,
@@ -140,15 +125,32 @@ SELECT non_partition_parameter_delete_sql(62);
 -- check after deletes
 SELECT * FROM prepare_table ORDER BY key, value;
 
+-- test running parameterized SQL function
+CREATE TABLE test_parameterized_sql(id integer, org_id integer);
+select create_distributed_table('test_parameterized_sql','org_id');
+
+CREATE OR REPLACE FUNCTION test_parameterized_sql_function(org_id_val integer)
+RETURNS TABLE (a bigint)
+AS $$
+    SELECT count(*) AS count_val from test_parameterized_sql where org_id = org_id_val;
+$$ LANGUAGE SQL STABLE;
+
+INSERT INTO test_parameterized_sql VALUES(1, 1);
+
+-- both of them should fail
+SELECT * FROM test_parameterized_sql_function(1);
+SELECT test_parameterized_sql_function(1);
+
 DROP TABLE temp_table;
+DROP TABLE test_parameterized_sql;
 
 -- clean-up functions
 DROP FUNCTION sql_test_no_1();
 DROP FUNCTION sql_test_no_2();
 DROP FUNCTION sql_test_no_3();
 DROP FUNCTION sql_test_no_4();
-DROP FUNCTION sql_test_no_6(int);
 DROP FUNCTION no_parameter_insert_sql();
 DROP FUNCTION non_partition_parameter_insert_sql(int);
 DROP FUNCTION non_partition_parameter_update_sql(int, int);
 DROP FUNCTION non_partition_parameter_delete_sql(int);
+DROP FUNCTION test_parameterized_sql_function(int);