From e56fc344045d971d1aa4a4cd7e845d11753a0a55 Mon Sep 17 00:00:00 2001 From: Teja Mupparti Date: Mon, 4 Apr 2022 19:32:20 -0700 Subject: [PATCH] Fixes: #5787 In prepared statements, map any unused parameters to a generic type. --- .../distributed/executor/adaptive_executor.c | 14 +++ src/backend/distributed/utils/param_utils.c | 88 +++++++++++++++++++ src/include/distributed/param_utils.h | 15 ++++ src/test/regress/expected/with_prepare.out | 54 ++++++++++++ src/test/regress/sql/with_prepare.sql | 22 +++++ 5 files changed, 193 insertions(+) create mode 100644 src/backend/distributed/utils/param_utils.c create mode 100644 src/include/distributed/param_utils.h diff --git a/src/backend/distributed/executor/adaptive_executor.c b/src/backend/distributed/executor/adaptive_executor.c index 347bd4d35..32198255c 100644 --- a/src/backend/distributed/executor/adaptive_executor.c +++ b/src/backend/distributed/executor/adaptive_executor.c @@ -152,6 +152,7 @@ #include "distributed/multi_partitioning_utils.h" #include "distributed/multi_physical_planner.h" #include "distributed/multi_server_executor.h" +#include "distributed/param_utils.h" #include "distributed/placement_access.h" #include "distributed/placement_connection.h" #include "distributed/relation_access_tracking.h" @@ -830,6 +831,19 @@ AdaptiveExecutor(CitusScanState *scanState) distributedPlan->modLevel, taskList, excludeFromXact); bool localExecutionSupported = true; + + /* + * In some rare cases, we have prepared statements that pass a parameter + * and never used in the query, mark such parameters' type as Invalid(0), + * which will be used later in ExtractParametersFromParamList() to map them + * to a generic datatype. Skip for dynamic parameters. + */ + if (paramListInfo && !paramListInfo->paramFetch) + { + paramListInfo = copyParamList(paramListInfo); + MarkUnreferencedExternParams((Node *) job->jobQuery, paramListInfo); + } + DistributedExecution *execution = CreateDistributedExecution( distributedPlan->modLevel, taskList, diff --git a/src/backend/distributed/utils/param_utils.c b/src/backend/distributed/utils/param_utils.c new file mode 100644 index 000000000..8aefecb7d --- /dev/null +++ b/src/backend/distributed/utils/param_utils.c @@ -0,0 +1,88 @@ +/*------------------------------------------------------------------------- + * + * param_utils.c + * Utilities to process paramaters. + * + * Copyright (c) Citus Data, Inc. + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include +#include +#include +#include +#include +#include +#include "distributed/param_utils.h" + +/* + * IsExternParamUsedInQuery returns true if the passed in paramId + * is used in the query, false otherwise. + */ +bool +GetParamsUsedInQuery(Node *expression, Bitmapset **paramBitmap) +{ + 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; + } + + /* Found a parameter, mark it in the bitmap and continue */ + *paramBitmap = bms_add_member(*paramBitmap, paramId); + + /* Continue searching */ + return false; + } + + /* keep traversing */ + if (IsA(expression, Query)) + { + return query_tree_walker((Query *) expression, + GetParamsUsedInQuery, + paramBitmap, + 0); + } + else + { + return expression_tree_walker(expression, + GetParamsUsedInQuery, + paramBitmap); + } +} + + +/* + * MarkUnreferencedExternParams marks parameter's type to zero if the + * parameter is not used in the query. + */ +void +MarkUnreferencedExternParams(Node *expression, ParamListInfo boundParams) +{ + int parameterCount = boundParams->numParams; + Bitmapset *paramBitmap = NULL; + + /* Fetch all parameters used in the query */ + GetParamsUsedInQuery(expression, ¶mBitmap); + + /* Check for any missing parameters */ + for (int parameterNum = 1; parameterNum <= parameterCount; parameterNum++) + { + if (!bms_is_member(parameterNum, paramBitmap)) + { + boundParams->params[parameterNum - 1].ptype = 0; + } + } +} diff --git a/src/include/distributed/param_utils.h b/src/include/distributed/param_utils.h new file mode 100644 index 000000000..3e2a6af86 --- /dev/null +++ b/src/include/distributed/param_utils.h @@ -0,0 +1,15 @@ +/*------------------------------------------------------------------------- + * param_utils.h + * + * Copyright (c) Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#ifndef PARAM_UTILS_H +#define PARAM_UTILS_H + +extern bool GetParamsUsedInQuery(Node *expression, Bitmapset **paramBitmap); +extern void MarkUnreferencedExternParams(Node *expression, ParamListInfo boundParams); + +#endif /* PARAM_UTILS_H */ diff --git a/src/test/regress/expected/with_prepare.out b/src/test/regress/expected/with_prepare.out index 993f905b8..6b0bf7d7f 100644 --- a/src/test/regress/expected/with_prepare.out +++ b/src/test/regress/expected/with_prepare.out @@ -211,6 +211,24 @@ ORDER BY user_id, time LIMIT 10; +-- +-- Test a prepared statement with unused argument +-- +CREATE TYPE foo as (x int, y int); +CREATE TABLE footest (x int, y int, z foo); +SELECT create_distributed_table('footest','x'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +INSERT INTO footest VALUES(1, 2, (3,4)); +-- Add a redundant parameter +PREPARE prepared_test_9(foo,foo) AS +WITH a AS ( + SELECT * FROM footest WHERE z = $1 AND x = 1 OFFSET 0 +) +SELECT * FROM a; EXECUTE prepared_test_1; user_id | time | value_1 | value_2 | value_3 | value_4 --------------------------------------------------------------------- @@ -883,6 +901,42 @@ EXECUTE prepared_test_8; (10 rows) ROLLBACK; +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + +EXECUTE prepared_test_9('(3,4)','(2,3)'); + x | y | z +--------------------------------------------------------------------- + 1 | 2 | (3,4) +(1 row) + EXECUTE prepared_partition_column_insert(1); user_id | time | value_1 | value_2 | value_3 | value_4 --------------------------------------------------------------------- diff --git a/src/test/regress/sql/with_prepare.sql b/src/test/regress/sql/with_prepare.sql index 891600b00..486b5af2b 100644 --- a/src/test/regress/sql/with_prepare.sql +++ b/src/test/regress/sql/with_prepare.sql @@ -225,6 +225,21 @@ ORDER BY time LIMIT 10; +-- +-- Test a prepared statement with unused argument +-- +CREATE TYPE foo as (x int, y int); +CREATE TABLE footest (x int, y int, z foo); +SELECT create_distributed_table('footest','x'); +INSERT INTO footest VALUES(1, 2, (3,4)); + +-- Add a redundant parameter +PREPARE prepared_test_9(foo,foo) AS +WITH a AS ( + SELECT * FROM footest WHERE z = $1 AND x = 1 OFFSET 0 +) +SELECT * FROM a; + EXECUTE prepared_test_1; EXECUTE prepared_test_1; EXECUTE prepared_test_1; @@ -301,6 +316,13 @@ EXECUTE prepared_test_8; EXECUTE prepared_test_8; ROLLBACK; +EXECUTE prepared_test_9('(3,4)','(2,3)'); +EXECUTE prepared_test_9('(3,4)','(2,3)'); +EXECUTE prepared_test_9('(3,4)','(2,3)'); +EXECUTE prepared_test_9('(3,4)','(2,3)'); +EXECUTE prepared_test_9('(3,4)','(2,3)'); +EXECUTE prepared_test_9('(3,4)','(2,3)'); + EXECUTE prepared_partition_column_insert(1); EXECUTE prepared_partition_column_insert(2); EXECUTE prepared_partition_column_insert(3);