[PATCH v20220922 04/13] FIXUP 0003-session-variables
Julien Rouhaud <julien.rouhaud@free.fr>
From: Julien Rouhaud <julien.rouhaud@free.fr>
To:
Date: 2022-09-15T15:33:42Z
Lists: pgsql-hackers
---
src/backend/commands/session_variable.c | 561 +++++++++++++-----------
src/include/commands/session_variable.h | 4 +-
2 files changed, 311 insertions(+), 254 deletions(-)
diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c
index d34f193284..53b0fe58a0 100644
--- a/src/backend/commands/session_variable.c
+++ b/src/backend/commands/session_variable.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * src/backend/commands/sessionvariable.c
+ * src/backend/commands/session_variable.c
*
*-------------------------------------------------------------------------
*/
@@ -36,62 +36,57 @@
* Values of session variables are stored in the backend local memory,
* in sessionvars hash table in binary format, in a dedicated memory
* context SVariableMemoryContext. A session variable value can stay
- * valid for longer than a transaction. To make sure that the
- * underlying memory is freed, we need to solve following issues:
+ * valid for longer than the transaction that assigns its value. To
+ * make sure that the underlying memory is eventually freed, but not
+ * before it's guarantee that the value won't be needed anymore, we
+ * need to handle the two following points:
*
- * - We need to free local memory when the variable is droppedn,
- * whether in the current transaction in the current session or by
- * other sessions (other users). To protect the content against
- * possibly rollbacked DROP VARIABLE commands, the memory shouldn't
- * be freed immediately but be postponed until the end of the
- * transaction.
+ * - We need detect when a variable is dropped, whether in the current
+ * transaction in the current session or by another session, and mark
+ * the underlying entries for removal. To protect the content against
+ * possibly rollbacked DROP VARIABLE commands, the entries (and
+ * memory) shouldn't be freed immediately but be postponed until the
+ * end of the transaction.
*
* - The session variable can be dropped explicitly (by DROP VARIABLE
* command) or implicitly (using ON COMMIT DROP clause), and the
* value can be implicitly removed (using the ON TRANSACTION END
- * clause). In all those cases the memory should be freed at the
- * transaction end.
+ * clause). In all those cases the memory should also be freed at
+ * the transaction end.
*
- * To achieve that, we maintain 4 queues (implemented as list
- * of variable oid, list of RecheckVariableRequests
- * (requests are created in reaction on sinval) and lists
- * of actions). List of actions are used whe we need to calculate
- * the final transaction state of an entry's creator's
- * subtransaction. List of session variables are used, when we
- * don't need to do this calculation (we know, so. every values
- * of session variables created with clauses ON COMMIT DROP or
- * ON TRANSACTION END RESET will be purged from memory), but
- * ON COMMIT DROP action or ON COMMIT RESET action can be done only
- * when related sub transaction is committed (we don't want to lost
- * value after aborted DROP VARIABLE command, we don't want to delete
- * record from system catalog after aborted CREATE VARIABLE command,
- * or we don't want to drop record from system catalog implicitly
- * when TEMP ON COMMIT DROP variable was successfully dropped).
- * Recheck of session's variable existence should be executed
- * only when some operation over session's variable is executed
- * first time in transaction or at the end of transaction. It
- * should not be executed more times inside transaction.
+ * To achieve that, we maintain 3 queues of actions to be performed at
+ * certain time:
+ * - a List of SVariableXActActionItem, to handle ON COMMIT DROP
+ * variables, and delayed memory cleanup of variable dropped by the
+ * current transaction. Those actions are transactional (for instance
+ * we don't want to cleanup the memory of a rollbacked DROP VARIABLE)
+ * so the structure is needed to keep track of the final transaction
+ * state
+ * - a List of variable Oid for the ON TRANSACTION ON RESET variables
+ * - a List of variable Oid for the concurrent DROP VARIABLE
+ * notification we receive via shared invalidations.
*
- * Although the variable's reset doesn't need full delete from
- * sessionvars hash table, we do. It is the most simple solution,
- * and it reduces possible state's space (and reduces sessionvars
- * bloating).
+ * Note that although resetting a variable doesn't technically require
+ * to remove the entry from the sessionvars hash table, we currently
+ * do it. It's a simple way to implement the reset, and helps to
+ * reduce memory usage and prevents the hash table from bloating.
*
- * There are two different ways to do final access to session
+ * There are two different ways to do the final access to session
* variables: buffered (indirect) or direct. Buffered access is used
- * in queries, where we have to ensure an stability of passed values
- * (then the session variable has same behaviour like external query
- * parameters, what is consistent with using PL/pgSQL's variables in
- * embedded queries in PL/pgSQL).
+ * in regular DML statements, where we have to ensure the stability of
+ * the variable values. The session variables have the same behaviour
+ * as external query parameters, which is consistent with using
+ * PL/pgSQL's variables in embedded queries in PL/pgSQL.
*
* This is implemented by using an aux buffer (an array) that holds a
- * copy of values of used (in query) session variables. In the final
- * end, the values from this array are passed as constant (EEOP_CONST).
+ * copy of values of used (in query) session variables, which is also
+ * transmitted to the parallel workers. The values from this array
+ * are passed as constant (EEOP_CONST).
*
* Direct access is used by simple expression evaluation (PLpgSQL).
* In this case we don't need to ensure the stability of passed
* values, and maintaining the buffer with copies of values of session
- * variables can be useless overhead. In this case we just read the
+ * variables would be useless overhead. In this case we just read the
* value of the session variable directly (EEOP_PARAM_VARIABLE). This
* strategy removes the necessity to modify related PL/pgSQL code to
* support session variables (the reading of session variables is
@@ -109,20 +104,21 @@ typedef struct SVariableXActActionItem
SVariableXActAction action;
/*
- * creating_subid is the ID of the creating subxact. If the action was
- * unregistered during the current transaction, deleting_subid is the ID
- * of the deleting subxact, otherwise InvalidSubTransactionId.
+ * creating_subid is the ID of the sub-transaction that registered
+ * the action. If the action was unregistered during the current
+ * transaction, deleting_subid is the ID of the deleting
+ * sub-transaction, otherwise InvalidSubTransactionId.
*/
SubTransactionId creating_subid;
SubTransactionId deleting_subid;
} SVariableXActActionItem;
-/* list holds fields of SVariableXActActionItem type */
+/* List of SVariableXActActionItem */
static List *xact_on_commit_actions = NIL;
/*
- * the ON COMMIT DROP and ON TRANSACTION END RESET variables
- * are purged from memory every time.
+ * To process ON TRANSACTION END RESET variables, for which we always
+ * need to clear the saved values.
*/
static List *xact_reset_varids = NIL;
@@ -134,25 +130,28 @@ static List *xact_reset_varids = NIL;
* hold list of possibly dropped session variables and at the end of
* transaction, we check session variables from this list against system
* catalog. This check can be postponed into next transaction if
- * current transactions is in aborted state.
+ * current transactions is in aborted state, as we wouldn't be able to
+ * access the system catalog.
*/
static List *xact_recheck_varids = NIL;
typedef struct SVariableData
{
- Oid varid; /* pg_variable OID of this sequence (hash key) */
+ Oid varid; /* pg_variable OID of this sequence
+ (hash key) */
/*
* The session variable is identified by oid. The oid is unique in
- * catalog. Unfortunately, the cleaning memory can be postponed to begin
- * of next transaction in session, and theoreticaly, there can be
- * different session variable with same oid. So we need another extra
- * identifier that helps with identity check. We can use LSN number in
- * session variable create time. The value of session variable (in memory)
- * is valid, when there is an record in pg_variable with same oid and same
- * create_lsn.
+ * catalog. Unfortunately, the memory cleanup can be postponed to
+ * the beginning
+ * of the session next transaction, and it's possible that this next
+ * transaction sees a different variable with the same oid. We
+ * therefore need an extra identifier to distinguish both cases. We
+ * use the LSN number of session variable at creation time. The
+ * value of session variable (in memory) is valid, when there is a
+ * record in pg_variable with same oid and same create_lsn.
*/
- XLogRecPtr create_lsn; /* used for session's variable identity check */
+ XLogRecPtr create_lsn;
bool isnull;
bool freeval;
@@ -169,7 +168,7 @@ typedef struct SVariableData
/*
* Top level local transaction id of the last transaction that dropped the
* variable if any. We need this information to avoid freeing memory for
- * variable dropped by the local backend that may be rollbacked.
+ * variable dropped by the local backend that may be eventually rollbacked.
*/
LocalTransactionId drop_lxid;
@@ -189,22 +188,33 @@ typedef struct SVariableData
typedef SVariableData *SVariable;
static HTAB *sessionvars = NULL; /* hash table for session variables */
-static HTAB *sessionvars_types = NULL; /* hash table for type fingerprints of
- * session variables */
static MemoryContext SVariableMemoryContext = NULL;
+static void create_sessionvars_hashtables(void);
+static void free_session_variable_value(SVariable svar);
static void init_session_variable(SVariable svar, Variable *var);
-
-static void register_session_variable_xact_action(Oid varid, SVariableXActAction action);
-static void unregister_session_variable_xact_action(Oid varid, SVariableXActAction action);
+static bool is_session_variable_valid(SVariable svar);
+static void pg_variable_cache_callback(Datum arg, int cacheid,
+ uint32 hashvalue);
+static SVariable prepare_variable_for_reading(Oid varid);
+static void register_session_variable_xact_action(Oid varid,
+ SVariableXActAction action);
+static void remove_session_variable(SVariable svar);
+static void remove_session_variable_by_id(Oid varid);
+static void set_session_variable(SVariable svar, Datum value, bool isnull,
+ bool init_mode);
+static const char *SVariableXActActionName(SVariableXActAction action);
+static void sync_sessionvars_all(void);
+static void unregister_session_variable_xact_action(Oid varid,
+ SVariableXActAction action);
/*
* Returns human readable name of SVariableXActAction value.
*/
static const char *
-SvariableXActActionName(SVariableXActAction action)
+SVariableXActActionName(SVariableXActAction action)
{
switch (action)
{
@@ -213,15 +223,14 @@ SvariableXActActionName(SVariableXActAction action)
case SVAR_ON_COMMIT_RESET:
return "ON COMMIT RESET";
default:
- elog(ERROR, "unknown SVariableXActAction action %d",
- action);
+ elog(ERROR, "unknown SVariableXActAction action %d", action);
}
}
/*
- * Releases stored data from session variable, but preserve the hash entry
- * in sessionvars. When we replace row value by new value with same type
- * fingerprint, we can keep field desc data.
+ * Free all memory allocated for the given session variable, but
+ * preserve the hash entry in sessionvars.
+ * This function shouldn't contain anything that could ereport(ERROR).
*/
static void
free_session_variable_value(SVariable svar)
@@ -245,8 +254,65 @@ free_session_variable_value(SVariable svar)
}
/*
- * Release the variable defined by varid from sessionvars
- * hashtab.
+ * Registration of actions to be executed on session variables at transaction
+ * end time. We want to drop temporary session variables with clause ON COMMIT
+ * DROP, or we want to clean (reset) local memory allocated by
+ * values of session variables dropped in other backends.
+ */
+static void
+register_session_variable_xact_action(Oid varid,
+ SVariableXActAction action)
+{
+ SVariableXActActionItem *xact_ai;
+ MemoryContext oldcxt;
+
+ elog(DEBUG1, "SVariableXActAction \"%s\" is registered for session variable (oid:%u)",
+ SVariableXActActionName(action), varid);
+
+ oldcxt = MemoryContextSwitchTo(TopTransactionContext);
+
+ xact_ai = (SVariableXActActionItem *)
+ palloc(sizeof(SVariableXActActionItem));
+
+ xact_ai->varid = varid;
+ xact_ai->action = action;
+
+ xact_ai->creating_subid = GetCurrentSubTransactionId();
+ xact_ai->deleting_subid = InvalidSubTransactionId;
+
+ xact_on_commit_actions = lcons(xact_ai, xact_on_commit_actions);
+
+ MemoryContextSwitchTo(oldcxt);
+}
+
+/*
+ * Unregister an action on a given session variable from the action list.
+ * The action is just marked as deleted by setting deleting_subid.
+ * The calling subtransaction even might be rollbacked, in which case the
+ * action shouldn't be removed.
+ */
+static void
+unregister_session_variable_xact_action(Oid varid,
+ SVariableXActAction action)
+{
+ ListCell *l;
+
+ elog(DEBUG1, "SVariableXActAction \"%s\" is unregistered for session variable (oid:%u)",
+ SVariableXActActionName(action), varid);
+
+ foreach(l, xact_on_commit_actions)
+ {
+ SVariableXActActionItem *xact_ai =
+ (SVariableXActActionItem *) lfirst(l);
+
+ if (xact_ai->action == action && xact_ai->varid == varid)
+ xact_ai->deleting_subid = GetCurrentSubTransactionId();
+ }
+}
+
+/*
+ * Release the given session variable from sessionvars hashtab and free
+ * all underlying allocated memory.
*/
static void
remove_session_variable(SVariable svar)
@@ -268,8 +334,8 @@ remove_session_variable(SVariable svar)
}
/*
- * Release the variable defined by varid from sessionvars
- * hashtab.
+ * Release the session variable defined by varid from sessionvars
+ * hashtab and free all underlying allocated memory.
*/
static void
remove_session_variable_by_id(Oid varid)
@@ -288,6 +354,8 @@ remove_session_variable_by_id(Oid varid)
/*
* Callback function for session variable invalidation.
+ *
+ * It queues a list of variable Oid in xact_recheck_varids.
*/
static void
pg_variable_cache_callback(Datum arg, int cacheid, uint32 hashvalue)
@@ -308,8 +376,8 @@ pg_variable_cache_callback(Datum arg, int cacheid, uint32 hashvalue)
/*
* When the hashvalue is not specified, then we have to recheck all
* currently used session variables. Since we can't guarantee the exact
- * session variable from its hashValue, we have to iterate over all items
- * of hash table.
+ * session variable from its hashValue, we also have to iterate over
+ * all items of the sessionvars hash table.
*/
hash_seq_init(&status, sessionvars);
@@ -332,14 +400,15 @@ pg_variable_cache_callback(Datum arg, int cacheid, uint32 hashvalue)
/*
* although it there is low probability, we have to iterate over all
- * locally set session variables, because hashvalue is not unique
+ * locally set session variables, because hashvalue is not a unique
* identifier.
*/
}
}
/*
- * Returns true, when the entry in pg_variable is valid for SVariable
+ * Returns true when the entry in pg_variable is valid for the given session
+ * variable.
*/
static bool
is_session_variable_valid(SVariable svar)
@@ -353,8 +422,9 @@ is_session_variable_valid(SVariable svar)
{
/*
* In this case, the only oid cannot be used as unique identifier,
- * because after oid's reset the oid can be used for new other session
- * variable. We do second check against 64bit unique identifier.
+ * because the oid counter can wraparound, and the oid can be used for
+ * new other session variable. We do a second check against 64bit
+ * unique identifier.
*/
if (svar->create_lsn == ((Form_pg_variable) GETSTRUCT(tp))->create_lsn)
result = true;
@@ -366,12 +436,12 @@ is_session_variable_valid(SVariable svar)
}
/*
- * The possible invalidated variables (in memory) are checked
- * against system catalog. This routine is called before
- * any read or any write from/to session variable.
+ * Recheck the possibly invalidated variables (in memory) against system
+ * catalog. This routine is called before any read or any write from/to session
+ * variables.
*/
static void
-sync_sessionvars_all()
+sync_sessionvars_all(void)
{
SVariable svar;
ListCell *l;
@@ -380,8 +450,9 @@ sync_sessionvars_all()
return;
/*
- * sessionvars is null after DISCARD VARIABLES. When we are sure, so there
- * are not any active session variable in this session.
+ * If the sessionvars hashtable is NULL (which can be done by DISCARD
+ * VARIABLES), we are sure that there aren't any active session variable
+ * in this session.
*/
if (!sessionvars)
{
@@ -394,7 +465,7 @@ sync_sessionvars_all()
/*
* This routine is called before any reading, so the session should be in
- * transaction state. This is required for access to system catalog.
+ * transaction state. This is required to access the system catalog.
*/
Assert(IsTransactionState());
@@ -408,9 +479,10 @@ sync_sessionvars_all()
/*
* Remove invalid variables, but don't touch variables that were
- * dropped by the current top level local transaction, as there's no
- * guarantee that the transaction will be committed. Such variables
- * will be removed in the next transaction if needed.
+ * dropped by the current top level local transaction or any
+ * subtransaction underneath, as there's no guarantee that the
+ * transaction will be committed. Such variables will be removed in
+ * the next transaction if needed.
*/
if (found)
{
@@ -430,7 +502,7 @@ sync_sessionvars_all()
}
/*
- * Create the hash table for storing session variables
+ * Create the hash table for storing session variables.
*/
static void
create_sessionvars_hashtables(void)
@@ -471,12 +543,11 @@ create_sessionvars_hashtables(void)
* Assign some content to the session variable. It's copied to
* SVariableMemoryContext if necessary.
*
- * init_mode is true, when the value of session variable is being initialized
- * by default expression or by null. Only in this moment we can allow to
- * modify immutable variables with default expression.
+ * init_mode is true when the value of session variable should be initialized
+ * by the default expression if any. This is the only case where we allow the
+ * modification of an immutable variables with default expression.
*
- * This routine have to do successfull change or leave memory without
- * change.
+ * If any error happens, the existing value shouldn't be modified.
*/
static void
set_session_variable(SVariable svar, Datum value, bool isnull, bool init_mode)
@@ -494,10 +565,10 @@ set_session_variable(SVariable svar, Datum value, bool isnull, bool init_mode)
get_session_variable_name(svar->varid))));
/*
- * Don't allow updating of immutable session variable that has assigned
- * not null value or has default expression (and then the value should be
- * result of default expression always). Don't do this check, when
- * variable is being initialized.
+ * Don't allow the modification of an immutable session variable that
+ * already has an assigned value (possibly NULL) or has a default
+ * expression (in which case the value should always be the result of
+ * default expression evaluation) unless the variable is being initialized.
*/
if (!init_mode &&
(svar->is_immutable &&
@@ -516,6 +587,16 @@ set_session_variable(SVariable svar, Datum value, bool isnull, bool init_mode)
MemoryContextSwitchTo(oldcxt);
}
+ else
+ {
+ /* The caller shouldn't have provided any real value. */
+ Assert(value == (Datum) 0);
+ }
+
+ /*
+ * No error should happen after this poiht, otherwise we could leak the
+ * newly allocated value if any.
+ */
free_session_variable_value(svar);
@@ -525,6 +606,13 @@ set_session_variable(SVariable svar, Datum value, bool isnull, bool init_mode)
svar->freeval = newval != value;
svar->is_valid = true;
+ /*
+ * XXX While unlikely, an error here is possible.
+ * It wouldn't leak memory as the allocated chunk has already been
+ * correctly assigned to the session variable, but would contradict this
+ * function contract, which is that this function should either succeed or
+ * leave the current value untouched.
+ */
elog(DEBUG1, "session variable \"%s.%s\" (oid:%u) has new value",
get_namespace_name(get_session_variable_namespace(svar->varid)),
get_session_variable_name(svar->varid),
@@ -532,9 +620,7 @@ set_session_variable(SVariable svar, Datum value, bool isnull, bool init_mode)
}
/*
- * Initialize svar from var
- * svar - SVariable - holds value
- * var - Variable - holds metadata
+ * Initialize session variable svar from variable var
*/
static void
init_session_variable(SVariable svar, Variable *var)
@@ -580,8 +666,8 @@ init_session_variable(SVariable svar, Variable *var)
}
/*
- * Search the given session variable in the hash table. If it doesn't
- * exist, then insert it (and calculate defexpr if it exists).
+ * Search a seesion variable in the hash table given its oid. If it
+ * doesn't exist, then insert it (and calculate defexpr if it exists).
*
* Caller is responsible for doing permission checks.
*
@@ -600,12 +686,12 @@ prepare_variable_for_reading(Oid varid)
if (!sessionvars)
create_sessionvars_hashtables();
- /* Ensure so all entries in sessionvars hash table are valid */
- sync_sessionvars_all();
-
/* Protect used session variable against drop until transaction end */
LockDatabaseObject(VariableRelationId, varid, 0, AccessShareLock);
+ /* Make sure that all entries in sessionvars hash table are valid */
+ sync_sessionvars_all();
+
svar = (SVariable) hash_search(sessionvars, &varid,
HASH_ENTER, &found);
@@ -625,7 +711,10 @@ prepare_variable_for_reading(Oid varid)
varid);
}
- /* Raise an error when we cannot initialize variable correctly */
+ /*
+ * Raise an error if this is a NOT NULL variable without default
+ * expression.
+ */
if (var.is_not_null && !var.defexpr)
ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
@@ -668,16 +757,18 @@ prepare_variable_for_reading(Oid varid)
/*
* Although the value of domain type should be valid (it is checked when
* it is assigned to session variable), we have to check related
- * constraints anytime. It can be more expensive than in PL/pgSQL.
- * PL/pgSQL forces domain checks when value is assigned to the variable or
- * when value is returned from function. Fortunately, domain types manage
- * cache of constraints by self.
+ * constraints each time we access the variable. It can be more expensive
+ * than in PL/pgSQL, as PL/pgSQL forces domain checks only when the value is assigned
+ * to the variable or when the value is returned from function.
+ * However, domain types have a constraint cache so it's not too much
+ * expensive..
*/
if (svar->is_domain)
{
/*
* Store domain_check extra in TopTransactionContext. When we are in
- * other transaction, the domain_check_extra cache is not valid.
+ * other transaction, the domain_check_extra cache is not valid
+ * anymore.
*/
if (svar->domain_check_extra_lxid != MyProc->lxid)
svar->domain_check_extra = NULL;
@@ -696,7 +787,6 @@ prepare_variable_for_reading(Oid varid)
* Store the given value in an SVariable, and cache it if not already present.
*
* Caller is responsible for doing permission checks.
- * We try not to break the previous value, if something is wrong.
*
* As side effect this function acquires AccessShareLock on
* related session variable until the end of the transaction.
@@ -710,12 +800,12 @@ SetSessionVariable(Oid varid, Datum value, bool isNull)
if (!sessionvars)
create_sessionvars_hashtables();
- /* Ensure so all entries in sessionvars hash table are valid */
- sync_sessionvars_all();
-
/* Protect used session variable against drop until transaction end */
LockDatabaseObject(VariableRelationId, varid, 0, AccessShareLock);
+ /* Ensure so all entries in sessionvars hash table are valid */
+ sync_sessionvars_all();
+
svar = (SVariable) hash_search(sessionvars, &varid,
HASH_ENTER, &found);
@@ -733,6 +823,10 @@ SetSessionVariable(Oid varid, Datum value, bool isNull)
varid);
}
+ /*
+ * This should either succeed or fail without changing the currently stored
+ * value.
+ */
set_session_variable(svar, value, isNull, false);
}
@@ -745,7 +839,7 @@ SetSessionVariableWithSecurityCheck(Oid varid, Datum value, bool isNull)
AclResult aclresult;
/*
- * Is possible to write to session variable?
+ * Is caller allowed to update the session variable?
*/
aclresult = pg_variable_aclcheck(varid, GetUserId(), ACL_UPDATE);
if (aclresult != ACLCHECK_OK)
@@ -755,7 +849,7 @@ SetSessionVariableWithSecurityCheck(Oid varid, Datum value, bool isNull)
}
/*
- * Returns a copy of value of the session variable specified by varid
+ * Returns a copy of the value of the session variable specified by its oid.
* Caller is responsible for doing permission checks.
*/
Datum
@@ -769,7 +863,7 @@ CopySessionVariable(Oid varid, bool *isNull, Oid *typid)
*typid = svar->typid;
- /* force copy of not null value */
+ /* force copy of non NULL value */
if (!svar->isnull)
{
result = datumCopy(svar->value, svar->typbyval, svar->typlen);
@@ -785,9 +879,9 @@ CopySessionVariable(Oid varid, bool *isNull, Oid *typid)
}
/*
- * Returns a copy of value of the session variable specified by varid
- * with check of expected type. Like previous function, the caller
- * is responsible for doing permission checks.
+ * Returns a copy of ths value of the session variable specified by its oid
+ * with a check of the expected type. Like previous CopySessionVariable, the
+ * caller is responsible for doing permission checks.
*/
Datum
CopySessionVariableWithTypeCheck(Oid varid, bool *isNull, Oid expected_typid)
@@ -818,9 +912,9 @@ CopySessionVariableWithTypeCheck(Oid varid, bool *isNull, Oid expected_typid)
}
/*
- * Returns a value of session variable identified by varid with
- * check of expected type. Like previous function, the called
- * is reposible for doing permission check.
+ * Returns the value of the session variable specified by its oid with a check
+ * of the expected type. Like CopySessionVariable, the caller is responsible
+ * for doing permission checks.
*/
Datum
GetSessionVariableWithTypeCheck(Oid varid, bool *isNull, Oid expected_typid)
@@ -894,15 +988,15 @@ SessionVariableDropPostprocess(Oid varid)
svar->drop_lxid = MyProc->lxid;
/*
- * For variables that are not purged by default we need to
+ * For variables that are not ON TRANSACTION END RESET, we need to
* register an SVAR_ON_COMMIT_RESET action to free the local
- * memory for this variable when this transaction or
- * subtransaction is committed (we don't need to wait for sinval
- * message). The cleaning action for one session variable can be
- * repeated in the action list without causing any problem, so we
+ * memory for this variable when the top level transaction
+ * is committed (we don't need to wait for sinval
+ * message). The cleanup action for one session variable can be
+ * duplicated in the action list without causing any problem, so we
* don't need to ensure uniqueness. We need a different action
* from RESET, because RESET is executed on any transaction end,
- * but we want to execute cleaning only when the current
+ * but we want to execute this cleanup only when the current
* transaction will be committed. This action can be reverted by
* ABORT of DROP VARIABLE command.
*/
@@ -914,10 +1008,9 @@ SessionVariableDropPostprocess(Oid varid)
}
/*
- * Fast drop of the complete content of all session variables hash table.
- * This is code for DISCARD VARIABLES command. This command
- * cannot be run inside transaction, so we don't need to handle
- * end of transaction actions.
+ * Fast drop of the complete content of all session variables hash table, and
+ * cleanup of any list that wouldn't be relevant anymore.
+ * This is used by DISCARD VARIABLES (and DISCARD ALL) command.
*/
void
ResetSessionVariables(void)
@@ -927,9 +1020,6 @@ ResetSessionVariables(void)
{
hash_destroy(sessionvars);
sessionvars = NULL;
-
- hash_destroy(sessionvars_types);
- sessionvars_types = NULL;
}
/* Release memory allocated by session variables */
@@ -953,65 +1043,10 @@ ResetSessionVariables(void)
}
/*
- * Registration of actions to be executed on session variables at transaction
- * end time. We want to drop temporary session variables with clause ON COMMIT
- * DROP, or we want to clean (reset) local memory allocated by
- * values of dropped session variables.
- */
-static void
-register_session_variable_xact_action(Oid varid,
- SVariableXActAction action)
-{
- SVariableXActActionItem *xact_ai;
- MemoryContext oldcxt;
-
- elog(DEBUG1, "SVariableXActAction \"%s\" is registered for session variable (oid:%u)",
- SvariableXActActionName(action), varid);
-
- oldcxt = MemoryContextSwitchTo(TopTransactionContext);
-
- xact_ai = (SVariableXActActionItem *)
- palloc(sizeof(SVariableXActActionItem));
-
- xact_ai->varid = varid;
- xact_ai->action = action;
-
- xact_ai->creating_subid = GetCurrentSubTransactionId();
- xact_ai->deleting_subid = InvalidSubTransactionId;
-
- xact_on_commit_actions = lcons(xact_ai, xact_on_commit_actions);
-
- MemoryContextSwitchTo(oldcxt);
-}
-
-/*
- * Unregister an action on a given session variable from action list. In this
- * moment, the action is just marked as deleted by setting deleting_subid. The
- * calling even might be rollbacked, in which case we should not lose this
- * action.
- */
-static void
-unregister_session_variable_xact_action(Oid varid,
- SVariableXActAction action)
-{
- ListCell *l;
-
- elog(DEBUG1, "SVariableXActAction \"%s\" is unregistered for session variable (oid:%u)",
- SvariableXActActionName(action), varid);
-
- foreach(l, xact_on_commit_actions)
- {
- SVariableXActActionItem *xact_ai =
- (SVariableXActActionItem *) lfirst(l);
-
- if (xact_ai->action == action && xact_ai->varid == varid)
- xact_ai->deleting_subid = GetCurrentSubTransactionId();
- }
-}
-
-/*
- * Perform ON TRANSACTION END RESET or ON COMMIT DROP
- * and COMMIT/ROLLBACK of transaction session variables.
+ * Perform the necessary work for ON TRANSACTION END RESET and ON COMMIT DROP
+ * session variables.
+ * If the transaction is committed, also process the delayed memory cleanup of
+ * local DROP VARIABLE and process all pending rechecks.
*/
void
AtPreEOXact_SessionVariable_on_xact_actions(bool isCommit)
@@ -1019,15 +1054,15 @@ AtPreEOXact_SessionVariable_on_xact_actions(bool isCommit)
ListCell *l;
/*
- * Clean memory for all eox_reset variables. Do it first, it reduces
- * enhancing action lists about RECHECK action.
+ * Clean memory for all ON TRANSACTION END RESET variables. Do it first,
+ * as it reduces the overhead of the RECHECK action list.
*/
foreach(l, xact_reset_varids)
{
remove_session_variable_by_id(lfirst_oid(l));
}
- /* We can clean xact_reset_varids */
+ /* We can now clean xact_reset_varids */
list_free(xact_reset_varids);
xact_reset_varids = NIL;
@@ -1036,50 +1071,57 @@ AtPreEOXact_SessionVariable_on_xact_actions(bool isCommit)
foreach(l, xact_on_commit_actions)
{
SVariableXActActionItem *xact_ai =
- (SVariableXActActionItem *) lfirst(l);
+ (SVariableXActActionItem *) lfirst(l);
/* Iterate only over entries that are still pending */
- if (xact_ai->deleting_subid == InvalidSubTransactionId)
+ if (xact_ai->deleting_subid != InvalidSubTransactionId)
+ continue;
+
+ /*
+ * ON COMMIT DROP is allowed only for temp session variables.
+ * So we should explicitly delete only when the current
+ * transaction is committed. When it's rollbacked, the session
+ * variable is removed automatically.
+ */
+ if (xact_ai->action == SVAR_ON_COMMIT_DROP)
{
- if (xact_ai->action == SVAR_ON_COMMIT_DROP)
- {
- ObjectAddress object;
-
- /*
- * ON COMMIT DROP is allowed only for temp session variables.
- * So we should explicitly delete only when current
- * transaction was committed. When it's rollback, then session
- * variable is removed automatically.
- */
-
- object.classId = VariableRelationId;
- object.objectId = xact_ai->varid;
- object.objectSubId = 0;
-
- /*
- * Since this is an automatic drop, rather than one directly
- * initiated by the user, we pass the
- * PERFORM_DELETION_INTERNAL flag.
- */
- elog(DEBUG1, "session variable (oid:%u) will be deleted (forced by SVAR_ON_COMMIT_DROP action)",
- xact_ai->varid);
-
- performDeletion(&object, DROP_CASCADE,
- PERFORM_DELETION_INTERNAL |
- PERFORM_DELETION_QUIETLY);
- }
- else
- {
- /*
- * When we process DROP VARIABLE statement, we create
- * SVAR_ON_COMMIT_RESET xact action. We want to process this
- * action only when related transaction is commited (when DROP
- * VARIABLE statement sucessfully processed). We want to preserve
- * variable content, when the transaction with DROP VARAIBLE
- * statement was reverted.
- */
- remove_session_variable_by_id(xact_ai->varid);
- }
+ ObjectAddress object;
+
+ object.classId = VariableRelationId;
+ object.objectId = xact_ai->varid;
+ object.objectSubId = 0;
+
+ /*
+ * Since this is an automatic drop, rather than one directly
+ * initiated by the user, we pass the
+ * PERFORM_DELETION_INTERNAL flag.
+ */
+ elog(DEBUG1, "session variable (oid:%u) will be deleted (forced by SVAR_ON_COMMIT_DROP action)",
+ xact_ai->varid);
+
+ /*
+ * If the variable was locally set, the memory will be
+ * automatically cleaned up when we process the underlying
+ * shared invalidation for this drop. There can't be a recheck
+ * action for this variable, so there's nothing to gain
+ * explicitly removing it here.
+ */
+ performDeletion(&object, DROP_CASCADE,
+ PERFORM_DELETION_INTERNAL |
+ PERFORM_DELETION_QUIETLY);
+ }
+ else
+ {
+ /*
+ * When we process DROP VARIABLE statement issued by the
+ * current transaction, we create an SVAR_ON_COMMIT_RESET xact
+ * action. We want to process this action only when related
+ * transaction is commited (when DROP VARIABLE statement
+ * sucessfully processed) as we need to preserve the variable
+ * content if the transaction that issued the DROP VARAIBLE
+ * statement is rollbacked.
+ */
+ remove_session_variable_by_id(xact_ai->varid);
}
}
}
@@ -1091,6 +1133,13 @@ AtPreEOXact_SessionVariable_on_xact_actions(bool isCommit)
list_free_deep(xact_on_commit_actions);
xact_on_commit_actions = NIL;
+ /*
+ * We process the list of recheck last for performance reason,the previous
+ * steps might remove entries from the hash table.
+ * We need catalog access to process the recheck, so this can only be done
+ * if the transaction is committed. Otherwise, we just keep the recheck
+ * list as-is and it will be processed at the next (committed) transaction.
+ */
if (isCommit && xact_recheck_varids)
{
Assert(sessionvars);
@@ -1117,14 +1166,15 @@ AtPreEOXact_SessionVariable_on_xact_actions(bool isCommit)
}
/*
- * Post-subcommit or post-subabort cleanup of xact action list.
+ * Post-subcommit or post-subabort cleanup of xact_on_commit_actions list.
*
* During subabort, we can immediately remove entries created during this
* subtransaction. During subcommit, just transfer entries marked during
* this subtransaction as being the parent's responsibility.
*/
void
-AtEOSubXact_SessionVariable_on_xact_actions(bool isCommit, SubTransactionId mySubid,
+AtEOSubXact_SessionVariable_on_xact_actions(bool isCommit,
+ SubTransactionId mySubid,
SubTransactionId parentSubid)
{
ListCell *cur_item;
@@ -1134,19 +1184,24 @@ AtEOSubXact_SessionVariable_on_xact_actions(bool isCommit, SubTransactionId mySu
SVariableXActActionItem *xact_ai =
(SVariableXActActionItem *) lfirst(cur_item);
+ /*
+ * The subtransaction that created this entry was rollbacked, we can
+ * remove it.
+ */
if (!isCommit && xact_ai->creating_subid == mySubid)
{
- /* cur_item must be removed */
xact_on_commit_actions = foreach_delete_current(xact_on_commit_actions, cur_item);
pfree(xact_ai);
}
else
{
- /* cur_item must be preserved */
+ /* Otherwise cur_item must be preserved */
if (xact_ai->creating_subid == mySubid)
xact_ai->creating_subid = parentSubid;
+
if (xact_ai->deleting_subid == mySubid)
- xact_ai->deleting_subid = isCommit ? parentSubid : InvalidSubTransactionId;
+ xact_ai->deleting_subid = isCommit ? parentSubid
+ : InvalidSubTransactionId;
}
}
}
@@ -1154,7 +1209,10 @@ AtEOSubXact_SessionVariable_on_xact_actions(bool isCommit, SubTransactionId mySu
/*
* pg_debug_show_used_session_variables - designed for testing
*
- * returns content of session vars
+ * This is a function designed for testing and debugging. It returns the
+ * content of sessionvars as-is, and can therefore display entries about
+ * session variables that were dropped but for which this backend didn't
+ * process the shared invalidations yet.
*/
Datum
pg_debug_show_used_session_variables(PG_FUNCTION_ARGS)
@@ -1224,8 +1282,8 @@ pg_debug_show_used_session_variables(PG_FUNCTION_ARGS)
else
{
/*
- * When session variable was removed from catalog, but still
- * it in memory. The memory was not purged yet.
+ * When session variable was removed from catalog, but we
+ * haven't processed the invlidation yet.
*/
nulls[1] = true;
nulls[2] = true;
@@ -1236,7 +1294,6 @@ pg_debug_show_used_session_variables(PG_FUNCTION_ARGS)
nulls[8] = true;
}
-
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
}
}
diff --git a/src/include/commands/session_variable.h b/src/include/commands/session_variable.h
index 3912ce39ef..aae3d25112 100644
--- a/src/include/commands/session_variable.h
+++ b/src/include/commands/session_variable.h
@@ -1,7 +1,7 @@
/*-------------------------------------------------------------------------
*
- * sessionvariable.h
- * prototypes for sessionvariable.c.
+ * session_variable.h
+ * prototypes for session_variable.c.
*
*
* Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
--
2.37.0
--z3ntqfnlkpftg4xg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="v20220922-0005-LET-command.patch"