From 091718e595cc77f5eb0f77cd35b7ca8f9f957437 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Tue, 13 Sep 2022 16:06:59 +0800
Subject: [PATCH v20220916 02/13] FIXUP
 0001-catalog-support-for-session-variables

---
 src/backend/catalog/dependency.c        |   2 +-
 src/backend/catalog/pg_variable.c       | 250 +++++++++++++++++-------
 src/backend/commands/session_variable.c | 120 ++----------
 src/backend/tcop/utility.c              |   2 +-
 src/include/catalog/pg_variable.h       |  20 +-
 src/include/commands/session_variable.h |   4 +-
 6 files changed, 207 insertions(+), 191 deletions(-)

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 6255c2c72f..d8d512973d 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1513,7 +1513,7 @@ doDeletion(const ObjectAddress *object, int flags)
 			break;
 
 		case OCLASS_VARIABLE:
-			RemoveSessionVariable(object->objectId);
+			DropVariable(object->objectId);
 			break;
 
 			/*
diff --git a/src/backend/catalog/pg_variable.c b/src/backend/catalog/pg_variable.c
index a844e5c217..f4a06c47a3 100644
--- a/src/backend/catalog/pg_variable.c
+++ b/src/backend/catalog/pg_variable.c
@@ -16,7 +16,6 @@
 
 #include "access/heapam.h"
 #include "access/htup_details.h"
-
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
@@ -26,82 +25,47 @@
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_variable.h"
+#include "commands/session_variable.h"
+#include "miscadmin.h"
+#include "parser/parse_coerce.h"
+#include "parser/parse_collate.h"
+#include "parser/parse_expr.h"
+#include "parser/parse_type.h"
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/pg_lsn.h"
 #include "utils/syscache.h"
 
-/*
- * Fetch attributes (without acl) of session variable from the syscache.
- * We don't work with acl directly, so we don't need to read it here.
- * Skip deserialization of defexpr when fast_only is true.
- */
-void
-initVariable(Variable *var, Oid varid, bool fast_only)
-{
-	HeapTuple	tup;
-	Form_pg_variable varform;
-	Datum		defexpr_datum;
-	bool		defexpr_isnull;
-
-	tup = SearchSysCache1(VARIABLEOID, ObjectIdGetDatum(varid));
 
-	if (!HeapTupleIsValid(tup))
-		elog(ERROR, "cache lookup failed for session variable %u", varid);
+static ObjectAddress create_variable(const char *varName,
+									 Oid varNamespace,
+									 Oid varType,
+									 int32 varTypmod,
+									 Oid varOwner,
+									 Oid varCollation,
+									 Node *varDefexpr,
+									 VariableEOXAction eoxaction,
+									 bool is_not_null,
+									 bool if_not_exists,
+									 bool is_immutable);
 
-	varform = (Form_pg_variable) GETSTRUCT(tup);
-
-	var->oid = varid;
-	var->create_lsn = varform->create_lsn;
-	var->name = pstrdup(NameStr(varform->varname));
-	var->namespaceid = varform->varnamespace;
-	var->typid = varform->vartype;
-	var->typmod = varform->vartypmod;
-	var->owner = varform->varowner;
-	var->collation = varform->varcollation;
-	var->is_immutable = varform->varisimmutable;
-	var->is_not_null = varform->varisnotnull;
-	var->eoxaction = varform->vareoxaction;
-
-	/* Get defexpr */
-	defexpr_datum = SysCacheGetAttr(VARIABLEOID,
-									tup,
-									Anum_pg_variable_vardefexpr,
-									&defexpr_isnull);
-
-	var->has_defexpr = !defexpr_isnull;
-
-	/*
-	 * Deserialize defexpr only when it is requested. We need to deserialize
-	 * Node with default expression, only when we read from session variable,
-	 * and this session variable has not assigned value, and this session
-	 * variable has default expression. For other cases, we skip skip this
-	 * operation.
-	 */
-	if (!fast_only && !defexpr_isnull)
-		var->defexpr = stringToNode(TextDatumGetCString(defexpr_datum));
-	else
-		var->defexpr = NULL;
-
-	ReleaseSysCache(tup);
-}
 
 /*
- * Create entry in pg_variable table
+ * Creates entry in pg_variable table
  */
-ObjectAddress
-VariableCreate(const char *varName,
-			   Oid varNamespace,
-			   Oid varType,
-			   int32 varTypmod,
-			   Oid varOwner,
-			   Oid varCollation,
-			   Node *varDefexpr,
-			   VariableEOXAction eoxaction,
-			   bool is_not_null,
-			   bool if_not_exists,
-			   bool is_immutable)
+static ObjectAddress
+create_variable(const char *varName,
+				Oid varNamespace,
+				Oid varType,
+				int32 varTypmod,
+				Oid varOwner,
+				Oid varCollation,
+				Node *varDefexpr,
+				VariableEOXAction eoxaction,
+				bool is_not_null,
+				bool if_not_exists,
+				bool is_immutable)
 {
 	Acl		   *varacl;
 	NameData	varname;
@@ -235,10 +199,102 @@ VariableCreate(const char *varName,
 }
 
 /*
- * Remove variable specified by id from pg_variable
+ * Creates a new variable
+ *
+ * Used by CREATE VARIABLE command
+ */
+ObjectAddress
+CreateVariable(ParseState *pstate, CreateSessionVarStmt *stmt)
+{
+	Oid			namespaceid;
+	AclResult	aclresult;
+	Oid			typid;
+	int32		typmod;
+	Oid			varowner = GetUserId();
+	Oid			collation;
+	Oid			typcollation;
+	ObjectAddress variable;
+
+	Node	   *cooked_default = NULL;
+
+	/*
+	 * Check consistency of arguments
+	 */
+	if (stmt->eoxaction == VARIABLE_EOX_DROP
+		&& stmt->variable->relpersistence != RELPERSISTENCE_TEMP)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 errmsg("ON COMMIT DROP can only be used on temporary variables")));
+
+	if (stmt->is_not_null && stmt->is_immutable && !stmt->defexpr)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 errmsg("IMMUTABLE NOT NULL variable requires default expression")));
+
+	namespaceid =
+		RangeVarGetAndCheckCreationNamespace(stmt->variable, NoLock, NULL);
+
+	typenameTypeIdAndMod(pstate, stmt->typeName, &typid, &typmod);
+	typcollation = get_typcollation(typid);
+
+	aclresult = pg_type_aclcheck(typid, GetUserId(), ACL_USAGE);
+	if (aclresult != ACLCHECK_OK)
+		aclcheck_error_type(aclresult, typid);
+
+	if (stmt->collClause)
+		collation = LookupCollation(pstate,
+									stmt->collClause->collname,
+									stmt->collClause->location);
+	else
+		collation = typcollation;;
+
+	/* Complain if COLLATE is applied to an uncollatable type */
+	if (OidIsValid(collation) && !OidIsValid(typcollation))
+		ereport(ERROR,
+				(errcode(ERRCODE_DATATYPE_MISMATCH),
+				 errmsg("collations are not supported by type %s",
+						format_type_be(typid)),
+				 parser_errposition(pstate, stmt->collClause->location)));
+
+	if (stmt->defexpr)
+	{
+		cooked_default = transformExpr(pstate, stmt->defexpr,
+									   EXPR_KIND_VARIABLE_DEFAULT);
+
+		cooked_default = coerce_to_specific_type(pstate,
+												 cooked_default, typid, "DEFAULT");
+		assign_expr_collations(pstate, cooked_default);
+	}
+
+	variable = create_variable(stmt->variable->relname,
+							   namespaceid,
+							   typid,
+							   typmod,
+							   varowner,
+							   collation,
+							   cooked_default,
+							   stmt->eoxaction,
+							   stmt->is_not_null,
+							   stmt->if_not_exists,
+							   stmt->is_immutable);
+
+	elog(DEBUG1, "record for session variable \"%s\" (oid:%d) was created in pg_variable",
+		 stmt->variable->relname, variable.objectId);
+
+	/* We want SessionVariableCreatePostprocess to see the catalog changes. */
+	CommandCounterIncrement();
+
+	SessionVariableCreatePostprocess(variable.objectId, stmt->eoxaction);
+
+	return variable;
+}
+
+/*
+ * Drop variable by OID, and register the needed session variable
+ * cleanup.
  */
 void
-VariableDrop(Oid varid)
+DropVariable(Oid varid)
 {
 	Relation	rel;
 	HeapTuple	tup;
@@ -255,4 +311,62 @@ VariableDrop(Oid varid)
 	ReleaseSysCache(tup);
 
 	table_close(rel, RowExclusiveLock);
+
+	/* Do the necessary cleanup if needed in local memory */
+	SessionVariableDropPostprocess(varid);
+}
+
+/*
+ * Fetch attributes (without acl) of session variable from the syscache.
+ * We don't work with acl directly, so we don't need to read it here.
+ * Skip deserialization of defexpr when fast_only is true.
+ */
+void
+InitVariable(Variable *var, Oid varid, bool fast_only)
+{
+	HeapTuple	tup;
+	Form_pg_variable varform;
+	Datum		defexpr_datum;
+	bool		defexpr_isnull;
+
+	tup = SearchSysCache1(VARIABLEOID, ObjectIdGetDatum(varid));
+
+	if (!HeapTupleIsValid(tup))
+		elog(ERROR, "cache lookup failed for session variable %u", varid);
+
+	varform = (Form_pg_variable) GETSTRUCT(tup);
+
+	var->oid = varid;
+	var->create_lsn = varform->create_lsn;
+	var->name = pstrdup(NameStr(varform->varname));
+	var->namespaceid = varform->varnamespace;
+	var->typid = varform->vartype;
+	var->typmod = varform->vartypmod;
+	var->owner = varform->varowner;
+	var->collation = varform->varcollation;
+	var->is_immutable = varform->varisimmutable;
+	var->is_not_null = varform->varisnotnull;
+	var->eoxaction = varform->vareoxaction;
+
+	/* Get defexpr */
+	defexpr_datum = SysCacheGetAttr(VARIABLEOID,
+									tup,
+									Anum_pg_variable_vardefexpr,
+									&defexpr_isnull);
+
+	var->has_defexpr = !defexpr_isnull;
+
+	/*
+	 * Deserialize defexpr only when it is requested. We need to deserialize
+	 * Node with default expression, only when we read from session variable,
+	 * and this session variable has not assigned value, and this session
+	 * variable has default expression. For other cases, we skip skip this
+	 * operation.
+	 */
+	if (!fast_only && !defexpr_isnull)
+		var->defexpr = stringToNode(TextDatumGetCString(defexpr_datum));
+	else
+		var->defexpr = NULL;
+
+	ReleaseSysCache(tup);
 }
diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c
index 4ac6ecf730..3eafc05f89 100644
--- a/src/backend/commands/session_variable.c
+++ b/src/backend/commands/session_variable.c
@@ -13,7 +13,7 @@
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
-#include "miscadmin.h"
+
 #include "access/heapam.h"
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
@@ -21,10 +21,7 @@
 #include "catalog/pg_variable.h"
 #include "commands/session_variable.h"
 #include "funcapi.h"
-#include "parser/parse_coerce.h"
-#include "parser/parse_collate.h"
-#include "parser/parse_expr.h"
-#include "parser/parse_type.h"
+#include "miscadmin.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
@@ -58,123 +55,40 @@ static List *xact_drop_actions = NIL;
 static void register_session_variable_xact_action(Oid varid, SVariableXActAction action);
 static void unregister_session_variable_xact_action(Oid varid, SVariableXActAction action);
 
-/*
- * Routines used for manipulation with session variables from
- * SQL level
- */
 
 /*
- * Creates new variable - entry in pg_catalog.pg_variable table
+ * Do the necessary work to setup local memory management of a new
+ * variable.
  *
- * Used by CREATE VARIABLE command
+ * Caller should already have created the necessary entry in catalog
+ * and made them visible.
  */
-ObjectAddress
-DefineSessionVariable(ParseState *pstate, CreateSessionVarStmt *stmt)
+void
+SessionVariableCreatePostprocess(Oid varid, char eoxaction)
 {
-	Oid			namespaceid;
-	AclResult	aclresult;
-	Oid			typid;
-	int32		typmod;
-	Oid			varowner = GetUserId();
-	Oid			collation;
-	Oid			typcollation;
-	ObjectAddress variable;
-
-	Node	   *cooked_default = NULL;
-
-	/*
-	 * Check consistency of arguments
-	 */
-	if (stmt->eoxaction == VARIABLE_EOX_DROP
-		&& stmt->variable->relpersistence != RELPERSISTENCE_TEMP)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("ON COMMIT DROP can only be used on temporary variables")));
-
-	if (stmt->is_not_null && stmt->is_immutable && !stmt->defexpr)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("IMMUTABLE NOT NULL variable requires default expression")));
-
-	namespaceid =
-		RangeVarGetAndCheckCreationNamespace(stmt->variable, NoLock, NULL);
-
-	typenameTypeIdAndMod(pstate, stmt->typeName, &typid, &typmod);
-	typcollation = get_typcollation(typid);
-
-	aclresult = pg_type_aclcheck(typid, GetUserId(), ACL_USAGE);
-	if (aclresult != ACLCHECK_OK)
-		aclcheck_error_type(aclresult, typid);
-
-	if (stmt->collClause)
-		collation = LookupCollation(pstate,
-									stmt->collClause->collname,
-									stmt->collClause->location);
-	else
-		collation = typcollation;;
-
-	/* Complain if COLLATE is applied to an uncollatable type */
-	if (OidIsValid(collation) && !OidIsValid(typcollation))
-		ereport(ERROR,
-				(errcode(ERRCODE_DATATYPE_MISMATCH),
-				 errmsg("collations are not supported by type %s",
-						format_type_be(typid)),
-				 parser_errposition(pstate, stmt->collClause->location)));
-
-	if (stmt->defexpr)
-	{
-		cooked_default = transformExpr(pstate, stmt->defexpr,
-									   EXPR_KIND_VARIABLE_DEFAULT);
-
-		cooked_default = coerce_to_specific_type(pstate,
-												 cooked_default, typid, "DEFAULT");
-		assign_expr_collations(pstate, cooked_default);
-	}
-
-	variable = VariableCreate(stmt->variable->relname,
-							  namespaceid,
-							  typid,
-							  typmod,
-							  varowner,
-							  collation,
-							  cooked_default,
-							  stmt->eoxaction,
-							  stmt->is_not_null,
-							  stmt->if_not_exists,
-							  stmt->is_immutable);
-
-	elog(DEBUG1, "record for session variable \"%s\" (oid:%d) was created in pg_variable",
-		 stmt->variable->relname, variable.objectId);
-
 	/*
 	 * For temporary variables, we need to create a new end of xact action to
 	 * ensure deletion from catalog.
 	 */
-	if (stmt->eoxaction == VARIABLE_EOX_DROP)
+	if (eoxaction == VARIABLE_EOX_DROP)
 	{
-		Assert(isTempNamespace(namespaceid));
+		Assert(isTempNamespace(get_session_variable_namespace(varid)));
 
-		register_session_variable_xact_action(variable.objectId,
-											  SVAR_ON_COMMIT_DROP);
+		register_session_variable_xact_action(varid, SVAR_ON_COMMIT_DROP);
 	}
-
-	return variable;
 }
 
 /*
- * Drop variable by OID. This routine doesn't try to remove
- * the value of session variable immediately. It will be
- * removed on transaction end in sync_sessionvars_xact_callback
- * routine. This routine manipulate just with system catalog.
+ * Handle the local memory cleanup for a DROP VARIABLE command.
+ *
+ * Caller should take care of removing the pg_variable entry first.
  */
 void
-RemoveSessionVariable(Oid varid)
+SessionVariableDropPostprocess(Oid varid)
 {
-	VariableDrop(varid);
-
 	/*
-	 * We removed entry from catalog already, we must not do it again at end
-	 * of xact time
+	 * The entry was removed from catalog already, we must not do it
+	 * again at end of xact time.
 	 */
 	unregister_session_variable_xact_action(varid, SVAR_ON_COMMIT_DROP);
 }
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 58f5c2b69c..0bba0ed480 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1396,7 +1396,7 @@ ProcessUtilitySlow(ParseState *pstate,
 				break;
 
 			case T_CreateSessionVarStmt:
-				address = DefineSessionVariable(pstate, (CreateSessionVarStmt *) parsetree);
+				address = CreateVariable(pstate, (CreateSessionVarStmt *) parsetree);
 				break;
 
 				/*
diff --git a/src/include/catalog/pg_variable.h b/src/include/catalog/pg_variable.h
index e75222f90c..cde36b0cea 100644
--- a/src/include/catalog/pg_variable.h
+++ b/src/include/catalog/pg_variable.h
@@ -112,21 +112,9 @@ typedef struct Variable
 	Node	   *defexpr;
 } Variable;
 
-extern void initVariable(Variable *var,
-						 Oid varid,
-						 bool fast_only);
-extern ObjectAddress VariableCreate(const char *varName,
-									Oid varNamespace,
-									Oid varType,
-									int32 varTypmod,
-									Oid varOwner,
-									Oid varCollation,
-									Node *varDefexpr,
-									VariableEOXAction eoxaction,
-									bool is_not_null,
-									bool if_not_exists,
-									bool is_immutable);
-
-extern void VariableDrop(Oid varid);
+extern ObjectAddress CreateVariable(ParseState *pstate,
+									CreateSessionVarStmt *stmt);
+extern void DropVariable(Oid varid);
+extern void InitVariable(Variable *var, Oid varid, bool fast_only);
 
 #endif							/* PG_VARIABLE_H */
diff --git a/src/include/commands/session_variable.h b/src/include/commands/session_variable.h
index 0daf6f41ec..51934e8d1a 100644
--- a/src/include/commands/session_variable.h
+++ b/src/include/commands/session_variable.h
@@ -24,8 +24,8 @@
 #include "utils/queryenvironment.h"
 
 extern void ResetSessionVariables(void);
-extern void RemoveSessionVariable(Oid varid);
-extern ObjectAddress DefineSessionVariable(ParseState *pstate, CreateSessionVarStmt *stmt);
+extern void SessionVariableCreatePostprocess(Oid varid, char eoxaction);
+extern void SessionVariableDropPostprocess(Oid varid);
 
 extern Datum CopySessionVariable(Oid varid, bool *isNull, Oid *typid);
 extern Datum CopySessionVariableWithTypeCheck(Oid varid, bool *isNull, Oid expected_typid);
-- 
2.37.0

