From 181a30877bb66b956c84be7729741e161cdb2c1a Mon Sep 17 00:00:00 2001 From: "okbob@github.com" Date: Sat, 22 Nov 2025 06:40:46 +0100 Subject: [PATCH 04/11] fill an auxiliary buffer with values of session variables used in query and locks variables used in query. Now we can read the content of any session variable. Direct reading from expression executor is not allowed, so we cannot to use session variables inside CALL or EXECUTE commands (can be supported with direct access to session variables (from expression executor) - postponed). Using session variables blocks parallel query execution. It is not technical problem (it just needs a serialization/deserialization of es_session_varibles buffer), but it increases a size of patch (and then it is postponed). --- src/backend/executor/execExpr.c | 29 ++++ src/backend/executor/execMain.c | 49 +++++++ src/include/nodes/execnodes.h | 14 ++ .../expected/session_variables_dml.out | 135 ++++++++++++++++++ src/test/regress/parallel_schedule | 5 + .../regress/sql/session_variables_dml.sql | 120 ++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 7 files changed, 353 insertions(+) create mode 100644 src/test/regress/expected/session_variables_dml.out create mode 100644 src/test/regress/sql/session_variables_dml.sql diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 77229141b38..8e813ba2d7a 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1102,6 +1102,35 @@ ExecInitExprRec(Expr *node, ExprState *state, ExprEvalPushStep(state, &scratch); } break; + case PARAM_VARIABLE: + { + int es_num_session_variables = 0; + SessionVariableValue *es_session_variables = NULL; + SessionVariableValue *var; + + if (state->parent && state->parent->state) + { + es_session_variables = state->parent->state->es_session_variables; + es_num_session_variables = state->parent->state->es_num_session_variables; + } + + Assert(es_session_variables); + + /* parameter sanity checks */ + if (param->paramid >= es_num_session_variables) + elog(ERROR, "paramid of PARAM_VARIABLE param is out of range"); + + var = &es_session_variables[param->paramid]; + + /* + * In this case, pass the value like a constant. + */ + scratch.opcode = EEOP_CONST; + scratch.d.constval.value = var->value; + scratch.d.constval.isnull = var->isnull; + ExprEvalPushStep(state, &scratch); + } + break; default: elog(ERROR, "unrecognized paramkind: %d", (int) param->paramkind); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 45e00c6af85..6c8e434bf37 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -45,6 +45,7 @@ #include "catalog/namespace.h" #include "catalog/partition.h" #include "commands/matview.h" +#include "commands/session_variable.h" #include "commands/trigger.h" #include "executor/executor.h" #include "executor/execPartition.h" @@ -198,6 +199,54 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags) Assert(queryDesc->sourceText != NULL); estate->es_sourceText = queryDesc->sourceText; + /* + * The executor doesn't work with session variables directly. Values of + * related session variables are copied to a dedicated array, and this + * array is passed to the executor. This array is stable "snapshot" of + * values of used session variables. There are three benefits of this + * strategy: + * + * - consistency with external parameters and plpgsql variables, + * + * - session variables can be parallel safe, + * + * - we don't need make fresh copy for any read of session variable (this + * is necessary because the internally the session variable can be changed + * inside query execution time, and then a reference to previously + * returned value can be corrupted). + */ + if (queryDesc->plannedstmt->sessionVariables) + { + int nSessionVariables; + int i = 0; + + /* + * In this case, the query uses session variables, but we have to + * prepare the array with passed values (of used session variables) + * first. + */ + Assert(!IsParallelWorker()); + nSessionVariables = list_length(queryDesc->plannedstmt->sessionVariables); + + /* create the array used for passing values of used session variables */ + estate->es_session_variables = palloc_array(SessionVariableValue, + nSessionVariables); + + /* fill the array */ + foreach_node(Param, param, queryDesc->plannedstmt->sessionVariables) + { + estate->es_session_variables[i].value = + GetSessionVariableWithTypecheck(param->paramvarname, + param->paramtype, + param->paramtypmod, + &estate->es_session_variables[i].isnull); + + i++; + } + + estate->es_num_session_variables = nSessionVariables; + } + /* * Fill in the query environment, if any, from queryDesc. */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 090cfccf65f..1723594e016 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -679,6 +679,16 @@ typedef struct AsyncRequest * tuples) */ } AsyncRequest; +/* ---------------- + * SessionVariableValue + * ---------------- + */ +typedef struct SessionVariableValue +{ + bool isnull; + Datum value; +} SessionVariableValue; + /* ---------------- * EState information * @@ -738,6 +748,10 @@ typedef struct EState ParamListInfo es_param_list_info; /* values of external params */ ParamExecData *es_param_exec_vals; /* values of internal params */ + /* Session variables info: */ + int es_num_session_variables; /* number of used variables */ + SessionVariableValue *es_session_variables; /* array of copies of values */ + QueryEnvironment *es_queryEnv; /* query environment */ /* Other working state: */ diff --git a/src/test/regress/expected/session_variables_dml.out b/src/test/regress/expected/session_variables_dml.out new file mode 100644 index 00000000000..1519bf723e0 --- /dev/null +++ b/src/test/regress/expected/session_variables_dml.out @@ -0,0 +1,135 @@ +CREATE TEMP VARIABLE temp_var01 AS int; +-- should not be accessible without variable's fence +-- should fail +SELECT temp_var01; +ERROR: column "temp_var01" does not exist +LINE 1: SELECT temp_var01; + ^ +-- should be ok +SELECT VARIABLE(temp_var01); + temp_var01 +------------ + +(1 row) + +-- should not crash +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(temp_var01); +END; +$$; +NOTICE: +-- variables cannot be used by persistent objects +-- that checks dependency +-- should fail +CREATE TEMP VIEW tempv AS SELECT VARIABLE(temp_var01); +ERROR: session variable "temp_var01" cannot be referenced in a catalog object +CREATE OR REPLACE FUNCTION testvar_sql() +RETURNS int AS $$ +SELECT VARIABLE(temp_var01); +$$ LANGUAGE sql; +SELECT testvar_sql(); + testvar_sql +------------- + +(1 row) + +-- session variable cannot be used as parameter of CALL or EXECUTE +CREATE OR REPLACE PROCEDURE testvar_proc(int) +AS $$ +BEGIN + RAISE NOTICE '%', $1; +END; +$$ LANGUAGE plpgsql; +-- should not crash +CALL testvar_proc(VARIABLE(temp_var01)); +ERROR: session variable reference is not supported here +LINE 1: CALL testvar_proc(VARIABLE(temp_var01)); + ^ +PREPARE prepstmt(int) AS SELECT $1; +-- should not crash +EXECUTE prepstmt(VARIABLE(temp_var01)); +ERROR: session variable reference is not supported here +LINE 1: EXECUTE prepstmt(VARIABLE(temp_var01)); + ^ +DROP PROCEDURE testvar_proc; +DEALLOCATE prepstmt; +CREATE ROLE regress_session_variable_test_role_03; +CREATE OR REPLACE FUNCTION testvar_sd() +RETURNS void AS $$ +BEGIN + RAISE NOTICE '%', VARIABLE(temp_var01); +END; +$$ LANGUAGE plpgsql; +-- only owner can read data +SET ROLE TO regress_session_variable_test_role_03; +-- should fail +SELECT VARIABLE(temp_var01); +ERROR: permission denied for session variable temp_var01 +-- fx with security definer should be ok +SELECT testvar_sd(); +ERROR: permission denied for session variable temp_var01 +CONTEXT: PL/pgSQL expression "VARIABLE(temp_var01)" +PL/pgSQL function testvar_sd() line 3 at RAISE +SET ROLE TO default; +DROP VARIABLE temp_var01; +-- there is not plan cache invalidation +-- but still functions that uses dropped variables +-- should not to crash +SELECT testvar_sd(); +ERROR: session variable "temp_var01" doesn't exist +CONTEXT: PL/pgSQL expression "VARIABLE(temp_var01)" +PL/pgSQL function testvar_sd() line 3 at RAISE +SELECT testvar_sql(); +ERROR: session variable "temp_var01" doesn't exist +CONTEXT: SQL function "testvar_sql" during inlining +DROP FUNCTION testvar_sql(); +DROP FUNCTION testvar_sd(); +DROP ROLE regress_session_variable_test_role_03; +CREATE TABLE testvar_testtab(a int); +CREATE TEMP VARIABLE temp_var02 AS int; +INSERT INTO testvar_testtab SELECT * FROM generate_series(1,1000); +CREATE INDEX testvar_testtab_a ON testvar_testtab(a); +ANALYZE testvar_testtab; +-- force index +SET enable_seqscan TO OFF; +-- index scan should be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = VARIABLE(temp_var02); + QUERY PLAN +------------------------------------------------------------ + Index Only Scan using testvar_testtab_a on testvar_testtab + Index Cond: (a = VARIABLE(temp_var02)) +(2 rows) + +DROP INDEX testvar_testtab_a; +SET enable_seqscan TO DEFAULT; +-- parallel execution should be blocked +-- Encourage use of parallel plans +SET parallel_setup_cost = 0; +SET parallel_tuple_cost = 0; +SET min_parallel_table_scan_size = 0; +SET max_parallel_workers_per_gather = 2; +-- parallel plan should be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = 100; + QUERY PLAN +-------------------------------------------- + Gather + Workers Planned: 2 + -> Parallel Seq Scan on testvar_testtab + Filter: (a = 100) +(4 rows) + +-- parallel plan should not be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = VARIABLE(temp_var02); + QUERY PLAN +-------------------------------------- + Seq Scan on testvar_testtab + Filter: (a = VARIABLE(temp_var02)) +(2 rows) + +RESET parallel_setup_cost; +RESET parallel_tuple_cost; +RESET min_parallel_table_scan_size; +RESET max_parallel_workers_per_gather; +DROP TABLE testvar_testtab; +DROP VARIABLE temp_var02; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index ec92c717bf6..b7c0ad00149 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -140,3 +140,8 @@ test: fast_default # run tablespace test at the end because it drops the tablespace created during # setup that other tests may use. test: tablespace + +# ---------- +# Another group of parallel tests (session variables related) +# ---------- +test: session_variables_ddl session_variables_dml diff --git a/src/test/regress/sql/session_variables_dml.sql b/src/test/regress/sql/session_variables_dml.sql new file mode 100644 index 00000000000..bf56b19467b --- /dev/null +++ b/src/test/regress/sql/session_variables_dml.sql @@ -0,0 +1,120 @@ +CREATE TEMP VARIABLE temp_var01 AS int; + +-- should not be accessible without variable's fence +-- should fail +SELECT temp_var01; + +-- should be ok +SELECT VARIABLE(temp_var01); + +-- should not crash +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(temp_var01); +END; +$$; + +-- variables cannot be used by persistent objects +-- that checks dependency +-- should fail +CREATE TEMP VIEW tempv AS SELECT VARIABLE(temp_var01); + +CREATE OR REPLACE FUNCTION testvar_sql() +RETURNS int AS $$ +SELECT VARIABLE(temp_var01); +$$ LANGUAGE sql; + +SELECT testvar_sql(); + +-- session variable cannot be used as parameter of CALL or EXECUTE +CREATE OR REPLACE PROCEDURE testvar_proc(int) +AS $$ +BEGIN + RAISE NOTICE '%', $1; +END; +$$ LANGUAGE plpgsql; + +-- should not crash +CALL testvar_proc(VARIABLE(temp_var01)); + +PREPARE prepstmt(int) AS SELECT $1; + +-- should not crash +EXECUTE prepstmt(VARIABLE(temp_var01)); + +DROP PROCEDURE testvar_proc; +DEALLOCATE prepstmt; + +CREATE ROLE regress_session_variable_test_role_03; + +CREATE OR REPLACE FUNCTION testvar_sd() +RETURNS void AS $$ +BEGIN + RAISE NOTICE '%', VARIABLE(temp_var01); +END; +$$ LANGUAGE plpgsql; + +-- only owner can read data +SET ROLE TO regress_session_variable_test_role_03; + +-- should fail +SELECT VARIABLE(temp_var01); + +-- fx with security definer should be ok +SELECT testvar_sd(); + +SET ROLE TO default; + +DROP VARIABLE temp_var01; + +-- there is not plan cache invalidation +-- but still functions that uses dropped variables +-- should not to crash + +SELECT testvar_sd(); +SELECT testvar_sql(); + +DROP FUNCTION testvar_sql(); +DROP FUNCTION testvar_sd(); + +DROP ROLE regress_session_variable_test_role_03; + +CREATE TABLE testvar_testtab(a int); +CREATE TEMP VARIABLE temp_var02 AS int; + +INSERT INTO testvar_testtab SELECT * FROM generate_series(1,1000); + +CREATE INDEX testvar_testtab_a ON testvar_testtab(a); + +ANALYZE testvar_testtab; + +-- force index +SET enable_seqscan TO OFF; + +-- index scan should be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = VARIABLE(temp_var02); + +DROP INDEX testvar_testtab_a; + +SET enable_seqscan TO DEFAULT; + +-- parallel execution should be blocked +-- Encourage use of parallel plans +SET parallel_setup_cost = 0; +SET parallel_tuple_cost = 0; +SET min_parallel_table_scan_size = 0; +SET max_parallel_workers_per_gather = 2; + +-- parallel plan should be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = 100; + +-- parallel plan should not be used +EXPLAIN (COSTS OFF) SELECT * FROM testvar_testtab WHERE a = VARIABLE(temp_var02); + +RESET parallel_setup_cost; +RESET parallel_tuple_cost; +RESET min_parallel_table_scan_size; +RESET max_parallel_workers_per_gather; + +DROP TABLE testvar_testtab; +DROP VARIABLE temp_var02; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 6f889e18a6d..04f1bd17dcc 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2805,6 +2805,7 @@ SerializedTransactionState Session SessionBackupState SessionEndType +SessionVariableValue SetConstraintState SetConstraintStateData SetConstraintTriggerData -- 2.53.0