diff --git a/src/backend/distributed/executor/multi_executor.c b/src/backend/distributed/executor/multi_executor.c index 9d2a8ea61..061f00619 100644 --- a/src/backend/distributed/executor/multi_executor.c +++ b/src/backend/distributed/executor/multi_executor.c @@ -52,6 +52,9 @@ multi_ExecutorStart(QueryDesc *queryDesc, int eflags) MultiExecutorType executorType = MULTI_EXECUTOR_INVALID_FIRST; Job *workerJob = multiPlan->workerJob; + /* ensure plan is executable */ + VerifyMultiPlanValidity(multiPlan); + ExecCheckRTPerms(planStatement->rtable, true); executorType = JobExecutorType(multiPlan); diff --git a/src/backend/distributed/planner/multi_explain.c b/src/backend/distributed/planner/multi_explain.c index fd4f59cef..46d410c90 100644 --- a/src/backend/distributed/planner/multi_explain.c +++ b/src/backend/distributed/planner/multi_explain.c @@ -167,6 +167,9 @@ MultiExplainOnePlan(PlannedStmt *plan, IntoClause *into, multiPlan = GetMultiPlan(plan); + /* ensure plan is executable */ + VerifyMultiPlanValidity(multiPlan); + if (!ExplainDistributedQueries) { appendStringInfo(es->str, "explain statements for distributed queries "); diff --git a/src/backend/distributed/planner/multi_planner.c b/src/backend/distributed/planner/multi_planner.c index 28a77fe77..ca8ea5769 100644 --- a/src/backend/distributed/planner/multi_planner.c +++ b/src/backend/distributed/planner/multi_planner.c @@ -25,6 +25,7 @@ #include "executor/executor.h" #include "nodes/makefuncs.h" +#include "nodes/nodeFuncs.h" #include "optimizer/planner.h" @@ -42,11 +43,13 @@ static PlannedStmt * MultiQueryContainerNode(PlannedStmt *result, static struct PlannedStmt * CreateDistributedPlan(PlannedStmt *localPlan, Query *originalQuery, Query *query, + ParamListInfo boundParams, RelationRestrictionContext * restrictionContext); static RelationRestrictionContext * CreateAndPushRestrictionContext(void); static RelationRestrictionContext * CurrentRestrictionContext(void); static void PopRestrictionContext(void); +static bool HasUnresolvedExternParamsWalker(Node *expression, ParamListInfo boundParams); /* Distributed planner hook */ @@ -103,7 +106,7 @@ multi_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) if (needsDistributedPlanning) { result = CreateDistributedPlan(result, originalQuery, parse, - restrictionContext); + boundParams, restrictionContext); } } PG_CATCH(); @@ -139,29 +142,49 @@ IsModifyCommand(Query *query) } +/* + * VerifyMultiPlanValidity verifies that multiPlan is ready for execution, or + * errors out if not. + * + * A plan may e.g. not be ready for execution because CreateDistributedPlan() + * couldn't find a plan due to unresolved prepared statement parameters, but + * didn't error out, because we expect custom plans to come to our rescue. + * But sql (not plpgsql) functions unfortunately don't go through a codepath + * supporting custom plans. + */ +void +VerifyMultiPlanValidity(MultiPlan *multiPlan) +{ + if (multiPlan->planningError) + { + RaiseDeferredError(multiPlan->planningError, ERROR); + } +} + + /* * CreateDistributedPlan encapsulates the logic needed to transform a particular * query into a distributed plan. */ static PlannedStmt * CreateDistributedPlan(PlannedStmt *localPlan, Query *originalQuery, Query *query, + ParamListInfo boundParams, RelationRestrictionContext *restrictionContext) { MultiPlan *distributedPlan = NULL; + PlannedStmt *resultPlan = NULL; + bool hasUnresolvedParams = false; + + if (HasUnresolvedExternParamsWalker((Node *) query, boundParams)) + { + hasUnresolvedParams = true; + } if (IsModifyCommand(query)) { - /* - * Modifications are always routed through the same - * planner/executor. As there's currently no other way to plan these, - * error out if the query is unsupported. - */ + /* modifications are always routed through the same planner/executor */ distributedPlan = CreateModifyPlan(originalQuery, query, restrictionContext); Assert(distributedPlan); - if (distributedPlan->planningError) - { - RaiseDeferredError(distributedPlan->planningError, ERROR); - } } else { @@ -182,8 +205,13 @@ CreateDistributedPlan(PlannedStmt *localPlan, Query *originalQuery, Query *query } } - /* router didn't yield a plan, try the full distributed planner */ - if (!distributedPlan || distributedPlan->planningError) + /* + * Router didn't yield a plan, try the full distributed planner. As + * real-time/task-tracker don't support prepared statement parameters, + * skip planning in that case (we'll later trigger an error in that + * case if necessary). + */ + if ((!distributedPlan || distributedPlan->planningError) && !hasUnresolvedParams) { /* Create and optimize logical plan */ MultiTreeRoot *logicalPlan = MultiLogicalPlanCreate(query); @@ -206,8 +234,62 @@ CreateDistributedPlan(PlannedStmt *localPlan, Query *originalQuery, Query *query } } + /* + * If no plan was generated, prepare a generic error to be emitted. + * Normally this error message will never returned to the user, as it's + * usually due to unresolved prepared statement parameters - in that case + * the logic below will force a custom plan (i.e. with parameters bound to + * specific values) to be generated. But sql (not plpgsql) functions + * unfortunately don't go through a codepath supporting custom plans - so + * we still need to have an error prepared. + */ + if (!distributedPlan) + { + /* currently always should have a more specific error otherwise */ + Assert(hasUnresolvedParams); + distributedPlan = CitusMakeNode(MultiPlan); + distributedPlan->planningError = + DeferredError(ERRCODE_FEATURE_NOT_SUPPORTED, + "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."); + } + + /* + * Error out if none of the planners resulted in a usable plan, unless the + * error was possibly triggered by missing parameters. In that case we'll + * not error out here, but instead rely on postgres' custom plan logic. + * Postgres re-plans prepared statements the first five executions + * (i.e. it produces custom plans), after that the cost of a generic plan + * is compared with the average custom plan cost. We support otherwise + * unsupported prepared statement parameters by assigning an exorbitant + * cost to the unsupported query. That'll lead to the custom plan being + * chosen. But for that to be possible we can't error out here, as + * otherwise that logic is never reached. + */ + if (distributedPlan->planningError && !hasUnresolvedParams) + { + RaiseDeferredError(distributedPlan->planningError, ERROR); + } + /* store required data into the planned statement */ - return MultiQueryContainerNode(localPlan, distributedPlan); + resultPlan = MultiQueryContainerNode(localPlan, distributedPlan); + + /* + * As explained above, force planning costs to be unrealistically high if + * query planning failed (possibly) due to prepared statement parameters. + */ + if (distributedPlan->planningError && hasUnresolvedParams) + { + /* + * Arbitraryly high cost, but low enough that it can be added up + * without overflowing by choose_custom_plan(). + */ + resultPlan->planTree->total_cost = FLT_MAX / 100000000; + } + + return resultPlan; } @@ -502,3 +584,70 @@ PopRestrictionContext(void) { relationRestrictionContextList = list_delete_first(relationRestrictionContextList); } + + +/* + * HasUnresolvedExternParamsWalker returns true if the passed in expression + * has external parameters that are not contained in boundParams, false + * otherwise. + */ +static bool +HasUnresolvedExternParamsWalker(Node *expression, ParamListInfo boundParams) +{ + if (expression == NULL) + { + return false; + } + + if (IsA(expression, Param)) + { + Param *param = (Param *) expression; + int paramId = param->paramid; + + /* only care about user supplied parameters */ + if (param->paramkind != PARAM_EXTERN) + { + return false; + } + + /* don't care about our special parameter, it'll be removed during planning */ + if (paramId == UNINSTANTIATED_PARAMETER_ID) + { + return false; + } + + /* check whether parameter is available (and valid) */ + if (boundParams && paramId > 0 && paramId <= boundParams->numParams) + { + ParamExternData *externParam = &boundParams->params[paramId - 1]; + + /* give hook a chance in case parameter is dynamic */ + if (!OidIsValid(externParam->ptype) && boundParams->paramFetch != NULL) + { + (*boundParams->paramFetch)(boundParams, paramId); + } + + if (OidIsValid(externParam->ptype)) + { + return false; + } + } + + return true; + } + + /* keep traversing */ + if (IsA(expression, Query)) + { + return query_tree_walker((Query *) expression, + HasUnresolvedExternParamsWalker, + boundParams, + 0); + } + else + { + return expression_tree_walker(expression, + HasUnresolvedExternParamsWalker, + boundParams); + } +} diff --git a/src/include/distributed/multi_planner.h b/src/include/distributed/multi_planner.h index baee8a064..1c1d7a337 100644 --- a/src/include/distributed/multi_planner.h +++ b/src/include/distributed/multi_planner.h @@ -57,5 +57,6 @@ extern struct MultiPlan * GetMultiPlan(PlannedStmt *planStatement); extern void multi_relation_restriction_hook(PlannerInfo *root, RelOptInfo *relOptInfo, Index index, RangeTblEntry *rte); extern bool IsModifyCommand(Query *query); +extern void VerifyMultiPlanValidity(struct MultiPlan *multiPlan); #endif /* MULTI_PLANNER_H */ diff --git a/src/test/regress/expected/multi_explain.out b/src/test/regress/expected/multi_explain.out index a71965210..bafe813bb 100644 --- a/src/test/regress/expected/multi_explain.out +++ b/src/test/regress/expected/multi_explain.out @@ -727,3 +727,10 @@ Distributed Query into pg_merge_job_570039 Master Query -> Aggregate -> Seq Scan on pg_merge_job_570039 +-- EXPLAIN EXECUTE of parametrized prepared statements is broken, but +-- at least make sure to fail without crashing +PREPARE router_executor_query_param(int) AS SELECT l_quantity FROM lineitem WHERE l_orderkey = $1; +EXPLAIN EXECUTE router_executor_query_param(5); +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 PLPGSQL functions instead. diff --git a/src/test/regress/expected/multi_explain_0.out b/src/test/regress/expected/multi_explain_0.out index 9e414231e..7425b80e3 100644 --- a/src/test/regress/expected/multi_explain_0.out +++ b/src/test/regress/expected/multi_explain_0.out @@ -698,3 +698,10 @@ Distributed Query into pg_merge_job_570039 Master Query -> Aggregate -> Seq Scan on pg_merge_job_570039 +-- EXPLAIN EXECUTE of parametrized prepared statements is broken, but +-- at least make sure to fail without crashing +PREPARE router_executor_query_param(int) AS SELECT l_quantity FROM lineitem WHERE l_orderkey = $1; +EXPLAIN EXECUTE router_executor_query_param(5); +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 PLPGSQL functions instead. diff --git a/src/test/regress/expected/multi_prepare_plsql.out b/src/test/regress/expected/multi_prepare_plsql.out index 3490beed7..837ab4c2d 100644 --- a/src/test/regress/expected/multi_prepare_plsql.out +++ b/src/test/regress/expected/multi_prepare_plsql.out @@ -265,9 +265,18 @@ SELECT plpgsql_test_2(); (1 row) -- run PL/pgsql functions with different parameters --- FIXME: temporarily disabled, waiting for proper parametrized query support --- SELECT plpgsql_test_6(155); --- SELECT plpgsql_test_6(1555); +SELECT plpgsql_test_6(155); + plpgsql_test_6 +---------------- + 11811 +(1 row) + +SELECT plpgsql_test_6(1555); + plpgsql_test_6 +---------------- + 10183 +(1 row) + -- test router executor parameterized PL/pgsql functions CREATE TABLE plpgsql_table ( key int, @@ -365,9 +374,11 @@ SELECT single_parameter_insert(5); (1 row) SELECT single_parameter_insert(6); -ERROR: values given for the partition column must be constants or constant expressions -CONTEXT: SQL statement "INSERT INTO plpgsql_table (key) VALUES (key_arg)" -PL/pgSQL function single_parameter_insert(integer) line 3 at SQL statement + single_parameter_insert +------------------------- + +(1 row) + CREATE FUNCTION double_parameter_insert(key_arg int, value_arg int) RETURNS void as $$ BEGIN @@ -406,9 +417,11 @@ SELECT double_parameter_insert(5, 50); (1 row) SELECT double_parameter_insert(6, 60); -ERROR: values given for the partition column must be constants or constant expressions -CONTEXT: SQL statement "INSERT INTO plpgsql_table (key, value) VALUES (key_arg, value_arg)" -PL/pgSQL function double_parameter_insert(integer,integer) line 3 at SQL statement + double_parameter_insert +------------------------- + +(1 row) + CREATE FUNCTION non_partition_parameter_insert(value_arg int) RETURNS void as $$ BEGIN @@ -478,7 +491,9 @@ SELECT * FROM plpgsql_table ORDER BY key, value; 4 | 5 | 50 5 | -(22 rows) + 6 | 60 + 6 | +(24 rows) -- check router executor select CREATE FUNCTION router_partition_column_select(key_arg int) @@ -498,6 +513,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT router_partition_column_select(1); router_partition_column_select -------------------------------- @@ -533,9 +549,13 @@ SELECT router_partition_column_select(5); (5,) (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT router_partition_column_select(6); +SELECT router_partition_column_select(6); + router_partition_column_select +-------------------------------- + (6,60) + (6,) +(2 rows) + CREATE FUNCTION router_non_partition_column_select(value_arg int) RETURNS TABLE(key int, value int) AS $$ DECLARE @@ -554,6 +574,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT router_non_partition_column_select(10); router_non_partition_column_select ------------------------------------ @@ -608,6 +629,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT real_time_non_partition_column_select(10); real_time_non_partition_column_select --------------------------------------- @@ -643,9 +665,13 @@ SELECT real_time_non_partition_column_select(50); (5,50) (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_non_partition_column_select(60); +SELECT real_time_non_partition_column_select(60); + real_time_non_partition_column_select +--------------------------------------- + (0,60) + (6,60) +(2 rows) + CREATE FUNCTION real_time_partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ DECLARE @@ -664,6 +690,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT real_time_partition_column_select(1); real_time_partition_column_select ----------------------------------- @@ -708,9 +735,15 @@ SELECT real_time_partition_column_select(5); (5,) (4 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_partition_column_select(6); +SELECT real_time_partition_column_select(6); + real_time_partition_column_select +----------------------------------- + (0,10) + (1,10) + (6,60) + (6,) +(4 rows) + -- check task-tracker executor SET citus.task_executor_type TO 'task-tracker'; CREATE FUNCTION task_tracker_non_partition_column_select(value_arg int) @@ -730,6 +763,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT task_tracker_non_partition_column_select(10); task_tracker_non_partition_column_select ------------------------------------------ @@ -765,9 +799,13 @@ SELECT task_tracker_non_partition_column_select(50); (5,50) (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_non_partition_column_select(60); +SELECT real_time_non_partition_column_select(60); + real_time_non_partition_column_select +--------------------------------------- + (0,60) + (6,60) +(2 rows) + CREATE FUNCTION task_tracker_partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ DECLARE @@ -786,6 +824,7 @@ BEGIN value; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT task_tracker_partition_column_select(1); task_tracker_partition_column_select -------------------------------------- @@ -830,9 +869,15 @@ SELECT task_tracker_partition_column_select(5); (5,) (4 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT task_tracker_partition_column_select(6); +SELECT task_tracker_partition_column_select(6); + task_tracker_partition_column_select +-------------------------------------- + (0,10) + (1,10) + (6,60) + (6,) +(4 rows) + SET citus.task_executor_type TO 'real-time'; -- check updates CREATE FUNCTION partition_parameter_update(int, int) RETURNS void as $$ @@ -871,8 +916,7 @@ SELECT partition_parameter_update(5, 51); (1 row) --- This fails with an unexpected error message -SELECT partition_parameter_update(5, 52); +SELECT partition_parameter_update(6, 61); ERROR: distributed modifications must target exactly one shard DETAIL: This command modifies all shards. HINT: Consider using an equality filter on partition column "key". You can use master_modify_multiple_shards() to perform multi-shard delete or update operations. @@ -946,7 +990,9 @@ SELECT * FROM plpgsql_table ORDER BY key, value; 4 | 41 5 | 51 5 | 51 -(22 rows) + 6 | 60 + 6 | +(24 rows) -- check deletes CREATE FUNCTION partition_parameter_delete(int, int) RETURNS void as $$ @@ -954,6 +1000,7 @@ BEGIN DELETE FROM plpgsql_table WHERE key = $1 AND value = $2; END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT partition_parameter_delete(1, 11); partition_parameter_delete ---------------------------- @@ -984,8 +1031,7 @@ SELECT partition_parameter_delete(5, 51); (1 row) --- This fails with an unexpected error message -SELECT partition_parameter_delete(0, 10); +SELECT partition_parameter_delete(6, 61); ERROR: distributed modifications must target exactly one shard DETAIL: This command modifies all shards. HINT: Consider using an equality filter on partition column "key". You can use master_modify_multiple_shards() to perform multi-shard delete or update operations. @@ -1043,7 +1089,9 @@ SELECT * FROM plpgsql_table ORDER BY key, value; 0 | 0 | 0 | -(6 rows) + 6 | 60 + 6 | +(8 rows) -- check whether we can handle execute parameters CREATE TABLE execute_parameter_test (key int, val date); diff --git a/src/test/regress/expected/multi_prepare_sql.out b/src/test/regress/expected/multi_prepare_sql.out index 5bac00df5..8f7e7424d 100644 --- a/src/test/regress/expected/multi_prepare_sql.out +++ b/src/test/regress/expected/multi_prepare_sql.out @@ -240,8 +240,12 @@ EXECUTE prepared_test_6(155); 11811 (1 row) --- FIXME: temporarily disabled --- EXECUTE prepared_test_6(1555); +EXECUTE prepared_test_6(1555); + count +------- + 10183 +(1 row) + -- test router executor with parameterized non-partition columns -- create a custom type which also exists on worker nodes CREATE TYPE test_composite_type AS ( @@ -325,17 +329,16 @@ EXECUTE prepared_select(6, 60); 1 (1 row) --- test that we don't crash on failing parameterized insert on the partition column +-- Test that parameterized partition column for an insert is supported PREPARE prepared_partition_column_insert(bigint) AS INSERT INTO router_executor_table VALUES ($1, 'arsenous', '(1,10)'); --- we error out on the 6th execution +-- execute 6 times to trigger prepared statement usage EXECUTE prepared_partition_column_insert(1); EXECUTE prepared_partition_column_insert(2); EXECUTE prepared_partition_column_insert(3); EXECUTE prepared_partition_column_insert(4); EXECUTE prepared_partition_column_insert(5); EXECUTE prepared_partition_column_insert(6); -ERROR: values given for the partition column must be constants or constant expressions DROP TYPE test_composite_type CASCADE; NOTICE: drop cascades to table router_executor_table column stats -- test router executor with prepare statements @@ -373,7 +376,6 @@ EXECUTE prepared_single_parameter_insert(3); EXECUTE prepared_single_parameter_insert(4); EXECUTE prepared_single_parameter_insert(5); EXECUTE prepared_single_parameter_insert(6); -ERROR: values given for the partition column must be constants or constant expressions PREPARE prepared_double_parameter_insert(int, int) AS INSERT INTO prepare_table (key, value) VALUES ($1, $2); -- execute 6 times to trigger prepared statement usage @@ -383,7 +385,6 @@ EXECUTE prepared_double_parameter_insert(3, 30); EXECUTE prepared_double_parameter_insert(4, 40); EXECUTE prepared_double_parameter_insert(5, 50); EXECUTE prepared_double_parameter_insert(6, 60); -ERROR: values given for the partition column must be constants or constant expressions PREPARE prepared_non_partition_parameter_insert(int) AS INSERT INTO prepare_table (key, value) VALUES (0, $1); -- execute 6 times to trigger prepared statement usage @@ -419,7 +420,9 @@ SELECT * FROM prepare_table ORDER BY key, value; 4 | 5 | 50 5 | -(22 rows) + 6 | 60 + 6 | +(24 rows) -- check router executor select PREPARE prepared_router_partition_column_select(int) AS @@ -468,9 +471,13 @@ EXECUTE prepared_router_partition_column_select(5); 5 | (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_router_partition_column_select(6); +EXECUTE prepared_router_partition_column_select(6); + key | value +-----+------- + 6 | 60 + 6 | +(2 rows) + PREPARE prepared_router_non_partition_column_select(int) AS SELECT prepare_table.key, @@ -566,9 +573,13 @@ EXECUTE prepared_real_time_non_partition_column_select(50); 5 | 50 (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_real_time_non_partition_column_select(60); +EXECUTE prepared_real_time_non_partition_column_select(60); + key | value +-----+------- + 0 | 60 + 6 | 60 +(2 rows) + PREPARE prepared_real_time_partition_column_select(int) AS SELECT prepare_table.key, @@ -625,9 +636,15 @@ EXECUTE prepared_real_time_partition_column_select(5); 5 | (4 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_real_time_partition_column_select(6); +EXECUTE prepared_real_time_partition_column_select(6); + key | value +-----+------- + 0 | 10 + 1 | 10 + 6 | 60 + 6 | +(4 rows) + -- check task-tracker executor SET citus.task_executor_type TO 'task-tracker'; PREPARE prepared_task_tracker_non_partition_column_select(int) AS @@ -676,9 +693,13 @@ EXECUTE prepared_task_tracker_non_partition_column_select(50); 5 | 50 (2 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_task_tracker_non_partition_column_select(60); +EXECUTE prepared_task_tracker_non_partition_column_select(60); + key | value +-----+------- + 0 | 60 + 6 | 60 +(2 rows) + PREPARE prepared_task_tracker_partition_column_select(int) AS SELECT prepare_table.key, @@ -735,9 +756,15 @@ EXECUTE prepared_task_tracker_partition_column_select(5); 5 | (4 rows) --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_task_tracker_partition_column_select(6); +EXECUTE prepared_task_tracker_partition_column_select(6); + key | value +-----+------- + 0 | 10 + 1 | 10 + 6 | 60 + 6 | +(4 rows) + SET citus.task_executor_type TO 'real-time'; -- check updates PREPARE prepared_partition_parameter_update(int, int) AS @@ -748,8 +775,7 @@ EXECUTE prepared_partition_parameter_update(2, 21); EXECUTE prepared_partition_parameter_update(3, 31); EXECUTE prepared_partition_parameter_update(4, 41); EXECUTE prepared_partition_parameter_update(5, 51); --- This fails with an unexpected error message -EXECUTE prepared_partition_parameter_update(5, 52); +EXECUTE prepared_partition_parameter_update(6, 61); ERROR: distributed modifications must target exactly one shard DETAIL: This command modifies all shards. HINT: Consider using an equality filter on partition column "key". You can use master_modify_multiple_shards() to perform multi-shard delete or update operations. @@ -788,7 +814,9 @@ SELECT * FROM prepare_table ORDER BY key, value; 4 | 41 5 | 51 5 | 51 -(22 rows) + 6 | 60 + 6 | +(24 rows) -- check deletes PREPARE prepared_partition_parameter_delete(int, int) AS @@ -798,8 +826,7 @@ EXECUTE prepared_partition_parameter_delete(2, 21); EXECUTE prepared_partition_parameter_delete(3, 31); EXECUTE prepared_partition_parameter_delete(4, 41); EXECUTE prepared_partition_parameter_delete(5, 51); --- This fails with an unexpected error message -EXECUTE prepared_partition_parameter_delete(0, 10); +EXECUTE prepared_partition_parameter_delete(6, 61); ERROR: distributed modifications must target exactly one shard DETAIL: This command modifies all shards. HINT: Consider using an equality filter on partition column "key". You can use master_modify_multiple_shards() to perform multi-shard delete or update operations. @@ -822,7 +849,9 @@ SELECT * FROM prepare_table ORDER BY key, value; 0 | 0 | 0 | -(6 rows) + 6 | 60 + 6 | +(8 rows) -- verify placement state updates invalidate shard state -- diff --git a/src/test/regress/expected/multi_sql_function.out b/src/test/regress/expected/multi_sql_function.out index e6882798d..5fea10339 100644 --- a/src/test/regress/expected/multi_sql_function.out +++ b/src/test/regress/expected/multi_sql_function.out @@ -315,7 +315,9 @@ SELECT * FROM prepare_table ORDER BY key, value; 0 | 0 | 0 | -(6 rows) + 6 | 60 + 6 | +(8 rows) DROP TABLE temp_table; -- clean-up functions diff --git a/src/test/regress/sql/multi_explain.sql b/src/test/regress/sql/multi_explain.sql index 9c445596c..c7874a38a 100644 --- a/src/test/regress/sql/multi_explain.sql +++ b/src/test/regress/sql/multi_explain.sql @@ -227,3 +227,8 @@ EXPLAIN EXECUTE router_executor_query; PREPARE real_time_executor_query AS SELECT avg(l_linenumber) FROM lineitem WHERE l_orderkey > 9030; EXPLAIN (COSTS FALSE) EXECUTE real_time_executor_query; + +-- EXPLAIN EXECUTE of parametrized prepared statements is broken, but +-- at least make sure to fail without crashing +PREPARE router_executor_query_param(int) AS SELECT l_quantity FROM lineitem WHERE l_orderkey = $1; +EXPLAIN EXECUTE router_executor_query_param(5); diff --git a/src/test/regress/sql/multi_prepare_plsql.sql b/src/test/regress/sql/multi_prepare_plsql.sql index cb1bab581..5018c2cb8 100644 --- a/src/test/regress/sql/multi_prepare_plsql.sql +++ b/src/test/regress/sql/multi_prepare_plsql.sql @@ -183,9 +183,8 @@ SELECT plpgsql_test_1(); SELECT plpgsql_test_2(); -- run PL/pgsql functions with different parameters --- FIXME: temporarily disabled, waiting for proper parametrized query support --- SELECT plpgsql_test_6(155); --- SELECT plpgsql_test_6(1555); +SELECT plpgsql_test_6(155); +SELECT plpgsql_test_6(1555); -- test router executor parameterized PL/pgsql functions CREATE TABLE plpgsql_table ( @@ -276,15 +275,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT router_partition_column_select(1); SELECT router_partition_column_select(2); SELECT router_partition_column_select(3); SELECT router_partition_column_select(4); SELECT router_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT router_partition_column_select(6); +SELECT router_partition_column_select(6); CREATE FUNCTION router_non_partition_column_select(value_arg int) RETURNS TABLE(key int, value int) AS $$ @@ -305,6 +302,7 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT router_non_partition_column_select(10); SELECT router_non_partition_column_select(20); SELECT router_non_partition_column_select(30); @@ -331,15 +329,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT real_time_non_partition_column_select(10); SELECT real_time_non_partition_column_select(20); SELECT real_time_non_partition_column_select(30); SELECT real_time_non_partition_column_select(40); SELECT real_time_non_partition_column_select(50); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_non_partition_column_select(60); +SELECT real_time_non_partition_column_select(60); CREATE FUNCTION real_time_partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ @@ -360,15 +356,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT real_time_partition_column_select(1); SELECT real_time_partition_column_select(2); SELECT real_time_partition_column_select(3); SELECT real_time_partition_column_select(4); SELECT real_time_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_partition_column_select(6); +SELECT real_time_partition_column_select(6); -- check task-tracker executor SET citus.task_executor_type TO 'task-tracker'; @@ -391,15 +385,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT task_tracker_non_partition_column_select(10); SELECT task_tracker_non_partition_column_select(20); SELECT task_tracker_non_partition_column_select(30); SELECT task_tracker_non_partition_column_select(40); SELECT task_tracker_non_partition_column_select(50); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT real_time_non_partition_column_select(60); +SELECT real_time_non_partition_column_select(60); CREATE FUNCTION task_tracker_partition_column_select(key_arg int) RETURNS TABLE(key int, value int) AS $$ @@ -420,15 +412,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT task_tracker_partition_column_select(1); SELECT task_tracker_partition_column_select(2); SELECT task_tracker_partition_column_select(3); SELECT task_tracker_partition_column_select(4); SELECT task_tracker_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- SELECT task_tracker_partition_column_select(6); +SELECT task_tracker_partition_column_select(6); SET citus.task_executor_type TO 'real-time'; @@ -445,8 +435,7 @@ SELECT partition_parameter_update(2, 21); SELECT partition_parameter_update(3, 31); SELECT partition_parameter_update(4, 41); SELECT partition_parameter_update(5, 51); --- This fails with an unexpected error message -SELECT partition_parameter_update(5, 52); +SELECT partition_parameter_update(6, 61); CREATE FUNCTION non_partition_parameter_update(int, int) RETURNS void as $$ BEGIN @@ -472,13 +461,13 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- execute 6 times to trigger prepared statement usage SELECT partition_parameter_delete(1, 11); SELECT partition_parameter_delete(2, 21); SELECT partition_parameter_delete(3, 31); SELECT partition_parameter_delete(4, 41); SELECT partition_parameter_delete(5, 51); --- This fails with an unexpected error message -SELECT partition_parameter_delete(0, 10); +SELECT partition_parameter_delete(6, 61); CREATE FUNCTION non_partition_parameter_delete(int) RETURNS void as $$ BEGIN diff --git a/src/test/regress/sql/multi_prepare_sql.sql b/src/test/regress/sql/multi_prepare_sql.sql index 8618b8f76..d35c5d936 100644 --- a/src/test/regress/sql/multi_prepare_sql.sql +++ b/src/test/regress/sql/multi_prepare_sql.sql @@ -149,8 +149,7 @@ EXECUTE prepared_test_2; -- execute prepared statements with different parameters EXECUTE prepared_test_6(155); --- FIXME: temporarily disabled --- EXECUTE prepared_test_6(1555); +EXECUTE prepared_test_6(1555); -- test router executor with parameterized non-partition columns @@ -194,12 +193,12 @@ EXECUTE prepared_select(4, 40); EXECUTE prepared_select(5, 50); EXECUTE prepared_select(6, 60); --- test that we don't crash on failing parameterized insert on the partition column +-- Test that parameterized partition column for an insert is supported PREPARE prepared_partition_column_insert(bigint) AS INSERT INTO router_executor_table VALUES ($1, 'arsenous', '(1,10)'); --- we error out on the 6th execution +-- execute 6 times to trigger prepared statement usage EXECUTE prepared_partition_column_insert(1); EXECUTE prepared_partition_column_insert(2); EXECUTE prepared_partition_column_insert(3); @@ -282,10 +281,7 @@ EXECUTE prepared_router_partition_column_select(2); EXECUTE prepared_router_partition_column_select(3); EXECUTE prepared_router_partition_column_select(4); EXECUTE prepared_router_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_router_partition_column_select(6); +EXECUTE prepared_router_partition_column_select(6); PREPARE prepared_router_non_partition_column_select(int) AS SELECT @@ -325,10 +321,7 @@ EXECUTE prepared_real_time_non_partition_column_select(20); EXECUTE prepared_real_time_non_partition_column_select(30); EXECUTE prepared_real_time_non_partition_column_select(40); EXECUTE prepared_real_time_non_partition_column_select(50); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_real_time_non_partition_column_select(60); +EXECUTE prepared_real_time_non_partition_column_select(60); PREPARE prepared_real_time_partition_column_select(int) AS SELECT @@ -348,10 +341,7 @@ EXECUTE prepared_real_time_partition_column_select(2); EXECUTE prepared_real_time_partition_column_select(3); EXECUTE prepared_real_time_partition_column_select(4); EXECUTE prepared_real_time_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_real_time_partition_column_select(6); +EXECUTE prepared_real_time_partition_column_select(6); -- check task-tracker executor SET citus.task_executor_type TO 'task-tracker'; @@ -373,10 +363,7 @@ EXECUTE prepared_task_tracker_non_partition_column_select(20); EXECUTE prepared_task_tracker_non_partition_column_select(30); EXECUTE prepared_task_tracker_non_partition_column_select(40); EXECUTE prepared_task_tracker_non_partition_column_select(50); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_task_tracker_non_partition_column_select(60); +EXECUTE prepared_task_tracker_non_partition_column_select(60); PREPARE prepared_task_tracker_partition_column_select(int) AS SELECT @@ -396,10 +383,7 @@ EXECUTE prepared_task_tracker_partition_column_select(2); EXECUTE prepared_task_tracker_partition_column_select(3); EXECUTE prepared_task_tracker_partition_column_select(4); EXECUTE prepared_task_tracker_partition_column_select(5); - --- FIXME: 6th execution is failing. We don't want to run the failing test --- because of changing output. After implementing this feature, uncomment this. --- EXECUTE prepared_task_tracker_partition_column_select(6); +EXECUTE prepared_task_tracker_partition_column_select(6); SET citus.task_executor_type TO 'real-time'; @@ -413,8 +397,7 @@ EXECUTE prepared_partition_parameter_update(2, 21); EXECUTE prepared_partition_parameter_update(3, 31); EXECUTE prepared_partition_parameter_update(4, 41); EXECUTE prepared_partition_parameter_update(5, 51); --- This fails with an unexpected error message -EXECUTE prepared_partition_parameter_update(5, 52); +EXECUTE prepared_partition_parameter_update(6, 61); PREPARE prepared_non_partition_parameter_update(int, int) AS UPDATE prepare_table SET value = $2 WHERE key = 0 AND value = $1; @@ -439,8 +422,7 @@ EXECUTE prepared_partition_parameter_delete(2, 21); EXECUTE prepared_partition_parameter_delete(3, 31); EXECUTE prepared_partition_parameter_delete(4, 41); EXECUTE prepared_partition_parameter_delete(5, 51); --- This fails with an unexpected error message -EXECUTE prepared_partition_parameter_delete(0, 10); +EXECUTE prepared_partition_parameter_delete(6, 61); PREPARE prepared_non_partition_parameter_delete(int) AS DELETE FROM prepare_table WHERE key = 0 AND value = $1;