From aa0f9e325dbda96661e68cd8744a6c8c794de0d1 Mon Sep 17 00:00:00 2001 From: "okbob@github.com" Date: Fri, 13 Dec 2024 06:28:45 +0100 Subject: [PATCH 09/22] possibility to control implicit visibility of session variables. Fenced variable is visible everywhere. Unfenced variable can be visible everywhere (when session_variables_visibility is everywhere), or can be visible nowhere (session_variables_visibility is nowhere) or can be visible inside stored procedures (session_variables_visibility is PL). --- doc/src/sgml/config.sgml | 53 +++++++++++ doc/src/sgml/ddl.sgml | 3 +- src/backend/parser/parse_expr.c | 30 ++++++- src/backend/parser/parse_node.c | 1 + src/backend/utils/misc/guc_tables.c | 18 ++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/parser/parse_expr.h | 14 +++ src/include/parser/parse_node.h | 3 + src/pl/plpgsql/src/pl_comp.c | 1 + .../regress/expected/session_variables.out | 90 +++++++++++++++++++ src/test/regress/sql/session_variables.sql | 73 +++++++++++++++ 11 files changed, 284 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 1500226e9b5..257aa903629 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -11062,6 +11062,59 @@ SELECT a, VARIABLE(b) FROM foo; + + session_variables_visibility (boolean) + + session_variables_visibility configuration parameter + + + + + This option specify, where the session variables are visible implicitly + without using variable fencing. The default is everywhere. + The allowed values of session_variables_visibility are + everywhere (allows using session variables without + variables's fences always), nowhere (requires variable's + fencing always) or pl (the variable fencing is optional + in stored procedures). + +SET session_variables_visibility TO 'pl'; +CREATE VARIABLE a int; + +LET a = 10; +DO $$ +BEGIN + RAISE NOTICE 'a = %', a; +END; +$$; + +SELECT a; +SELECT VARIABLE(s); + + + +NOTICE: a = 10 + +ERROR: column "a" does not exist +LINE 1: SELECT a; + ^ + +SELECT variable(a); + a +---- + 10 +(1 row) + + + + + When you afraid about unwanted usage of session variables, set + this feature to nowhere or minimaly to + pl. + + + + standard_conforming_strings (boolean) stringsstandard conforming diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 4e8e7c6a42a..748219b3804 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 controlled by . diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index dd03b1f14db..bc751b4a441 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -47,7 +47,7 @@ bool Transform_null_equals = false; bool session_variables_ambiguity_warning = false; bool session_variables_use_fence_warning_guard = false; - +int session_variables_visibility = SESSIONVARIABLE_VISIBILITY_EVERYWHERE; static Node *transformExprRecurse(ParseState *pstate, Node *expr); static Node *transformParamRef(ParseState *pstate, ParamRef *pref); @@ -115,6 +115,7 @@ static Node *make_nulltest_from_distinct(ParseState *pstate, static Node *makeParamSessionVariable(ParseState *pstate, Oid varid, Oid typid, int32 typmod, Oid collid, char *attrname, bool fenced, int location); +static bool session_variables_are_visible(ParseState *pstate); /* @@ -949,8 +950,14 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref) * need to identify session variables in such a context, but identifying * them allows us to raise meaningful error messages like "you cannot use * session variables here". + * + * The visibility of session variables can be controlled by GUG + * session_variables_visibility. Don't try to detect session variables + * when are in context, where are not visible and variable fence are + * required. */ - if (expr_kind_allows_session_variables(pstate->p_expr_kind)) + if (expr_kind_allows_session_variables(pstate->p_expr_kind) && + session_variables_are_visible(pstate)) { Oid varid = InvalidOid; char *attrname = NULL; @@ -1225,6 +1232,25 @@ transformVariableFence(ParseState *pstate, VariableFence *vf) return result; } +/* returns true, when session variables are visible without variable fence */ +static bool +session_variables_are_visible(ParseState *pstate) +{ + switch (session_variables_visibility) + { + case SESSIONVARIABLE_VISIBILITY_NOWHERE: + return false; + + case SESSIONVARIABLE_VISIBILITY_EVERYWHERE: + return true; + + case SESSIONVARIABLE_VISIBILITY_PL: + return pstate->p_is_PL; + } + + return false; +} + /* Test whether an a_expr is a plain NULL constant or not */ static bool exprIsNullConstant(Node *arg) diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c index d6feb16aef3..4e337c3af82 100644 --- a/src/backend/parser/parse_node.c +++ b/src/backend/parser/parse_node.c @@ -51,6 +51,7 @@ make_parsestate(ParseState *parentParseState) if (parentParseState) { pstate->p_sourcetext = parentParseState->p_sourcetext; + pstate->p_is_PL = parentParseState->p_is_PL; /* all hooks are copied from parent */ pstate->p_pre_columnref_hook = parentParseState->p_pre_columnref_hook; pstate->p_post_columnref_hook = parentParseState->p_post_columnref_hook; diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 237b1212e7a..3886d199cea 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -423,6 +423,13 @@ static const struct config_enum_entry debug_logical_replication_streaming_option {NULL, 0, false} }; +static const struct config_enum_entry session_variables_visibility_options[] = { + {"nowhere", SESSIONVARIABLE_VISIBILITY_NOWHERE, false}, + {"everywhere", SESSIONVARIABLE_VISIBILITY_EVERYWHERE, false}, + {"pl", SESSIONVARIABLE_VISIBILITY_PL, false}, + {NULL, 0, false} +}; + StaticAssertDecl(lengthof(ssl_protocol_versions_info) == (PG_TLS1_3_VERSION + 2), "array length mismatch"); @@ -5283,6 +5290,17 @@ struct config_enum ConfigureNamesEnum[] = NULL, NULL, NULL }, + { + {"session_variables_visibility", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Chooses the context where session variables can be visible without variable fence."), + NULL + }, + &session_variables_visibility, + SESSIONVARIABLE_VISIBILITY_EVERYWHERE, session_variables_visibility_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 b84e8659ae1..46b05ee081a 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -737,6 +737,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_visibility = 'everywhere' #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/parser/parse_expr.h b/src/include/parser/parse_expr.h index 5bde43758d1..e2f8d832242 100644 --- a/src/include/parser/parse_expr.h +++ b/src/include/parser/parse_expr.h @@ -15,10 +15,24 @@ #include "parser/parse_node.h" +/* + * Specify the dynamic context where variable fence is required + */ +typedef enum SessionVariablesVisibility +{ + /* Variable fencing is required everywhere */ + SESSIONVARIABLE_VISIBILITY_NOWHERE, + /* Variable fencing is required nowhere */ + SESSIONVARIABLE_VISIBILITY_EVERYWHERE, + /* Variable fencing is required outside PL */ + SESSIONVARIABLE_VISIBILITY_PL, +} SessionVariablesVisibility; + /* GUC parameters */ extern PGDLLIMPORT bool Transform_null_equals; extern PGDLLIMPORT bool session_variables_ambiguity_warning; extern PGDLLIMPORT bool session_variables_use_fence_warning_guard; +extern PGDLLIMPORT int session_variables_visibility; extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 0b7b69a4159..bb6b9251083 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -248,6 +248,9 @@ struct ParseState bool p_hasModifyingCTE; bool p_hasSessionVariables; + /* true, when the query is executed from PL environment */ + bool p_is_PL; + Node *p_last_srf; /* most recent set-returning func/op found */ /* diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c index f36a244140e..a5749d902e1 100644 --- a/src/pl/plpgsql/src/pl_comp.c +++ b/src/pl/plpgsql/src/pl_comp.c @@ -1101,6 +1101,7 @@ plpgsql_parser_setup(struct ParseState *pstate, PLpgSQL_expr *expr) pstate->p_paramref_hook = plpgsql_param_ref; /* no need to use p_coerce_param_hook */ pstate->p_ref_hook_state = expr; + pstate->p_is_PL = true; } /* diff --git a/src/test/regress/expected/session_variables.out b/src/test/regress/expected/session_variables.out index 9ac08a14e0b..eda4eb20798 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_visibility +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); +SET session_variables_visibility TO 'everywhere'; +-- 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_visibility TO 'nowhere'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: column "var1" does not exist +LINE 1: SELECT var1, VARIABLE(var1); + ^ +-- should fail +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; +END; +$$; +ERROR: column "var1" does not exist +LINE 1: SELECT a FROM vartest_tab1 WHERE a = var1 + ^ +QUERY: SELECT a FROM vartest_tab1 WHERE a = var1 +CONTEXT: 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: column "var1" does not exist +LINE 1: var1 + ^ +QUERY: var1 +CONTEXT: 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_visibility TO 'pl'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: column "var1" does not exist +LINE 1: 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; +$$; +NOTICE: 20 t +NOTICE: 20 t +SET session_variables_visibility 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..08d669f86d2 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_visibility +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); + +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); + +SET session_variables_visibility TO 'everywhere'; + +-- 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_visibility TO 'nowhere'; + +-- 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_visibility TO 'pl'; + +-- 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_visibility TO DEFAULT; + +DROP VARIABLE var1; +DROP TABLE vartest_tab1; -- 2.48.1