From de446b6d5460a0800a40a217669c8939c8e37fc1 Mon Sep 17 00:00:00 2001 From: "okbob@github.com" Date: Fri, 13 Dec 2024 06:28:45 +0100 Subject: [PATCH 09/22] dynamic check of usage of session variable fences This patch allows to specify dynamic context where fencing of session variables is required. Possible contexts are none, nospi or all. With this check is possible to force variable fencing outside stored procedures. --- doc/src/sgml/config.sgml | 56 ++++++++++++ doc/src/sgml/ddl.sgml | 3 +- src/backend/commands/session_variable.c | 33 +++++++ src/backend/executor/execExpr.c | 4 + src/backend/executor/spi.c | 8 ++ src/backend/utils/misc/guc_tables.c | 19 ++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/commands/session_variable.h | 16 ++++ src/include/executor/spi.h | 1 + .../regress/expected/session_variables.out | 90 +++++++++++++++++++ src/test/regress/sql/session_variables.sql | 73 +++++++++++++++ 11 files changed, 303 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 465dfaffff9..03296d00083 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10928,6 +10928,62 @@ DETAIL: Session variables can be shadowed by columns, routine's variables and r + + session_variables_use_fence_context_guard (boolean) + + session_variables_use_fence_context_guard configuration parameter + + + + + When on, an error is raised when a session variable identifier is used + inside a query without variable fence in choosed context. The default + is none. The error is raised only when variable is + used in places, where an collisions with column names is possible (and + when variable fences are possible). The allowed values of + session_variables_use_fence_context_guard are + none (allow using session variables without + variable's fences always), all (requires variable + fencing everywhere), and nospi (requires variable + fencing outside SPI contexts (outside stored procedures). + +SET session_variables_use_fence_context_guard TO 'nospi'; +CREATE VARIABLE a int; + +LET a = 10; +DO $$ +BEGIN + RAISE NOTICE 'a = %', a; +END; +$$; + +SELECT a; +SELECT VARIABLE(s); + + + +NOTICE: a = 10 + +ERROR: session variable "public.a" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". + +SELECT variable(a); + a +---- + 10 +(1 row) + + + + + When you afraid about unwanted usage of session variables, set + this feature to all or minimaly to + nospi. + + + + session_variables_use_fence_warning_guard (boolean) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 4e8e7c6a42a..ba6bc6d3206 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -5469,7 +5469,8 @@ SELECT VARIABLE(current_user_id); When there is a risk of possible collisions between variable identifiers and column names, then using variable fence syntax can be recommended. Usage session variable without variable fence can be detected by - warning . + warning + or by check . diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c index 4a29ed6e0c3..7c159860b47 100644 --- a/src/backend/commands/session_variable.c +++ b/src/backend/commands/session_variable.c @@ -17,6 +17,7 @@ #include "access/xact.h" #include "catalog/pg_variable.h" #include "commands/session_variable.h" +#include "executor/spi.h" #include "executor/svariableReceiver.h" #include "funcapi.h" #include "miscadmin.h" @@ -31,6 +32,13 @@ #include "utils/snapmgr.h" #include "utils/syscache.h" +/* + * The session variables use fence context allow to specify + * the contexts where using session variable fences are required. + */ +int session_variables_use_fence_context_guard = + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE; + /* * The values of session variables are stored in the backend's private memory * in the dedicated memory context SVariableMemoryContext in binary format. @@ -790,3 +798,28 @@ ResetSessionVariables(void) if (SVariableMemoryContext != NULL) MemoryContextReset(SVariableMemoryContext); } + +/* + * Raise error when unfenced variable is used in wrong context. + * This check allows to force session variable fencing outside + * stored procedures environment. + */ +void +SessionVariablesUseFenceContextCheck(Oid varid, bool is_fenced) +{ + if (is_fenced) + return; + + if (((session_variables_use_fence_context_guard == + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL) && !is_fenced) || + ((session_variables_use_fence_context_guard == + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI) && + !SPI_inside_spi_context())) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("session variable \"%s.%s\" is not used inside variable fence", + get_namespace_name(get_session_variable_namespace(varid)), + get_session_variable_name(varid)), + errdetail("There is a risk of unwanted usage of session variable."), + errhint("Use variable fence \"VARIABLE(varname) for access to variable\"."))); +} diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 270e6d344be..682d821afc4 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -34,6 +34,7 @@ #include "catalog/objectaccess.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "commands/session_variable.h" #include "executor/execExpr.h" #include "executor/nodeSubplan.h" #include "funcapi.h" @@ -1048,6 +1049,9 @@ ExecInitExprRec(Expr *node, ExprState *state, SessionVariableValue *es_session_variables = NULL; SessionVariableValue *var; + SessionVariablesUseFenceContextCheck(param->paramvarid, + param->paramvarfenced); + if (state->parent && state->parent->state) { es_session_variables = state->parent->state->es_session_variables; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index ecb2e4ccaa1..4ce6bc5f298 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -590,6 +590,14 @@ SPI_inside_nonatomic_context(void) return true; } +/* + * Are we executed inside a SPI? + */ +bool +SPI_inside_spi_context(void) +{ + return (_SPI_current != NULL); +} /* Parse, plan, and execute a query string */ int diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index f6553a64433..7873e67473b 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -40,6 +40,7 @@ #include "catalog/storage.h" #include "commands/async.h" #include "commands/event_trigger.h" +#include "commands/session_variable.h" #include "commands/tablespace.h" #include "commands/trigger.h" #include "commands/user.h" @@ -423,6 +424,13 @@ static const struct config_enum_entry debug_logical_replication_streaming_option {NULL, 0, false} }; +static const struct config_enum_entry session_variables_use_fence_context_guard_options[] = { + {"none", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, false}, + {"all", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL, false}, + {"nospi", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI, false}, + {NULL, 0, false} +}; + StaticAssertDecl(lengthof(ssl_protocol_versions_info) == (PG_TLS1_3_VERSION + 2), "array length mismatch"); @@ -5263,6 +5271,17 @@ struct config_enum ConfigureNamesEnum[] = NULL, NULL, NULL }, + { + {"session_variables_use_fence_context_guard", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Chooses the context where session variables should be used inside fence."), + NULL + }, + &session_variables_use_fence_context_guard, + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, session_variables_use_fence_context_guard_options, + NULL, NULL, NULL + }, + + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index f19046c145c..ede83fb81f3 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -732,6 +732,7 @@ autovacuum_worker_slots = 16 # autovacuum worker slots to allocate #session_replication_role = 'origin' #session_variables_ambiguity_warning = off #session_variables_use_fence_warning_guard = off +#session_variables_use_fence_context_guard = 'none' #statement_timeout = 0 # in milliseconds, 0 is disabled #transaction_timeout = 0 # in milliseconds, 0 is disabled #lock_timeout = 0 # in milliseconds, 0 is disabled diff --git a/src/include/commands/session_variable.h b/src/include/commands/session_variable.h index d3cc79b5608..e58a5a0110a 100644 --- a/src/include/commands/session_variable.h +++ b/src/include/commands/session_variable.h @@ -30,5 +30,21 @@ extern void ExecuteLetStmt(ParseState *pstate, LetStmt *stmt, ParamListInfo para QueryEnvironment *queryEnv, QueryCompletion *qc); extern void ResetSessionVariables(void); +extern void SessionVariablesUseFenceContextCheck(Oid varid, bool is_fenced); + +/* + * Specify the dynamic context where variable fence is required + */ +typedef enum SessionVariablesUseFenceContextGuard +{ + /* Disable fence usage context guard */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, + /* Enable fence usage context guard everywhere */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL, + /* Enable fence usage context guard when SPI is not used */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI, +} SessionVariablesUseFenceContextGuard; + +extern PGDLLIMPORT int session_variables_use_fence_context_guard; #endif diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index d064d1a9b76..4690a95f39a 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -203,5 +203,6 @@ extern void SPI_rollback_and_chain(void); extern void AtEOXact_SPI(bool isCommit); extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid); extern bool SPI_inside_nonatomic_context(void); +extern bool SPI_inside_spi_context(void); #endif /* SPI_H */ diff --git a/src/test/regress/expected/session_variables.out b/src/test/regress/expected/session_variables.out index 9ac08a14e0b..90eec27a43f 100644 --- a/src/test/regress/expected/session_variables.out +++ b/src/test/regress/expected/session_variables.out @@ -2028,3 +2028,93 @@ DROP SCHEMA testvar; SET session_variables_ambiguity_warning TO DEFAULT; SET session_variables_use_fence_warning_guard TO DEFAULT; SET search_path TO DEFAULT; +-- test session_variables_use_fence_context_guard +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); +SET session_variables_use_fence_context_guard TO 'none'; +-- should be ok +SELECT var1, VARIABLE(var1); + var1 | var1 +------+------ + 20 | 20 +(1 row) + +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO 'all'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +-- should fail +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; +END; +$$; +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +CONTEXT: SQL statement "SELECT a FROM vartest_tab1 WHERE a = var1" +PL/pgSQL function inline_code_block line 4 at SQL statement +-- should fail +DO $$ +DECLARE t int DEFAULT 20; +BEGIN + RAISE NOTICE '% %', var1, var1 = t; +END; +$$; +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +CONTEXT: PL/pgSQL expression "var1" +PL/pgSQL function inline_code_block line 4 at RAISE +-- should be ok +SELECT VARIABLE(var1), VARIABLE(var1); + var1 | var1 +------+------ + 20 | 20 +(1 row) + +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO 'nospi'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +-- should be ok +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO DEFAULT; +DROP VARIABLE var1; +DROP TABLE vartest_tab1; diff --git a/src/test/regress/sql/session_variables.sql b/src/test/regress/sql/session_variables.sql index c5420183d94..330b0e58f8e 100644 --- a/src/test/regress/sql/session_variables.sql +++ b/src/test/regress/sql/session_variables.sql @@ -1401,3 +1401,76 @@ DROP SCHEMA testvar; SET session_variables_ambiguity_warning TO DEFAULT; SET session_variables_use_fence_warning_guard TO DEFAULT; SET search_path TO DEFAULT; + +-- test session_variables_use_fence_context_guard +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); + +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); + +SET session_variables_use_fence_context_guard TO 'none'; + +-- should be ok +SELECT var1, VARIABLE(var1); +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO 'all'; + +-- should fail +SELECT var1, VARIABLE(var1); + +-- should fail +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; +END; +$$; + +-- should fail +DO $$ +DECLARE t int DEFAULT 20; +BEGIN + RAISE NOTICE '% %', var1, var1 = t; +END; +$$; + +-- should be ok +SELECT VARIABLE(var1), VARIABLE(var1); +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO 'nospi'; + +-- should fail +SELECT var1, VARIABLE(var1); + +-- should be ok +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO DEFAULT; + +DROP VARIABLE var1; +DROP TABLE vartest_tab1; -- 2.48.1