... and here is a review for patch 4
I didn't change any code, just added the odd article to a comment.
While running the regression tests with "make installcheck", I noticed two
problems:
--- /home/laurenz/postgresql/src/test/regress/expected/session_variables.out
2024-10-24 11:14:06.717663613 +0300
+++ /home/laurenz/postgresql/src/test/regress/results/session_variables.out
2024-10-24 11:15:37.999286228 +0300
@@ -30,6 +30,7 @@
GRANT ALL ON SCHEMA svartest TO regress_variable_owner;
CREATE VARIABLE svartest.var1 AS int;
CREATE ROLE regress_variable_reader;
+ERROR: role "regress_variable_reader" already exists
I suggest that patch 0001 should drop role "regress_variable_reader" again.
@@ -107,7 +108,7 @@
CONTEXT: SQL function "sqlfx" statement 1
SELECT plpgsqlfx(20);
ERROR: permission denied for session variable var1
-CONTEXT: SQL expression "$1 + var1"
+CONTEXT: PL/pgSQL expression "$1 + var1"
That looks like bit rot from your commit 4af123ad45.
Yours,
Laurenz Albe
From 027fb062ecc0840bc5c2d135ebbc8ddc6b046f96 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Thu, 24 Oct 2024 11:17:12 +0300
Subject: [PATCH v20241024] DISCARD VARIABLES
Implementation of DISCARD VARIABLES commands by removing hash table with session variables
and resetting related memory context.
---
doc/src/sgml/ref/discard.sgml | 13 +++-
src/backend/commands/discard.c | 6 ++
src/backend/commands/session_variable.c | 28 ++++++++-
src/backend/parser/gram.y | 6 ++
src/backend/tcop/utility.c | 3 +
src/bin/psql/tab-complete.in.c | 2 +-
src/include/commands/session_variable.h | 2 +
src/include/nodes/parsenodes.h | 1 +
src/include/tcop/cmdtaglist.h | 1 +
.../regress/expected/session_variables.out | 63 +++++++++++++++++++
src/test/regress/sql/session_variables.sql | 36 +++++++++++
11 files changed, 158 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/ref/discard.sgml b/doc/src/sgml/ref/discard.sgml
index bf44c523cac..61b967f9c9b 100644
--- a/doc/src/sgml/ref/discard.sgml
+++ b/doc/src/sgml/ref/discard.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-DISCARD { ALL | PLANS | SEQUENCES | TEMPORARY | TEMP }
+DISCARD { ALL | PLANS | SEQUENCES | TEMPORARY | TEMP | VARIABLES }
</synopsis>
</refsynopsisdiv>
@@ -66,6 +66,16 @@ DISCARD { ALL | PLANS | SEQUENCES | TEMPORARY | TEMP }
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>VARIABLES</literal></term>
+ <listitem>
+ <para>
+ Resets the value of all session variables. If a variable
+ is later reused, it is re-initialized to <literal>NULL</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>TEMPORARY</literal> or <literal>TEMP</literal></term>
<listitem>
@@ -93,6 +103,7 @@ SELECT pg_advisory_unlock_all();
DISCARD PLANS;
DISCARD TEMP;
DISCARD SEQUENCES;
+DISCARD VARIABLES;
</programlisting></para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/discard.c b/src/backend/commands/discard.c
index 92d983ac748..d06ebaba6f4 100644
--- a/src/backend/commands/discard.c
+++ b/src/backend/commands/discard.c
@@ -18,6 +18,7 @@
#include "commands/async.h"
#include "commands/discard.h"
#include "commands/prepare.h"
+#include "commands/session_variable.h"
#include "commands/sequence.h"
#include "utils/guc.h"
#include "utils/portal.h"
@@ -48,6 +49,10 @@ DiscardCommand(DiscardStmt *stmt, bool isTopLevel)
ResetTempTableNamespace();
break;
+ case DISCARD_VARIABLES:
+ ResetSessionVariables();
+ break;
+
default:
elog(ERROR, "unrecognized DISCARD target: %d", stmt->target);
}
@@ -75,4 +80,5 @@ DiscardAll(bool isTopLevel)
ResetPlanCache();
ResetTempTableNamespace();
ResetSequenceCaches();
+ ResetSessionVariables();
}
diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c
index 19f772a9fb6..eedce691bc0 100644
--- a/src/backend/commands/session_variable.c
+++ b/src/backend/commands/session_variable.c
@@ -94,7 +94,13 @@ pg_variable_cache_callback(Datum arg, int cacheid, uint32 hashvalue)
elog(DEBUG1, "pg_variable_cache_callback %u %u", cacheid, hashvalue);
- Assert(sessionvars);
+ /*
+ * There is no guarantee of session variables being initialized, even when
+ * receiving an invalidation callback, as DISCARD [ ALL | VARIABLES ]
+ * destroys the hash table entirely.
+ */
+ if (!sessionvars)
+ return;
/*
* If the hashvalue is not specified, we have to recheck all currently
@@ -658,3 +664,23 @@ pg_session_variables(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+
+/*
+ * Fast drop of the complete content of the session variables hash table, and
+ * cleanup of any list that wouldn't be relevant anymore.
+ * This is used by the DISCARD VARIABLES (and DISCARD ALL) command.
+ */
+void
+ResetSessionVariables(void)
+{
+ /* destroy hash table and reset related memory context */
+ if (sessionvars)
+ {
+ hash_destroy(sessionvars);
+ sessionvars = NULL;
+ }
+
+ /* release memory allocated by session variables */
+ if (SVariableMemoryContext != NULL)
+ MemoryContextReset(SVariableMemoryContext);
+}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index eeffa561731..929e340ac48 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2096,7 +2096,13 @@ DiscardStmt:
n->target = DISCARD_SEQUENCES;
$$ = (Node *) n;
}
+ | DISCARD VARIABLES
+ {
+ DiscardStmt *n = makeNode(DiscardStmt);
+ n->target = DISCARD_VARIABLES;
+ $$ = (Node *) n;
+ }
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index bdac691af13..84415f0caa1 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2959,6 +2959,9 @@ CreateCommandTag(Node *parsetree)
case DISCARD_SEQUENCES:
tag = CMDTAG_DISCARD_SEQUENCES;
break;
+ case DISCARD_VARIABLES:
+ tag = CMDTAG_DISCARD_VARIABLES;
+ break;
default:
tag = CMDTAG_UNKNOWN;
}
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 172a37ed760..deb9563a598 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -4083,7 +4083,7 @@ match_previous_words(int pattern_id,
/* DISCARD */
else if (Matches("DISCARD"))
- COMPLETE_WITH("ALL", "PLANS", "SEQUENCES", "TEMP");
+ COMPLETE_WITH("ALL", "PLANS", "SEQUENCES", "TEMP", "VARIABLES");
/* DO */
else if (Matches("DO"))
diff --git a/src/include/commands/session_variable.h b/src/include/commands/session_variable.h
index b3f03c65827..443afbafd4a 100644
--- a/src/include/commands/session_variable.h
+++ b/src/include/commands/session_variable.h
@@ -29,4 +29,6 @@ extern Datum GetSessionVariableWithTypeCheck(Oid varid, bool *isNull, Oid expect
extern void ExecuteLetStmt(ParseState *pstate, LetStmt *stmt, ParamListInfo params,
QueryEnvironment *queryEnv, QueryCompletion *qc);
+extern void ResetSessionVariables(void);
+
#endif
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 20ed8394c78..781db0bb303 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3977,6 +3977,7 @@ typedef enum DiscardMode
DISCARD_PLANS,
DISCARD_SEQUENCES,
DISCARD_TEMP,
+ DISCARD_VARIABLES,
} DiscardMode;
typedef struct DiscardStmt
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index a921af2486f..bd7964aea6e 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -135,6 +135,7 @@ PG_CMDTAG(CMDTAG_DISCARD_ALL, "DISCARD ALL", false, false, false)
PG_CMDTAG(CMDTAG_DISCARD_PLANS, "DISCARD PLANS", false, false, false)
PG_CMDTAG(CMDTAG_DISCARD_SEQUENCES, "DISCARD SEQUENCES", false, false, false)
PG_CMDTAG(CMDTAG_DISCARD_TEMP, "DISCARD TEMP", false, false, false)
+PG_CMDTAG(CMDTAG_DISCARD_VARIABLES, "DISCARD VARIABLES", false, false, false)
PG_CMDTAG(CMDTAG_DO, "DO", false, false, false)
PG_CMDTAG(CMDTAG_DROP_ACCESS_METHOD, "DROP ACCESS METHOD", true, false, false)
PG_CMDTAG(CMDTAG_DROP_AGGREGATE, "DROP AGGREGATE", true, false, false)
diff --git a/src/test/regress/expected/session_variables.out b/src/test/regress/expected/session_variables.out
index 16e1d01ddc6..bcc8c8d847d 100644
--- a/src/test/regress/expected/session_variables.out
+++ b/src/test/regress/expected/session_variables.out
@@ -977,3 +977,66 @@ DROP VARIABLE :"DBNAME".:"DBNAME".b;
DROP VARIABLE :"DBNAME".:"DBNAME".:"DBNAME";
DROP SCHEMA :"DBNAME";
RESET search_path;
+-- memory cleaning by DISCARD command
+CREATE VARIABLE var1 AS varchar;
+LET var1 = 'Hello';
+SELECT var1;
+ var1
+-------
+ Hello
+(1 row)
+
+DISCARD ALL;
+SELECT var1;
+ var1
+------
+
+(1 row)
+
+LET var1 = 'AHOJ';
+SELECT var1;
+ var1
+------
+ AHOJ
+(1 row)
+
+DISCARD VARIABLES;
+SELECT var1;
+ var1
+------
+
+(1 row)
+
+DROP VARIABLE var1;
+-- initial test of debug pg_session_variables function
+-- should be zero now
+DISCARD VARIABLES;
+SELECT count(*) FROM pg_session_variables();
+ count
+-------
+ 0
+(1 row)
+
+CREATE VARIABLE var1 AS varchar;
+-- should be zero still
+SELECT count(*) FROM pg_session_variables();
+ count
+-------
+ 0
+(1 row)
+
+LET var1 = 'AHOJ';
+SELECT name, typname, can_select, can_update FROM pg_session_variables();
+ name | typname | can_select | can_update
+------+-------------------+------------+------------
+ var1 | character varying | t | t
+(1 row)
+
+DISCARD VARIABLES;
+-- should be zero again
+SELECT count(*) FROM pg_session_variables();
+ count
+-------
+ 0
+(1 row)
+
diff --git a/src/test/regress/sql/session_variables.sql b/src/test/regress/sql/session_variables.sql
index d5bb5dc83be..d4165f34ffa 100644
--- a/src/test/regress/sql/session_variables.sql
+++ b/src/test/regress/sql/session_variables.sql
@@ -696,3 +696,39 @@ DROP VARIABLE :"DBNAME".:"DBNAME".:"DBNAME";
DROP SCHEMA :"DBNAME";
RESET search_path;
+
+-- memory cleaning by DISCARD command
+CREATE VARIABLE var1 AS varchar;
+LET var1 = 'Hello';
+SELECT var1;
+
+DISCARD ALL;
+SELECT var1;
+
+LET var1 = 'AHOJ';
+SELECT var1;
+
+DISCARD VARIABLES;
+SELECT var1;
+
+DROP VARIABLE var1;
+
+-- initial test of debug pg_session_variables function
+-- should be zero now
+DISCARD VARIABLES;
+
+SELECT count(*) FROM pg_session_variables();
+
+CREATE VARIABLE var1 AS varchar;
+
+-- should be zero still
+SELECT count(*) FROM pg_session_variables();
+
+LET var1 = 'AHOJ';
+
+SELECT name, typname, can_select, can_update FROM pg_session_variables();
+
+DISCARD VARIABLES;
+
+-- should be zero again
+SELECT count(*) FROM pg_session_variables();
--
2.47.0
