From 9653c91f89570cdce9a9f6d73606807b33096cb1 Mon Sep 17 00:00:00 2001 From: "okbob@github.com" Date: Sun, 1 Jun 2025 07:32:01 +0200 Subject: [PATCH 09/15] 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 | 56 +++++ src/backend/utils/cache/plancache.c | 24 ++- src/include/nodes/execnodes.h | 14 ++ .../expected/session_variables_dml.out | 191 ++++++++++++++++++ src/test/regress/parallel_schedule | 7 +- .../regress/sql/session_variables_dml.sql | 161 +++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 8 files changed, 479 insertions(+), 4 deletions(-) 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 f1569879b52..0457a729537 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1069,6 +1069,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 ff12e2e1364..ca4e77cd5b4 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -43,7 +43,9 @@ #include "access/xact.h" #include "catalog/namespace.h" #include "catalog/partition.h" +#include "catalog/pg_variable.h" #include "commands/matview.h" +#include "commands/session_variable.h" #include "commands/trigger.h" #include "executor/executor.h" #include "executor/execPartition.h" @@ -196,6 +198,60 @@ 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 = (SessionVariableValue *) + palloc(nSessionVariables * sizeof(SessionVariableValue)); + + /* fill the array */ + foreach_oid(varid, queryDesc->plannedstmt->sessionVariables) + { + AclResult aclresult; + + aclresult = object_aclcheck(VariableRelationId, varid, + GetUserId(), ACL_SELECT); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_VARIABLE, + get_session_variable_name(varid)); + + estate->es_session_variables[i].value = + GetSessionVariable(varid, + &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/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index ab1f2af13e5..1d20f3382b0 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -2043,9 +2043,12 @@ ScanQueryForLocks(Query *parsetree, bool acquire) /* * Recurse into sublink subqueries, too. But we already did the ones in - * the rtable and cteList. + * the rtable and cteList. We need to force a recursive call for session + * variables too, to find and lock variables used in the query (see + * ScanQueryWalker). */ - if (parsetree->hasSubLinks) + if (parsetree->hasSubLinks || + parsetree->hasSessionVariables) { query_tree_walker(parsetree, ScanQueryWalker, &acquire, QTW_IGNORE_RC_SUBQUERIES); @@ -2053,7 +2056,8 @@ ScanQueryForLocks(Query *parsetree, bool acquire) } /* - * Walker to find sublink subqueries for ScanQueryForLocks + * Walker to find sublink subqueries or referenced session variables + * for ScanQueryForLocks */ static bool ScanQueryWalker(Node *node, bool *acquire) @@ -2068,6 +2072,20 @@ ScanQueryWalker(Node *node, bool *acquire) ScanQueryForLocks(castNode(Query, sub->subselect), *acquire); /* Fall through to process lefthand args of SubLink */ } + else if (IsA(node, Param)) + { + Param *p = (Param *) node; + + if (p->paramkind == PARAM_VARIABLE) + { + if (acquire) + LockDatabaseObject(VariableRelationId, p->paramvarid, + 0, AccessShareLock); + else + UnlockDatabaseObject(VariableRelationId, p->paramvarid, + 0, AccessShareLock); + } + } /* * Do NOT recurse into Query nodes, because ScanQueryForLocks already diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 71857feae48..a86b9a53d1a 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -647,6 +647,16 @@ typedef struct AsyncRequest * tuples) */ } AsyncRequest; +/* ---------------- + * SessionVariableValue + * ---------------- + */ +typedef struct SessionVariableValue +{ + bool isnull; + Datum value; +} SessionVariableValue; + /* ---------------- * EState information * @@ -706,6 +716,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..3e21059acc2 --- /dev/null +++ b/src/test/regress/expected/session_variables_dml.out @@ -0,0 +1,191 @@ +CREATE VARIABLE sesvar40 AS int; +-- should not be accessible without variable's fence +-- should fail +SELECT sesvar40; +ERROR: column "sesvar40" does not exist +LINE 1: SELECT sesvar40; + ^ +-- should be ok +SELECT VARIABLE(sesvar40); + sesvar40 +---------- + +(1 row) + +CREATE SCHEMA svartest_dml; +CREATE VARIABLE svartest_dml.sesvar41 AS int; +-- identifier collision test +CREATE TABLE svartest_dml(sesvar41 int); +INSERT INTO svartest_dml VALUES(100); +SELECT sesvar41 FROM svartest_dml; -- 100 + sesvar41 +---------- + 100 +(1 row) + +SELECT VARIABLE(svartest_dml.sesvar41); -- NULL + sesvar41 +---------- + +(1 row) + +-- should fail +SELECT VARIABLE(sesvar41); +ERROR: session variable "sesvar41" doesn't exist +LINE 1: SELECT VARIABLE(sesvar41); + ^ +SET SEARCH_PATH TO svartest_dml; +-- should be ok +SELECT VARIABLE(sesvar41); + sesvar41 +---------- + +(1 row) + +SET SEARCH_PATH TO DEFAULT; +-- should not crash +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; +NOTICE: +CREATE OR REPLACE FUNCTION svartest_dml.testsql() +RETURNS int AS $$ +SELECT VARIABLE(svartest_dml.sesvar41); +$$ LANGUAGE sql; +SELECT svartest_dml.testsql(); + testsql +--------- + +(1 row) + +-- session variable cannot be used as parameter of CALL or EXECUTE +CREATE OR REPLACE PROCEDURE svartest_dml.proc(int) +AS $$ +BEGIN + RAISE NOTICE '%', $1; +END; +$$ LANGUAGE plpgsql; +-- should not crash +CALL svartest_dml.proc(VARIABLE(svartest_dml.sesvar41)); +ERROR: session variable reference is not supported here +LINE 1: CALL svartest_dml.proc(VARIABLE(svartest_dml.sesvar41)); + ^ +PREPARE svartest_dml_prepstmt(int) AS SELECT $1; +-- should not crash +EXECUTE svartest_dml_prepstmt(VARIABLE(svartest_dml.sesvar41)); +ERROR: session variable reference is not supported here +LINE 1: EXECUTE svartest_dml_prepstmt(VARIABLE(svartest_dml.sesvar41... + ^ +DROP PROCEDURE svartest_dml.proc; +DEALLOCATE svartest_dml_prepstmt; +-- domains are supported +CREATE DOMAIN svartest_dml_int_not_null AS int CHECK(value IS NOT NULL); +CREATE VARIABLE svartest_dml.svartest42 AS svartest_dml_int_not_null; +-- should fail +SELECT VARIABLE(svartest_dml.svartest42); +ERROR: value for domain svartest_dml_int_not_null violates check constraint "svartest_dml_int_not_null_check" +DROP VARIABLE svartest_dml.svartest42; +DROP DOMAIN svartest_dml_int_not_null; +CREATE ROLE regress_svartest_dml_read_role; +CREATE OR REPLACE FUNCTION svartest_dml.func_secdef() +RETURNS int AS $$ +SELECT VARIABLE(svartest_dml.sesvar41); +$$ LANGUAGE SQL SECURITY DEFINER; +GRANT USAGE ON SCHEMA svartest_dml TO regress_svartest_dml_read_role; +SET ROLE TO regress_svartest_dml_read_role; +-- should fail +SELECT VARIABLE(svartest_dml.sesvar41); +ERROR: permission denied for session variable sesvar41 +-- should fail +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; +ERROR: permission denied for session variable sesvar41 +CONTEXT: PL/pgSQL expression "VARIABLE(svartest_dml.sesvar41)" +PL/pgSQL function inline_code_block line 3 at RAISE +-- using sql function should to fail +SELECT svartest_dml.testsql(); +ERROR: permission denied for session variable sesvar41 +CONTEXT: SQL function "testsql" statement 1 +-- using security definer should be ok +SELECT svartest_dml.func_secdef(); + func_secdef +------------- + +(1 row) + +SET ROLE TO DEFAULT; +DROP FUNCTION svartest_dml.func_secdef(); +GRANT SELECT ON VARIABLE svartest_dml.sesvar41 TO regress_svartest_dml_read_role; +SET ROLE TO regress_svartest_dml_read_role; +-- should be ok +SELECT VARIABLE(svartest_dml.sesvar41); + sesvar41 +---------- + +(1 row) + +-- should be ok +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; +NOTICE: +SET ROLE TO DEFAULT; +CREATE TABLE svartest_dml.testtab(a int); +INSERT INTO svartest_dml.testtab SELECT * FROM generate_series(1,1000); +CREATE INDEX svartest_dml_testtab_a ON svartest_dml.testtab(a); +ANALYZE svartest_dml.testtab; +-- force index +SET enable_seqscan TO OFF; +-- index scan should be used +EXPLAIN (COSTS OFF) SELECT * FROM svartest_dml.testtab WHERE a = VARIABLE(sesvar40); + QUERY PLAN +--------------------------------------------------------- + Index Only Scan using svartest_dml_testtab_a on testtab + Index Cond: (a = VARIABLE(sesvar40)) +(2 rows) + +DROP INDEX svartest_dml.svartest_dml_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 svartest_dml.testtab WHERE a = 100; + QUERY PLAN +------------------------------------ + Gather + Workers Planned: 2 + -> Parallel Seq Scan on testtab + Filter: (a = 100) +(4 rows) + +-- parallel plan should not be used +EXPLAIN (COSTS OFF) SELECT * FROM svartest_dml.testtab WHERE a = VARIABLE(sesvar40); + QUERY PLAN +------------------------------------ + Seq Scan on testtab + Filter: (a = VARIABLE(sesvar40)) +(2 rows) + +RESET parallel_setup_cost; +RESET parallel_tuple_cost; +RESET min_parallel_table_scan_size; +RESET max_parallel_workers_per_gather; +DROP SCHEMA svartest_dml CASCADE; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to session variable svartest_dml.sesvar41 +drop cascades to function svartest_dml.testsql() +drop cascades to table svartest_dml.testtab +DROP ROLE regress_svartest_dml_read_role; +DROP VARIABLE sesvar40; +DROP TABLE svartest_dml; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 95c76baf9ae..b16ec065950 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -115,7 +115,7 @@ test: json jsonb json_encoding jsonpath jsonpath_encoding jsonb_jsonpath sqljson # NB: temp.sql does reconnects which transiently use 2 connections, # so keep this parallel group to at most 19 tests # ---------- -test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion truncate alter_table sequence polymorphism rowtypes returning largeobject with xml session_variables_ddl session_variables_acl +test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion truncate alter_table sequence polymorphism rowtypes returning largeobject with xml # ---------- # Another group of parallel tests @@ -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_acl 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..b2870dde9e9 --- /dev/null +++ b/src/test/regress/sql/session_variables_dml.sql @@ -0,0 +1,161 @@ +CREATE VARIABLE sesvar40 AS int; + +-- should not be accessible without variable's fence +-- should fail +SELECT sesvar40; + +-- should be ok +SELECT VARIABLE(sesvar40); + +CREATE SCHEMA svartest_dml; +CREATE VARIABLE svartest_dml.sesvar41 AS int; + +-- identifier collision test +CREATE TABLE svartest_dml(sesvar41 int); +INSERT INTO svartest_dml VALUES(100); + +SELECT sesvar41 FROM svartest_dml; -- 100 +SELECT VARIABLE(svartest_dml.sesvar41); -- NULL + +-- should fail +SELECT VARIABLE(sesvar41); + +SET SEARCH_PATH TO svartest_dml; + +-- should be ok +SELECT VARIABLE(sesvar41); + +SET SEARCH_PATH TO DEFAULT; + +-- should not crash +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; + +CREATE OR REPLACE FUNCTION svartest_dml.testsql() +RETURNS int AS $$ +SELECT VARIABLE(svartest_dml.sesvar41); +$$ LANGUAGE sql; + +SELECT svartest_dml.testsql(); + +-- session variable cannot be used as parameter of CALL or EXECUTE +CREATE OR REPLACE PROCEDURE svartest_dml.proc(int) +AS $$ +BEGIN + RAISE NOTICE '%', $1; +END; +$$ LANGUAGE plpgsql; + +-- should not crash +CALL svartest_dml.proc(VARIABLE(svartest_dml.sesvar41)); + +PREPARE svartest_dml_prepstmt(int) AS SELECT $1; + +-- should not crash +EXECUTE svartest_dml_prepstmt(VARIABLE(svartest_dml.sesvar41)); + +DROP PROCEDURE svartest_dml.proc; +DEALLOCATE svartest_dml_prepstmt; + +-- domains are supported +CREATE DOMAIN svartest_dml_int_not_null AS int CHECK(value IS NOT NULL); +CREATE VARIABLE svartest_dml.svartest42 AS svartest_dml_int_not_null; + +-- should fail +SELECT VARIABLE(svartest_dml.svartest42); + +DROP VARIABLE svartest_dml.svartest42; +DROP DOMAIN svartest_dml_int_not_null; + +CREATE ROLE regress_svartest_dml_read_role; + +CREATE OR REPLACE FUNCTION svartest_dml.func_secdef() +RETURNS int AS $$ +SELECT VARIABLE(svartest_dml.sesvar41); +$$ LANGUAGE SQL SECURITY DEFINER; + +GRANT USAGE ON SCHEMA svartest_dml TO regress_svartest_dml_read_role; + +SET ROLE TO regress_svartest_dml_read_role; + +-- should fail +SELECT VARIABLE(svartest_dml.sesvar41); + +-- should fail +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; + +-- using sql function should to fail +SELECT svartest_dml.testsql(); + +-- using security definer should be ok +SELECT svartest_dml.func_secdef(); + +SET ROLE TO DEFAULT; + +DROP FUNCTION svartest_dml.func_secdef(); + +GRANT SELECT ON VARIABLE svartest_dml.sesvar41 TO regress_svartest_dml_read_role; + +SET ROLE TO regress_svartest_dml_read_role; + +-- should be ok +SELECT VARIABLE(svartest_dml.sesvar41); + +-- should be ok +DO $$ +BEGIN + RAISE NOTICE '%', VARIABLE(svartest_dml.sesvar41); +END; +$$; + +SET ROLE TO DEFAULT; + +CREATE TABLE svartest_dml.testtab(a int); + +INSERT INTO svartest_dml.testtab SELECT * FROM generate_series(1,1000); + +CREATE INDEX svartest_dml_testtab_a ON svartest_dml.testtab(a); + +ANALYZE svartest_dml.testtab; + +-- force index +SET enable_seqscan TO OFF; + +-- index scan should be used +EXPLAIN (COSTS OFF) SELECT * FROM svartest_dml.testtab WHERE a = VARIABLE(sesvar40); + +DROP INDEX svartest_dml.svartest_dml_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 svartest_dml.testtab WHERE a = 100; + +-- parallel plan should not be used +EXPLAIN (COSTS OFF) SELECT * FROM svartest_dml.testtab WHERE a = VARIABLE(sesvar40); + +RESET parallel_setup_cost; +RESET parallel_tuple_cost; +RESET min_parallel_table_scan_size; +RESET max_parallel_workers_per_gather; + +DROP SCHEMA svartest_dml CASCADE; +DROP ROLE regress_svartest_dml_read_role; + +DROP VARIABLE sesvar40; + +DROP TABLE svartest_dml; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f8d95c8e330..f32dcc29023 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2713,6 +2713,7 @@ SerializedTransactionState Session SessionBackupState SessionEndType +SessionVariableValue SetConstraintState SetConstraintStateData SetConstraintTriggerData -- 2.51.0