Faster "SET search_path"

From f0b826ffec2b21ea9eb595771ebc83d271f1e3a1 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:11:42 -0700
Subject: [PATCH v2 1/3] Transform proconfig for faster execution.

Store function config settings as a list of (name, value) pairs to
avoid the need to parse for each function invocation.
---
 src/backend/utils/fmgr/fmgr.c | 27 ++++++++++++-------
 src/backend/utils/misc/guc.c  | 49 ++++++++++++++++++++++++++++-------
 src/include/utils/guc.h       |  1 +
 3 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9208c31fe0..3b49684991 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -612,7 +612,7 @@ struct fmgr_security_definer_cache
 {
 	FmgrInfo	flinfo;			/* lookup info for target function */
 	Oid			userid;			/* userid to set, or InvalidOid */
-	ArrayType  *proconfig;		/* GUC values to set, or NULL */
+	List	   *configList;		/* GUC values to set, or NULL */
 	Datum		arg;			/* passthrough argument for plugin modules */
 };
 
@@ -634,6 +634,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 	FmgrInfo   *save_flinfo;
 	Oid			save_userid;
 	int			save_sec_context;
+	ListCell   *lc;
 	volatile int save_nestlevel;
 	PgStat_FunctionCallUsage fcusage;
 
@@ -666,8 +667,10 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 								&isnull);
 		if (!isnull)
 		{
+			ArrayType *array;
 			oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
-			fcache->proconfig = DatumGetArrayTypePCopy(datum);
+			array = DatumGetArrayTypeP(datum);
+			fcache->configList = TransformGUCArray(array);
 			MemoryContextSwitchTo(oldcxt);
 		}
 
@@ -680,7 +683,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	/* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
 	GetUserIdAndSecContext(&save_userid, &save_sec_context);
-	if (fcache->proconfig)		/* Need a new GUC nesting level */
+	if (fcache->configList != NIL)		/* Need a new GUC nesting level */
 		save_nestlevel = NewGUCNestLevel();
 	else
 		save_nestlevel = 0;		/* keep compiler quiet */
@@ -689,12 +692,18 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 		SetUserIdAndSecContext(fcache->userid,
 							   save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
-	if (fcache->proconfig)
+	foreach(lc, fcache->configList)
 	{
-		ProcessGUCArray(fcache->proconfig,
-						(superuser() ? PGC_SUSET : PGC_USERSET),
-						PGC_S_SESSION,
-						GUC_ACTION_SAVE);
+		GucContext	 context = superuser() ? PGC_SUSET : PGC_USERSET;
+		GucSource	 source	 = PGC_S_SESSION;
+		GucAction	 action	 = GUC_ACTION_SAVE;
+		List		*pair	 = lfirst(lc);
+		char		*name	 = linitial(pair);
+		char		*value	 = lsecond(pair);
+
+		(void) set_config_option(name, value,
+								 context, source,
+								 action, true, 0, false);
 	}
 
 	/* function manager hook */
@@ -737,7 +746,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	fcinfo->flinfo = save_flinfo;
 
-	if (fcache->proconfig)
+	if (fcache->configList != NIL)
 		AtEOXact_GUC(true, save_nestlevel);
 	if (OidIsValid(fcache->userid))
 		SetUserIdAndSecContext(save_userid, save_sec_context);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5308896c87..ea089de3ec 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6213,17 +6213,15 @@ ParseLongOption(const char *string, char **name, char **value)
 			*cp = '_';
 }
 
-
 /*
- * Handle options fetched from pg_db_role_setting.setconfig,
- * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
- *
- * The array parameter must be an array of TEXT (it must not be NULL).
+ * Transform array of GUC settings into a list of (name, value) pairs. The
+ * list is faster to process in cases where the settings must be applied
+ * repeatedly (e.g. for each function invocation).
  */
-void
-ProcessGUCArray(ArrayType *array,
-				GucContext context, GucSource source, GucAction action)
+List *
+TransformGUCArray(ArrayType *array)
 {
+	List	   *result = NIL;
 	int			i;
 
 	Assert(array != NULL);
@@ -6238,6 +6236,7 @@ ProcessGUCArray(ArrayType *array,
 		char	   *s;
 		char	   *name;
 		char	   *value;
+		List	   *pair;
 
 		d = array_ref(array, 1, &i,
 					  -1 /* varlenarray */ ,
@@ -6262,14 +6261,46 @@ ProcessGUCArray(ArrayType *array,
 			continue;
 		}
 
+		pair = list_make2(pstrdup(name), pstrdup(value));
+		result = lappend(result, pair);
+
+		pfree(name);
+		pfree(value);
+		pfree(s);
+	}
+
+	return result;
+}
+
+/*
+ * Handle options fetched from pg_db_role_setting.setconfig,
+ * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
+ *
+ * The array parameter must be an array of TEXT (it must not be NULL).
+ */
+void
+ProcessGUCArray(ArrayType *array,
+				GucContext context, GucSource source, GucAction action)
+{
+	List *gucList = TransformGUCArray(array);
+	ListCell *lc;
+
+	foreach(lc, gucList)
+	{
+		List *pair = lfirst(lc);
+		char *name = linitial(pair);
+		char *value = lsecond(pair);
+
 		(void) set_config_option(name, value,
 								 context, source,
 								 action, true, 0, false);
 
 		pfree(name);
 		pfree(value);
-		pfree(s);
+		list_free(pair);
 	}
+
+	list_free(gucList);
 }
 
 
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 223a19f80d..713a121312 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -391,6 +391,7 @@ extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
 								   bool missing_ok);
 
+extern List *TransformGUCArray(ArrayType *array);
 extern void ProcessGUCArray(ArrayType *array,
 							GucContext context, GucSource source, GucAction action);
 extern ArrayType *GUCArrayAdd(ArrayType *array, const char *name, const char *value);
-- 
2.34.1

From cb692de4e59317cdad4c58b660841dd96ae757a8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:23:51 -0700
Subject: [PATCH v2 2/3] Optimize check_search_path().

Introduce CheckIdentifierString(), which validates a string list of
identifiers without the work of actually storing them in a list.

This speeds up frequent search_path changes, such as when a function
has search_path set in proconfig. Based on profiling.
---
 src/backend/catalog/namespace.c | 13 +-----
 src/backend/utils/adt/varlena.c | 70 +++++++++++++++++++++++++++++++++
 src/include/utils/varlena.h     |  1 +
 3 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 1f76b5d7f7..f457f778cb 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -4101,19 +4101,13 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
-	char	   *rawname;
-	List	   *namelist;
-
-	/* Need a modifiable copy of string */
-	rawname = pstrdup(*newval);
+	char *rawname = *newval;
 
 	/* Parse string into list of identifiers */
-	if (!SplitIdentifierString(rawname, ',', &namelist))
+	if (!CheckIdentifierString(rawname, ','))
 	{
 		/* syntax error in name list */
 		GUC_check_errdetail("List syntax is invalid.");
-		pfree(rawname);
-		list_free(namelist);
 		return false;
 	}
 
@@ -4125,9 +4119,6 @@ check_search_path(char **newval, void **extra, GucSource source)
 	 * requirement is syntactic validity of the identifier list.
 	 */
 
-	pfree(rawname);
-	list_free(namelist);
-
 	return true;
 }
 
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index b1ec5c32ce..c8d36fd868 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3427,6 +3427,76 @@ textToQualifiedNameList(text *textval)
 	return result;
 }
 
+/*
+ * Check that an identifier list is syntactically valid; that is, that there
+ * are no mismatched quotes and that identifiers are separated with the given
+ * separator. Empty identifiers are considered acceptable if (and only if)
+ * quoted.
+ *
+ * Note that an empty string is considered okay here, though not in
+ * textToQualifiedNameList.
+ */
+bool
+CheckIdentifierString(const char *rawstring, char separator)
+{
+	const char *nextp = rawstring;
+	bool		done = false;
+
+	while (scanner_isspace(*nextp))
+		nextp++;				/* skip leading whitespace */
+
+	if (*nextp == '\0')
+		return true;			/* allow empty string */
+
+	/* At the top of the loop, we are at start of a new identifier. */
+	do
+	{
+		if (*nextp == '"')
+		{
+			for (;;)
+			{
+				nextp = strchr(nextp + 1, '"');
+				if (nextp == NULL)
+					return false;	/* mismatched quotes */
+				if (nextp[1] != '"')
+					break;		/* found end of quoted name */
+				nextp++;
+			}
+
+			nextp++;
+		}
+		else
+		{
+			/* Unquoted name --- extends to separator or whitespace */
+			const char *curname = nextp;
+			while (*nextp && *nextp != separator &&
+				   !scanner_isspace(*nextp))
+				nextp++;
+			if (curname == nextp)
+				return false;	/* empty unquoted name not allowed */
+		}
+
+		while (scanner_isspace(*nextp))
+			nextp++;			/* skip trailing whitespace */
+
+		if (*nextp == separator)
+		{
+			nextp++;
+			while (scanner_isspace(*nextp))
+				nextp++;		/* skip leading whitespace for next */
+			/* we expect another name, so done remains false */
+		}
+		else if (*nextp == '\0')
+			done = true;
+		else
+			return false;		/* invalid syntax */
+
+		/* Loop back if we didn't reach end of string */
+	} while (!done);
+
+	return true;
+}
+
 /*
  * SplitIdentifierString --- parse a string containing identifiers
  *
diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h
index 77f5b24735..4ff0cecc91 100644
--- a/src/include/utils/varlena.h
+++ b/src/include/utils/varlena.h
@@ -27,6 +27,7 @@ extern int	varstr_levenshtein_less_equal(const char *source, int slen,
 										  int ins_c, int del_c, int sub_c,
 										  int max_d, bool trusted);
 extern List *textToQualifiedNameList(text *textval);
+extern bool CheckIdentifierString(const char *rawstring, char separator);
 extern bool SplitIdentifierString(char *rawstring, char separator,
 								  List **namelist);
 extern bool SplitDirectoriesString(char *rawstring, char separator,
-- 
2.34.1

From 54b79e769c2e593b4d0deab1a19f5891af002d8f Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v2 3/3] Add cache for recomputeNamespacePath().

When search_path is changed to something that was previously set, and
no invalidation happened in between, use the cached list of namespace
OIDs rather than recomputing them. This avoids syscache lookups and
ACL checks.

Important when the search_path changes frequently, such as when set in
proconfig.
---
 src/backend/catalog/namespace.c | 282 +++++++++++++++++++++++++-------
 1 file changed, 219 insertions(+), 63 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index f457f778cb..3be2317d70 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
+#include "common/hashfn.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -109,11 +110,12 @@
  * activeSearchPath is always the actually active path; it points to
  * to baseSearchPath which is the list derived from namespace_search_path.
  *
- * If baseSearchPathValid is false, then baseSearchPath (and other
- * derived variables) need to be recomputed from namespace_search_path.
- * We mark it invalid upon an assignment to namespace_search_path or receipt
- * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next lookup attempt.
+ * If baseSearchPathValid is false, then baseSearchPath (and other derived
+ * variables) need to be recomputed from namespace_search_path, or retrieved
+ * from the search path cache if there haven't been any syscache
+ * invalidations.  We mark it invalid upon an assignment to
+ * namespace_search_path or receipt of a syscache invalidation event for
+ * pg_namespace.  The recomputation is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -153,6 +155,22 @@ static Oid	namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
+static bool searchPathCacheValid = false;
+static MemoryContext SearchPathCacheContext = NULL;
+
+typedef struct SearchPathCacheKey
+{
+	const char	*searchPath;
+	Oid			 roleid;
+} SearchPathCacheKey;
+
+typedef struct SearchPathCacheEntry
+{
+	SearchPathCacheKey	*key;
+	List				*oidlist;
+	bool				 temp_missing;
+	char				 status;
+} SearchPathCacheEntry;
 
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@@ -193,6 +211,44 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
 
+/*
+ * Recomputing the namespace path can be costly when done frequently, such as
+ * when a function has search_path set in proconfig. Add a cache that can be
+ * used when the search_path changes to a value that was previously set, and
+ * no invalidation intervened.
+ *
+ * The cache is a simple hash table in its own memory context, with a key of
+ * search_path and role ID.
+ */
+
+static inline uint32
+nspcachekey_hash(SearchPathCacheKey *key)
+{
+	const unsigned char	*bytes = (const unsigned char *)key->searchPath;
+	int					 blen  = strlen(key->searchPath);
+
+	return hash_bytes(bytes, blen) ^ hash_uint32(key->roleid);
+}
+
+static inline bool
+nspcachekey_equal(SearchPathCacheKey *a, SearchPathCacheKey *b)
+{
+	return a->roleid == b->roleid &&
+		strcmp(a->searchPath, b->searchPath) == 0;
+}
+
+#define SH_PREFIX		nsphash
+#define SH_ELEMENT_TYPE	SearchPathCacheEntry
+#define SH_KEY_TYPE		SearchPathCacheKey *
+#define	SH_KEY			key
+#define SH_HASH_KEY(tb, key)   	nspcachekey_hash(key)
+#define SH_EQUAL(tb, a, b)		nspcachekey_equal(a, b)
+#define	SH_SCOPE		static inline
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static nsphash_hash *SearchPathCache = NULL;
 
 /*
  * RangeVarGetRelidExtended
@@ -3370,6 +3426,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 	 */
 
 	baseSearchPathValid = false;	/* may need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 
@@ -3633,28 +3690,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
 }
 
 /*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Look up namespace IDs and perform ACL checks. Return newly-allocated list.
  */
-static void
-recomputeNamespacePath(void)
+static List *
+preprocessNamespacePath(const char *searchPath, Oid roleid,
+						bool *temp_missing)
 {
-	Oid			roleid = GetUserId();
 	char	   *rawname;
 	List	   *namelist;
 	List	   *oidlist;
-	List	   *newpath;
 	ListCell   *l;
-	bool		temp_missing;
-	Oid			firstNS;
-	bool		pathChanged;
-	MemoryContext oldcxt;
 
-	/* Do nothing if path is already valid. */
-	if (baseSearchPathValid && namespaceUser == roleid)
-		return;
-
-	/* Need a modifiable copy of namespace_search_path string */
-	rawname = pstrdup(namespace_search_path);
+	/* Need a modifiable copy */
+	rawname = pstrdup(searchPath);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -3671,7 +3719,7 @@ recomputeNamespacePath(void)
 	 * already been accepted.)	Don't make duplicate entries, either.
 	 */
 	oidlist = NIL;
-	temp_missing = false;
+	*temp_missing = false;
 	foreach(l, namelist)
 	{
 		char	   *curname = (char *) lfirst(l);
@@ -3691,10 +3739,8 @@ recomputeNamespacePath(void)
 				namespaceId = get_namespace_oid(rname, true);
 				ReleaseSysCache(tuple);
 				if (OidIsValid(namespaceId) &&
-					!list_member_oid(oidlist, namespaceId) &&
 					object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-									ACL_USAGE) == ACLCHECK_OK &&
-					InvokeNamespaceSearchHook(namespaceId, false))
+									ACL_USAGE) == ACLCHECK_OK)
 					oidlist = lappend_oid(oidlist, namespaceId);
 			}
 		}
@@ -3702,16 +3748,12 @@ recomputeNamespacePath(void)
 		{
 			/* pg_temp --- substitute temp namespace, if any */
 			if (OidIsValid(myTempNamespace))
-			{
-				if (!list_member_oid(oidlist, myTempNamespace) &&
-					InvokeNamespaceSearchHook(myTempNamespace, false))
-					oidlist = lappend_oid(oidlist, myTempNamespace);
-			}
+				oidlist = lappend_oid(oidlist, myTempNamespace);
 			else
 			{
 				/* If it ought to be the creation namespace, set flag */
 				if (oidlist == NIL)
-					temp_missing = true;
+					*temp_missing = true;
 			}
 		}
 		else
@@ -3719,63 +3761,166 @@ recomputeNamespacePath(void)
 			/* normal namespace reference */
 			namespaceId = get_namespace_oid(curname, true);
 			if (OidIsValid(namespaceId) &&
-				!list_member_oid(oidlist, namespaceId) &&
 				object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-								ACL_USAGE) == ACLCHECK_OK &&
-				InvokeNamespaceSearchHook(namespaceId, false))
+								ACL_USAGE) == ACLCHECK_OK)
 				oidlist = lappend_oid(oidlist, namespaceId);
 		}
 	}
 
+	pfree(rawname);
+	list_free(namelist);
+
+	return oidlist;
+}
+
+/*
+ * Remove duplicates, run namespace search hooks, and prepend
+ * implicitly-searched namespaces. Return newly-allocated list.
+ *
+ * All of this must happen after retrieving from cache, because we don't know
+ * what might invalidate the result from the last time the hook was
+ * invoked. It may seem that duplicate elimination is not dependent on the
+ * result of the hook, but if a hook returns different results on different
+ * calls for the same namespace ID, then it could affect the order in which
+ * that namespace appears in the final list.
+ */
+static List *
+finalNamespacePath(List *oidlist, Oid *firstNS)
+{
+	List		*finalPath = NIL;
+	ListCell	*lc;
+
+	foreach(lc, oidlist)
+	{
+		Oid namespaceId = lfirst_oid(lc);
+		if (!list_member_oid(finalPath, namespaceId) &&
+			InvokeNamespaceSearchHook(namespaceId, false))
+			finalPath = lappend_oid(finalPath, namespaceId);
+	}
+
 	/*
 	 * Remember the first member of the explicit list.  (Note: this is
 	 * nominally wrong if temp_missing, but we need it anyway to distinguish
 	 * explicit from implicit mention of pg_catalog.)
 	 */
-	if (oidlist == NIL)
-		firstNS = InvalidOid;
+	if (finalPath == NIL)
+		*firstNS = InvalidOid;
 	else
-		firstNS = linitial_oid(oidlist);
+		*firstNS = linitial_oid(finalPath);
 
 	/*
 	 * Add any implicitly-searched namespaces to the list.  Note these go on
 	 * the front, not the back; also notice that we do not check USAGE
 	 * permissions for these.
 	 */
-	if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
-		oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
+	if (!list_member_oid(finalPath, PG_CATALOG_NAMESPACE))
+		finalPath = lcons_oid(PG_CATALOG_NAMESPACE, finalPath);
 
 	if (OidIsValid(myTempNamespace) &&
-		!list_member_oid(oidlist, myTempNamespace))
-		oidlist = lcons_oid(myTempNamespace, oidlist);
+		!list_member_oid(finalPath, myTempNamespace))
+		finalPath = lcons_oid(myTempNamespace, finalPath);
+
+	return finalPath;
+}
+
+/*
+ * Retrieve search path information from the cache; or if not present,
+ * preprocess it and add it to the cache. Return OID list allocated in
+ * SearchPathCacheContext; caller should not free.
+ */
+static List *
+cachedNamespacePath(const char *searchPath, Oid roleid,
+					bool *temp_missing)
+{
+	SearchPathCacheKey		 cachekey;
+	SearchPathCacheEntry	*entry;
+	bool					 found;
+
+	/* clear the cache and start over when invalidated */
+	if (!searchPathCacheValid)
+	{
+		MemoryContextReset(SearchPathCacheContext);
+		/* arbitrary initial starting size of 16 elements */
+		SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
+		searchPathCacheValid = true;
+	}
+
+	cachekey.searchPath = searchPath;
+	cachekey.roleid = roleid;
+
+	entry = nsphash_insert(SearchPathCache, &cachekey, &found);
+
+	if (!found)
+	{
+		MemoryContext		 oldcxt;
+		SearchPathCacheKey	*newkey;
+		List				*oidlist;
+
+		oidlist = preprocessNamespacePath(searchPath, roleid,
+										  &entry->temp_missing);
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+
+		newkey = palloc(sizeof(SearchPathCacheKey));
+		newkey->searchPath = pstrdup(searchPath);
+		newkey->roleid = roleid;
+
+		entry->key = newkey;
+		entry->oidlist = list_copy(oidlist);
+
+		MemoryContextSwitchTo(oldcxt);
+
+		/* oidlist already copied into cache entry */
+		list_free(oidlist);
+	}
+
+	*temp_missing = entry->temp_missing;
+	return entry->oidlist;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+	Oid			roleid = GetUserId();
+	List	   *oidlist;
+	Oid			firstNS;
+	bool		temp_missing;
+	bool		pathChanged;
+	List	   *finalPath;
+	MemoryContext	oldcxt;
+
+	/* Do nothing if path is already valid. */
+	if (baseSearchPathValid && namespaceUser == roleid)
+		return;
+
+	oidlist = cachedNamespacePath(namespace_search_path, roleid,
+								  &temp_missing);
+
+	/* Must save OID list in permanent storage. */
+	oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+	finalPath = finalNamespacePath(oidlist, &firstNS);
+	MemoryContextSwitchTo(oldcxt);
 
-	/*
-	 * We want to detect the case where the effective value of the base search
-	 * path variables didn't change.  As long as we're doing so, we can avoid
-	 * copying the OID list unnecessarily.
-	 */
 	if (baseCreationNamespace == firstNS &&
 		baseTempCreationPending == temp_missing &&
-		equal(oidlist, baseSearchPath))
+		equal(finalPath, baseSearchPath))
 	{
 		pathChanged = false;
 	}
 	else
 	{
 		pathChanged = true;
-
-		/* Must save OID list in permanent storage. */
-		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-		newpath = list_copy(oidlist);
-		MemoryContextSwitchTo(oldcxt);
-
-		/* Now safe to assign to state variables. */
-		list_free(baseSearchPath);
-		baseSearchPath = newpath;
-		baseCreationNamespace = firstNS;
-		baseTempCreationPending = temp_missing;
 	}
 
+	/* Now safe to assign to state variables. */
+	list_free(baseSearchPath);
+	baseSearchPath = finalPath;
+	baseCreationNamespace = firstNS;
+	baseTempCreationPending = temp_missing;
+
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
 	namespaceUser = roleid;
@@ -3791,11 +3936,6 @@ recomputeNamespacePath(void)
 	 */
 	if (pathChanged)
 		activePathGeneration++;
-
-	/* Clean up. */
-	pfree(rawname);
-	list_free(namelist);
-	list_free(oidlist);
 }
 
 /*
@@ -3950,6 +4090,7 @@ InitTempTableNamespace(void)
 	myTempNamespaceSubID = GetCurrentSubTransactionId();
 
 	baseSearchPathValid = false;	/* need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 /*
@@ -3975,6 +4116,7 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4016,6 +4158,7 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4130,6 +4273,10 @@ assign_search_path(const char *newval, void *extra)
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
 	 * initialization, or outside a transaction.
+	 *
+	 * This does not invalidate the search path cache, so if this value had
+	 * been previously set and no syscache invalidations happened,
+	 * recomputation may not be necessary.
 	 */
 	baseSearchPathValid = false;
 }
@@ -4164,6 +4311,9 @@ InitializeSearchPath(void)
 	}
 	else
 	{
+		SearchPathCacheContext = AllocSetContextCreate(TopMemoryContext,
+													   "search_path processing cache",
+													   ALLOCSET_DEFAULT_SIZES);
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
 		 * of pg_namespace rows.
@@ -4173,6 +4323,7 @@ InitializeSearchPath(void)
 									  (Datum) 0);
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
+		searchPathCacheValid = false;
 	}
 }
 
@@ -4183,8 +4334,13 @@ InitializeSearchPath(void)
 static void
 NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
-	/* Force search path to be recomputed on next use */
+	/*
+	 * Force search path to be recomputed on next use, also invalidating the
+	 * search path cache (because namespace names, ACLs, or role names may
+	 * have changed).
+	 */
 	baseSearchPathValid = false;
+	searchPathCacheValid = false;
 }
 
 /*
-- 
2.34.1

From d9a3d4364731a152096d4a8c8cf6a81c1e86f8ab Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:11:42 -0700
Subject: [PATCH v3 1/3] Transform proconfig for faster execution.

Store function config settings as a list of (name, value) pairs to
avoid the need to parse for each function invocation.
---
 src/backend/utils/fmgr/fmgr.c | 29 +++++++++++++++-------
 src/backend/utils/misc/guc.c  | 45 ++++++++++++++++++++++++++++-------
 src/include/utils/guc.h       |  2 ++
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9208c31fe0..6b2d7d7be3 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -612,7 +612,8 @@ struct fmgr_security_definer_cache
 {
 	FmgrInfo	flinfo;			/* lookup info for target function */
 	Oid			userid;			/* userid to set, or InvalidOid */
-	ArrayType  *proconfig;		/* GUC values to set, or NULL */
+	List	   *configNames;	/* GUC names to set, or NIL */
+	List	   *configValues;	/* GUC values to set, or NIL */
 	Datum		arg;			/* passthrough argument for plugin modules */
 };
 
@@ -634,6 +635,8 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 	FmgrInfo   *save_flinfo;
 	Oid			save_userid;
 	int			save_sec_context;
+	ListCell   *lc1;
+	ListCell   *lc2;
 	volatile int save_nestlevel;
 	PgStat_FunctionCallUsage fcusage;
 
@@ -666,8 +669,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 								&isnull);
 		if (!isnull)
 		{
+			ArrayType *array;
 			oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
-			fcache->proconfig = DatumGetArrayTypePCopy(datum);
+			array = DatumGetArrayTypeP(datum);
+			TransformGUCArray(array, &fcache->configNames,
+							  &fcache->configValues);
 			MemoryContextSwitchTo(oldcxt);
 		}
 
@@ -680,7 +686,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	/* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
 	GetUserIdAndSecContext(&save_userid, &save_sec_context);
-	if (fcache->proconfig)		/* Need a new GUC nesting level */
+	if (fcache->configNames != NIL)		/* Need a new GUC nesting level */
 		save_nestlevel = NewGUCNestLevel();
 	else
 		save_nestlevel = 0;		/* keep compiler quiet */
@@ -689,12 +695,17 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 		SetUserIdAndSecContext(fcache->userid,
 							   save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
-	if (fcache->proconfig)
+	forboth(lc1, fcache->configNames, lc2, fcache->configValues)
 	{
-		ProcessGUCArray(fcache->proconfig,
-						(superuser() ? PGC_SUSET : PGC_USERSET),
-						PGC_S_SESSION,
-						GUC_ACTION_SAVE);
+		GucContext	 context = superuser() ? PGC_SUSET : PGC_USERSET;
+		GucSource	 source	 = PGC_S_SESSION;
+		GucAction	 action	 = GUC_ACTION_SAVE;
+		char		*name	 = lfirst(lc1);
+		char		*value	 = lfirst(lc2);
+
+		(void) set_config_option(name, value,
+								 context, source,
+								 action, true, 0, false);
 	}
 
 	/* function manager hook */
@@ -737,7 +748,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	fcinfo->flinfo = save_flinfo;
 
-	if (fcache->proconfig)
+	if (fcache->configNames != NIL)
 		AtEOXact_GUC(true, save_nestlevel);
 	if (OidIsValid(fcache->userid))
 		SetUserIdAndSecContext(save_userid, save_sec_context);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5308896c87..fa96b79192 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6213,16 +6213,13 @@ ParseLongOption(const char *string, char **name, char **value)
 			*cp = '_';
 }
 
-
 /*
- * Handle options fetched from pg_db_role_setting.setconfig,
- * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
- *
- * The array parameter must be an array of TEXT (it must not be NULL).
+ * Transform array of GUC settings into a list of (name, value) pairs. The
+ * list is faster to process in cases where the settings must be applied
+ * repeatedly (e.g. for each function invocation).
  */
 void
-ProcessGUCArray(ArrayType *array,
-				GucContext context, GucSource source, GucAction action)
+TransformGUCArray(ArrayType *array, List **names, List **values)
 {
 	int			i;
 
@@ -6231,6 +6228,8 @@ ProcessGUCArray(ArrayType *array,
 	Assert(ARR_NDIM(array) == 1);
 	Assert(ARR_LBOUND(array)[0] == 1);
 
+	*names = NIL;
+	*values = NIL;
 	for (i = 1; i <= ARR_DIMS(array)[0]; i++)
 	{
 		Datum		d;
@@ -6262,14 +6261,44 @@ ProcessGUCArray(ArrayType *array,
 			continue;
 		}
 
+		*names = lappend(*names, name);
+		*values = lappend(*values, value);
+
+		pfree(s);
+	}
+}
+
+/*
+ * Handle options fetched from pg_db_role_setting.setconfig,
+ * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
+ *
+ * The array parameter must be an array of TEXT (it must not be NULL).
+ */
+void
+ProcessGUCArray(ArrayType *array,
+				GucContext context, GucSource source, GucAction action)
+{
+	List *gucNames;
+	List *gucValues;
+	ListCell *lc1;
+	ListCell *lc2;
+
+	TransformGUCArray(array, &gucNames, &gucValues);
+	forboth(lc1, gucNames, lc2, gucValues)
+	{
+		char *name = lfirst(lc1);
+		char *value = lfirst(lc2);
+
 		(void) set_config_option(name, value,
 								 context, source,
 								 action, true, 0, false);
 
 		pfree(name);
 		pfree(value);
-		pfree(s);
 	}
+
+	list_free(gucNames);
+	list_free(gucValues);
 }
 
 
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 223a19f80d..e89083ee0e 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -391,6 +391,8 @@ extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
 								   bool missing_ok);
 
+extern void TransformGUCArray(ArrayType *array, List **configNames,
+							  List **configValues);
 extern void ProcessGUCArray(ArrayType *array,
 							GucContext context, GucSource source, GucAction action);
 extern ArrayType *GUCArrayAdd(ArrayType *array, const char *name, const char *value);
-- 
2.34.1

From c87434fb268a1dea49bf6d085ff47d6c8d22fab5 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:23:51 -0700
Subject: [PATCH v3 2/3] Optimize check_search_path().

Introduce CheckIdentifierString(), which validates a string list of
identifiers without the work of actually storing them in a list.

This speeds up frequent search_path changes, such as when a function
has search_path set in proconfig. Based on profiling.
---
 src/backend/catalog/namespace.c | 13 +-----
 src/backend/utils/adt/varlena.c | 70 +++++++++++++++++++++++++++++++++
 src/include/utils/varlena.h     |  1 +
 3 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 1f76b5d7f7..f457f778cb 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -4101,19 +4101,13 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
-	char	   *rawname;
-	List	   *namelist;
-
-	/* Need a modifiable copy of string */
-	rawname = pstrdup(*newval);
+	char *rawname = *newval;
 
 	/* Parse string into list of identifiers */
-	if (!SplitIdentifierString(rawname, ',', &namelist))
+	if (!CheckIdentifierString(rawname, ','))
 	{
 		/* syntax error in name list */
 		GUC_check_errdetail("List syntax is invalid.");
-		pfree(rawname);
-		list_free(namelist);
 		return false;
 	}
 
@@ -4125,9 +4119,6 @@ check_search_path(char **newval, void **extra, GucSource source)
 	 * requirement is syntactic validity of the identifier list.
 	 */
 
-	pfree(rawname);
-	list_free(namelist);
-
 	return true;
 }
 
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index b1ec5c32ce..c8d36fd868 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3427,6 +3427,76 @@ textToQualifiedNameList(text *textval)
 	return result;
 }
 
+/*
+ * Check that an identifier list is syntactically valid; that is, that there
+ * are no mismatched quotes and that identifiers are separated with the given
+ * separator. Empty identifiers are considered acceptable if (and only if)
+ * quoted.
+ *
+ * Note that an empty string is considered okay here, though not in
+ * textToQualifiedNameList.
+ */
+bool
+CheckIdentifierString(const char *rawstring, char separator)
+{
+	const char *nextp = rawstring;
+	bool		done = false;
+
+	while (scanner_isspace(*nextp))
+		nextp++;				/* skip leading whitespace */
+
+	if (*nextp == '\0')
+		return true;			/* allow empty string */
+
+	/* At the top of the loop, we are at start of a new identifier. */
+	do
+	{
+		if (*nextp == '"')
+		{
+			for (;;)
+			{
+				nextp = strchr(nextp + 1, '"');
+				if (nextp == NULL)
+					return false;	/* mismatched quotes */
+				if (nextp[1] != '"')
+					break;		/* found end of quoted name */
+				nextp++;
+			}
+
+			nextp++;
+		}
+		else
+		{
+			/* Unquoted name --- extends to separator or whitespace */
+			const char *curname = nextp;
+			while (*nextp && *nextp != separator &&
+				   !scanner_isspace(*nextp))
+				nextp++;
+			if (curname == nextp)
+				return false;	/* empty unquoted name not allowed */
+		}
+
+		while (scanner_isspace(*nextp))
+			nextp++;			/* skip trailing whitespace */
+
+		if (*nextp == separator)
+		{
+			nextp++;
+			while (scanner_isspace(*nextp))
+				nextp++;		/* skip leading whitespace for next */
+			/* we expect another name, so done remains false */
+		}
+		else if (*nextp == '\0')
+			done = true;
+		else
+			return false;		/* invalid syntax */
+
+		/* Loop back if we didn't reach end of string */
+	} while (!done);
+
+	return true;
+}
+
 /*
  * SplitIdentifierString --- parse a string containing identifiers
  *
diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h
index 77f5b24735..4ff0cecc91 100644
--- a/src/include/utils/varlena.h
+++ b/src/include/utils/varlena.h
@@ -27,6 +27,7 @@ extern int	varstr_levenshtein_less_equal(const char *source, int slen,
 										  int ins_c, int del_c, int sub_c,
 										  int max_d, bool trusted);
 extern List *textToQualifiedNameList(text *textval);
+extern bool CheckIdentifierString(const char *rawstring, char separator);
 extern bool SplitIdentifierString(char *rawstring, char separator,
 								  List **namelist);
 extern bool SplitDirectoriesString(char *rawstring, char separator,
-- 
2.34.1

From ee777f9f668330f7578af5b7be331b625a5b1fd8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v3 3/3] Add cache for recomputeNamespacePath().

When search_path is changed to something that was previously set, and
no invalidation happened in between, use the cached list of namespace
OIDs rather than recomputing them. This avoids syscache lookups and
ACL checks.

Important when the search_path changes frequently, such as when set in
proconfig.
---
 src/backend/catalog/namespace.c | 336 ++++++++++++++++++++++++++------
 1 file changed, 273 insertions(+), 63 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index f457f778cb..58b78f798d 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
+#include "common/hashfn.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -109,11 +110,12 @@
  * activeSearchPath is always the actually active path; it points to
  * to baseSearchPath which is the list derived from namespace_search_path.
  *
- * If baseSearchPathValid is false, then baseSearchPath (and other
- * derived variables) need to be recomputed from namespace_search_path.
- * We mark it invalid upon an assignment to namespace_search_path or receipt
- * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next lookup attempt.
+ * If baseSearchPathValid is false, then baseSearchPath (and other derived
+ * variables) need to be recomputed from namespace_search_path, or retrieved
+ * from the search path cache if there haven't been any syscache
+ * invalidations.  We mark it invalid upon an assignment to
+ * namespace_search_path or receipt of a syscache invalidation event for
+ * pg_namespace.  The recomputation is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -153,6 +155,33 @@ static Oid	namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
+static bool searchPathCacheValid = false;
+static MemoryContext SearchPathCacheContext = NULL;
+
+typedef struct SearchPathCacheKey
+{
+	const char	*searchPath;
+	Oid			 roleid;
+} SearchPathCacheKey;
+
+typedef struct SearchPathCacheEntry
+{
+	SearchPathCacheKey	*key;
+	List				*oidlist;	/* namespace OIDs that pass ACL checks */
+	List				*finalPath;	/* cached final computed search path */
+	Oid					 firstNS;	/* first explicitly-listed namespace */
+	bool				 temp_missing;
+
+	/*
+	 * If true, some namespaces were previously filtered out of finalPath by a
+	 * namespace search hook. The next time it's read from the cache, we need
+	 * to recreate finalPath from the oidlist.
+	 */
+	bool				 hook_filtered;
+
+	/* needed for simplehash */
+	char				 status;
+} SearchPathCacheEntry;
 
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@@ -193,6 +222,45 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
 
+/*
+ * Recomputing the namespace path can be costly when done frequently, such as
+ * when a function has search_path set in proconfig. Add a cache that can be
+ * used when the search_path changes to a value that was previously set, and
+ * no invalidation intervened.
+ *
+ * The cache is a simple hash table in its own memory context, with a key of
+ * search_path and role ID.
+ */
+
+static inline uint32
+nspcachekey_hash(SearchPathCacheKey *key)
+{
+	const unsigned char	*bytes = (const unsigned char *)key->searchPath;
+	int					 blen  = strlen(key->searchPath);
+
+	return hash_combine(hash_bytes(bytes, blen),
+						hash_uint32(key->roleid));
+}
+
+static inline bool
+nspcachekey_equal(SearchPathCacheKey *a, SearchPathCacheKey *b)
+{
+	return a->roleid == b->roleid &&
+		strcmp(a->searchPath, b->searchPath) == 0;
+}
+
+#define SH_PREFIX		nsphash
+#define SH_ELEMENT_TYPE	SearchPathCacheEntry
+#define SH_KEY_TYPE		SearchPathCacheKey *
+#define	SH_KEY			key
+#define SH_HASH_KEY(tb, key)   	nspcachekey_hash(key)
+#define SH_EQUAL(tb, a, b)		nspcachekey_equal(a, b)
+#define	SH_SCOPE		static inline
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static nsphash_hash *SearchPathCache = NULL;
 
 /*
  * RangeVarGetRelidExtended
@@ -3370,6 +3438,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 	 */
 
 	baseSearchPathValid = false;	/* may need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 
@@ -3633,28 +3702,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
 }
 
 /*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Look up namespace IDs and perform ACL checks. Return newly-allocated list.
  */
-static void
-recomputeNamespacePath(void)
+static List *
+preprocessNamespacePath(const char *searchPath, Oid roleid,
+						bool *temp_missing)
 {
-	Oid			roleid = GetUserId();
 	char	   *rawname;
 	List	   *namelist;
 	List	   *oidlist;
-	List	   *newpath;
 	ListCell   *l;
-	bool		temp_missing;
-	Oid			firstNS;
-	bool		pathChanged;
-	MemoryContext oldcxt;
 
-	/* Do nothing if path is already valid. */
-	if (baseSearchPathValid && namespaceUser == roleid)
-		return;
-
-	/* Need a modifiable copy of namespace_search_path string */
-	rawname = pstrdup(namespace_search_path);
+	/* Need a modifiable copy */
+	rawname = pstrdup(searchPath);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -3671,7 +3731,7 @@ recomputeNamespacePath(void)
 	 * already been accepted.)	Don't make duplicate entries, either.
 	 */
 	oidlist = NIL;
-	temp_missing = false;
+	*temp_missing = false;
 	foreach(l, namelist)
 	{
 		char	   *curname = (char *) lfirst(l);
@@ -3691,10 +3751,8 @@ recomputeNamespacePath(void)
 				namespaceId = get_namespace_oid(rname, true);
 				ReleaseSysCache(tuple);
 				if (OidIsValid(namespaceId) &&
-					!list_member_oid(oidlist, namespaceId) &&
 					object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-									ACL_USAGE) == ACLCHECK_OK &&
-					InvokeNamespaceSearchHook(namespaceId, false))
+									ACL_USAGE) == ACLCHECK_OK)
 					oidlist = lappend_oid(oidlist, namespaceId);
 			}
 		}
@@ -3702,16 +3760,12 @@ recomputeNamespacePath(void)
 		{
 			/* pg_temp --- substitute temp namespace, if any */
 			if (OidIsValid(myTempNamespace))
-			{
-				if (!list_member_oid(oidlist, myTempNamespace) &&
-					InvokeNamespaceSearchHook(myTempNamespace, false))
-					oidlist = lappend_oid(oidlist, myTempNamespace);
-			}
+				oidlist = lappend_oid(oidlist, myTempNamespace);
 			else
 			{
 				/* If it ought to be the creation namespace, set flag */
 				if (oidlist == NIL)
-					temp_missing = true;
+					*temp_missing = true;
 			}
 		}
 		else
@@ -3719,63 +3773,207 @@ recomputeNamespacePath(void)
 			/* normal namespace reference */
 			namespaceId = get_namespace_oid(curname, true);
 			if (OidIsValid(namespaceId) &&
-				!list_member_oid(oidlist, namespaceId) &&
 				object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-								ACL_USAGE) == ACLCHECK_OK &&
-				InvokeNamespaceSearchHook(namespaceId, false))
+								ACL_USAGE) == ACLCHECK_OK)
 				oidlist = lappend_oid(oidlist, namespaceId);
 		}
 	}
 
+	pfree(rawname);
+	list_free(namelist);
+
+	return oidlist;
+}
+
+/*
+ * Remove duplicates, run namespace search hooks, and prepend
+ * implicitly-searched namespaces. Return newly-allocated list.
+ *
+ * All of this must happen after retrieving from cache, because we don't know
+ * what might invalidate the result from the last time the hook was
+ * invoked. It may seem that duplicate elimination is not dependent on the
+ * result of the hook, but if a hook returns different results on different
+ * calls for the same namespace ID, then it could affect the order in which
+ * that namespace appears in the final list.
+ */
+static List *
+finalNamespacePath(List *oidlist, Oid *firstNS, bool *hook_filtered)
+{
+	List		*finalPath = NIL;
+	ListCell	*lc;
+
+	*hook_filtered = false;
+	foreach(lc, oidlist)
+	{
+		Oid namespaceId = lfirst_oid(lc);
+		if (!list_member_oid(finalPath, namespaceId))
+		{
+			if (InvokeNamespaceSearchHook(namespaceId, false))
+				finalPath = lappend_oid(finalPath, namespaceId);
+			else
+				*hook_filtered = true;
+		}
+	}
+
 	/*
 	 * Remember the first member of the explicit list.  (Note: this is
 	 * nominally wrong if temp_missing, but we need it anyway to distinguish
 	 * explicit from implicit mention of pg_catalog.)
 	 */
-	if (oidlist == NIL)
-		firstNS = InvalidOid;
+	if (finalPath == NIL)
+		*firstNS = InvalidOid;
 	else
-		firstNS = linitial_oid(oidlist);
+		*firstNS = linitial_oid(finalPath);
 
 	/*
 	 * Add any implicitly-searched namespaces to the list.  Note these go on
 	 * the front, not the back; also notice that we do not check USAGE
 	 * permissions for these.
 	 */
-	if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
-		oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
+	if (!list_member_oid(finalPath, PG_CATALOG_NAMESPACE))
+		finalPath = lcons_oid(PG_CATALOG_NAMESPACE, finalPath);
 
 	if (OidIsValid(myTempNamespace) &&
-		!list_member_oid(oidlist, myTempNamespace))
-		oidlist = lcons_oid(myTempNamespace, oidlist);
+		!list_member_oid(finalPath, myTempNamespace))
+		finalPath = lcons_oid(myTempNamespace, finalPath);
+
+	return finalPath;
+}
+
+/*
+ * Retrieve search path information from the cache; or if not present,
+ * recompute it and add it to the cache.
+ *
+ * This function is also able to take the previous path and compare it to the
+ * newly-computed path. We perform the equality check in this function rather
+ * than the caller because this function can often avoid making a temporary
+ * copy of the previous path.
+ *
+ * Return OID list allocated in SearchPathCacheContext. The result is valid
+ * until the next call to this function, which may deallocate previous
+ * results.
+ */
+static List *
+cachedNamespacePath(const char *searchPath, Oid roleid,
+					Oid *firstNS, bool *temp_missing,
+					List *prevPath, bool *prevPathEqual)
+{
+	MemoryContext			 oldcxt;
+	SearchPathCacheKey		 cachekey;
+	SearchPathCacheEntry	*entry;
+	bool					 found;
+	bool					 prevPathFree = false;
+
+	/* clear the cache and start over when invalidated */
+	if (!searchPathCacheValid)
+	{
+		/* temporarily save prevPath for comparison after reset */
+		prevPath = list_copy(prevPath);
+		prevPathFree = true;
+		MemoryContextReset(SearchPathCacheContext);
+
+		/* arbitrary initial starting size of 16 elements */
+		SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
+		searchPathCacheValid = true;
+	}
+
+	cachekey.searchPath = searchPath;
+	cachekey.roleid = roleid;
+
+	entry = nsphash_insert(SearchPathCache, &cachekey, &found);
+
+	if (!found)
+	{
+		SearchPathCacheKey	*newkey;
+		List				*oidlist;
+
+		oidlist = preprocessNamespacePath(searchPath, roleid,
+										  &entry->temp_missing);
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+
+		newkey = palloc(sizeof(SearchPathCacheKey));
+		newkey->searchPath = pstrdup(searchPath);
+		newkey->roleid = roleid;
+
+		entry->key = newkey;
+		entry->oidlist = list_copy(oidlist);
+		entry->finalPath = finalNamespacePath(oidlist,
+											  &entry->firstNS,
+											  &entry->hook_filtered);
+		MemoryContextSwitchTo(oldcxt);
+
+		/* oidlist already copied into cache entry */
+		list_free(oidlist);
+	}
+	else
+	{
+		/*
+		 * The namespace search hook doesn't invalidate the cache, so if a
+		 * hook is set now (or a previously-set hook filtered out namespaces
+		 * from finalPath), then we must recreate the finalPath from the
+		 * oidlist.
+		 */
+		if (object_access_hook || entry->hook_filtered)
+		{
+			pfree(entry->finalPath);
+			oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+			entry->finalPath = finalNamespacePath(entry->oidlist,
+												  &entry->firstNS,
+												  &entry->hook_filtered);
+			MemoryContextSwitchTo(oldcxt);
+		}
+	}
+
+	*firstNS = entry->firstNS;
+	*temp_missing = entry->temp_missing;
+	*prevPathEqual = equal(prevPath, entry->finalPath);
+	if (prevPathFree)
+		list_free(prevPath);
+
+	return entry->finalPath;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+	Oid			roleid = GetUserId();
+	List	   *finalPath;
+	Oid			firstNS;
+	bool		temp_missing;
+	bool		prevPathEqual;
+	bool		pathChanged;
+
+	/* Do nothing if path is already valid. */
+	if (baseSearchPathValid && namespaceUser == roleid)
+		return;
+
+	finalPath = cachedNamespacePath(namespace_search_path, roleid,
+									&firstNS, &temp_missing, baseSearchPath,
+									&prevPathEqual);
+
+	/* may have been invalidated when retrieving from cache */
+	baseSearchPath = NULL;
 
-	/*
-	 * We want to detect the case where the effective value of the base search
-	 * path variables didn't change.  As long as we're doing so, we can avoid
-	 * copying the OID list unnecessarily.
-	 */
 	if (baseCreationNamespace == firstNS &&
 		baseTempCreationPending == temp_missing &&
-		equal(oidlist, baseSearchPath))
+		prevPathEqual)
 	{
 		pathChanged = false;
 	}
 	else
 	{
 		pathChanged = true;
-
-		/* Must save OID list in permanent storage. */
-		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-		newpath = list_copy(oidlist);
-		MemoryContextSwitchTo(oldcxt);
-
-		/* Now safe to assign to state variables. */
-		list_free(baseSearchPath);
-		baseSearchPath = newpath;
-		baseCreationNamespace = firstNS;
-		baseTempCreationPending = temp_missing;
 	}
 
+	/* Now safe to assign to state variables. */
+	baseSearchPath = finalPath;
+	baseCreationNamespace = firstNS;
+	baseTempCreationPending = temp_missing;
+
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
 	namespaceUser = roleid;
@@ -3791,11 +3989,6 @@ recomputeNamespacePath(void)
 	 */
 	if (pathChanged)
 		activePathGeneration++;
-
-	/* Clean up. */
-	pfree(rawname);
-	list_free(namelist);
-	list_free(oidlist);
 }
 
 /*
@@ -3950,6 +4143,7 @@ InitTempTableNamespace(void)
 	myTempNamespaceSubID = GetCurrentSubTransactionId();
 
 	baseSearchPathValid = false;	/* need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 /*
@@ -3975,6 +4169,7 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4016,6 +4211,7 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4130,6 +4326,10 @@ assign_search_path(const char *newval, void *extra)
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
 	 * initialization, or outside a transaction.
+	 *
+	 * This does not invalidate the search path cache, so if this value had
+	 * been previously set and no syscache invalidations happened,
+	 * recomputation may not be necessary.
 	 */
 	baseSearchPathValid = false;
 }
@@ -4164,6 +4364,10 @@ InitializeSearchPath(void)
 	}
 	else
 	{
+		SearchPathCacheContext = AllocSetContextCreate(
+			TopMemoryContext, "search_path processing cache",
+			ALLOCSET_DEFAULT_SIZES);
+
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
 		 * of pg_namespace rows.
@@ -4173,6 +4377,7 @@ InitializeSearchPath(void)
 									  (Datum) 0);
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
+		searchPathCacheValid = false;
 	}
 }
 
@@ -4183,8 +4388,13 @@ InitializeSearchPath(void)
 static void
 NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
-	/* Force search path to be recomputed on next use */
+	/*
+	 * Force search path to be recomputed on next use, also invalidating the
+	 * search path cache (because namespace names, ACLs, or role names may
+	 * have changed).
+	 */
 	baseSearchPathValid = false;
+	searchPathCacheValid = false;
 }
 
 /*
-- 
2.34.1

From f45ee8c5ea6517907000d1d3566dcc766ca25732 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:11:42 -0700
Subject: [PATCH v4 1/4] Transform proconfig for faster execution.

Store function config settings as a list of (name, value) pairs to
avoid the need to parse for each function invocation.
---
 src/backend/utils/fmgr/fmgr.c | 29 +++++++++++++++-------
 src/backend/utils/misc/guc.c  | 45 ++++++++++++++++++++++++++++-------
 src/include/utils/guc.h       |  2 ++
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9208c31fe0..6b2d7d7be3 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -612,7 +612,8 @@ struct fmgr_security_definer_cache
 {
 	FmgrInfo	flinfo;			/* lookup info for target function */
 	Oid			userid;			/* userid to set, or InvalidOid */
-	ArrayType  *proconfig;		/* GUC values to set, or NULL */
+	List	   *configNames;	/* GUC names to set, or NIL */
+	List	   *configValues;	/* GUC values to set, or NIL */
 	Datum		arg;			/* passthrough argument for plugin modules */
 };
 
@@ -634,6 +635,8 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 	FmgrInfo   *save_flinfo;
 	Oid			save_userid;
 	int			save_sec_context;
+	ListCell   *lc1;
+	ListCell   *lc2;
 	volatile int save_nestlevel;
 	PgStat_FunctionCallUsage fcusage;
 
@@ -666,8 +669,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 								&isnull);
 		if (!isnull)
 		{
+			ArrayType *array;
 			oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
-			fcache->proconfig = DatumGetArrayTypePCopy(datum);
+			array = DatumGetArrayTypeP(datum);
+			TransformGUCArray(array, &fcache->configNames,
+							  &fcache->configValues);
 			MemoryContextSwitchTo(oldcxt);
 		}
 
@@ -680,7 +686,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	/* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
 	GetUserIdAndSecContext(&save_userid, &save_sec_context);
-	if (fcache->proconfig)		/* Need a new GUC nesting level */
+	if (fcache->configNames != NIL)		/* Need a new GUC nesting level */
 		save_nestlevel = NewGUCNestLevel();
 	else
 		save_nestlevel = 0;		/* keep compiler quiet */
@@ -689,12 +695,17 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 		SetUserIdAndSecContext(fcache->userid,
 							   save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
 
-	if (fcache->proconfig)
+	forboth(lc1, fcache->configNames, lc2, fcache->configValues)
 	{
-		ProcessGUCArray(fcache->proconfig,
-						(superuser() ? PGC_SUSET : PGC_USERSET),
-						PGC_S_SESSION,
-						GUC_ACTION_SAVE);
+		GucContext	 context = superuser() ? PGC_SUSET : PGC_USERSET;
+		GucSource	 source	 = PGC_S_SESSION;
+		GucAction	 action	 = GUC_ACTION_SAVE;
+		char		*name	 = lfirst(lc1);
+		char		*value	 = lfirst(lc2);
+
+		(void) set_config_option(name, value,
+								 context, source,
+								 action, true, 0, false);
 	}
 
 	/* function manager hook */
@@ -737,7 +748,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 
 	fcinfo->flinfo = save_flinfo;
 
-	if (fcache->proconfig)
+	if (fcache->configNames != NIL)
 		AtEOXact_GUC(true, save_nestlevel);
 	if (OidIsValid(fcache->userid))
 		SetUserIdAndSecContext(save_userid, save_sec_context);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5308896c87..fa96b79192 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6213,16 +6213,13 @@ ParseLongOption(const char *string, char **name, char **value)
 			*cp = '_';
 }
 
-
 /*
- * Handle options fetched from pg_db_role_setting.setconfig,
- * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
- *
- * The array parameter must be an array of TEXT (it must not be NULL).
+ * Transform array of GUC settings into a list of (name, value) pairs. The
+ * list is faster to process in cases where the settings must be applied
+ * repeatedly (e.g. for each function invocation).
  */
 void
-ProcessGUCArray(ArrayType *array,
-				GucContext context, GucSource source, GucAction action)
+TransformGUCArray(ArrayType *array, List **names, List **values)
 {
 	int			i;
 
@@ -6231,6 +6228,8 @@ ProcessGUCArray(ArrayType *array,
 	Assert(ARR_NDIM(array) == 1);
 	Assert(ARR_LBOUND(array)[0] == 1);
 
+	*names = NIL;
+	*values = NIL;
 	for (i = 1; i <= ARR_DIMS(array)[0]; i++)
 	{
 		Datum		d;
@@ -6262,14 +6261,44 @@ ProcessGUCArray(ArrayType *array,
 			continue;
 		}
 
+		*names = lappend(*names, name);
+		*values = lappend(*values, value);
+
+		pfree(s);
+	}
+}
+
+/*
+ * Handle options fetched from pg_db_role_setting.setconfig,
+ * pg_proc.proconfig, etc.  Caller must specify proper context/source/action.
+ *
+ * The array parameter must be an array of TEXT (it must not be NULL).
+ */
+void
+ProcessGUCArray(ArrayType *array,
+				GucContext context, GucSource source, GucAction action)
+{
+	List *gucNames;
+	List *gucValues;
+	ListCell *lc1;
+	ListCell *lc2;
+
+	TransformGUCArray(array, &gucNames, &gucValues);
+	forboth(lc1, gucNames, lc2, gucValues)
+	{
+		char *name = lfirst(lc1);
+		char *value = lfirst(lc2);
+
 		(void) set_config_option(name, value,
 								 context, source,
 								 action, true, 0, false);
 
 		pfree(name);
 		pfree(value);
-		pfree(s);
 	}
+
+	list_free(gucNames);
+	list_free(gucValues);
 }
 
 
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 223a19f80d..e89083ee0e 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -391,6 +391,8 @@ extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
 								   bool missing_ok);
 
+extern void TransformGUCArray(ArrayType *array, List **configNames,
+							  List **configValues);
 extern void ProcessGUCArray(ArrayType *array,
 							GucContext context, GucSource source, GucAction action);
 extern ArrayType *GUCArrayAdd(ArrayType *array, const char *name, const char *value);
-- 
2.34.1

From 313136c308641d2dc47a19330cd134451224918e Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 27 Jul 2023 15:23:51 -0700
Subject: [PATCH v4 2/4] Optimize check_search_path().

Introduce CheckIdentifierString(), which validates a string list of
identifiers without the work of actually storing them in a list.

This speeds up frequent search_path changes, such as when a function
has search_path set in proconfig. Based on profiling.
---
 src/backend/catalog/namespace.c | 13 +-----
 src/backend/utils/adt/varlena.c | 70 +++++++++++++++++++++++++++++++++
 src/include/utils/varlena.h     |  1 +
 3 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 1f76b5d7f7..f457f778cb 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -4101,19 +4101,13 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
-	char	   *rawname;
-	List	   *namelist;
-
-	/* Need a modifiable copy of string */
-	rawname = pstrdup(*newval);
+	char *rawname = *newval;
 
 	/* Parse string into list of identifiers */
-	if (!SplitIdentifierString(rawname, ',', &namelist))
+	if (!CheckIdentifierString(rawname, ','))
 	{
 		/* syntax error in name list */
 		GUC_check_errdetail("List syntax is invalid.");
-		pfree(rawname);
-		list_free(namelist);
 		return false;
 	}
 
@@ -4125,9 +4119,6 @@ check_search_path(char **newval, void **extra, GucSource source)
 	 * requirement is syntactic validity of the identifier list.
 	 */
 
-	pfree(rawname);
-	list_free(namelist);
-
 	return true;
 }
 
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index b1ec5c32ce..c8d36fd868 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3427,6 +3427,76 @@ textToQualifiedNameList(text *textval)
 	return result;
 }
 
+/*
+ * Check that an identifier list is syntactically valid; that is, that there
+ * are no mismatched quotes and that identifiers are separated with the given
+ * separator. Empty identifiers are considered acceptable if (and only if)
+ * quoted.
+ *
+ * Note that an empty string is considered okay here, though not in
+ * textToQualifiedNameList.
+ */
+bool
+CheckIdentifierString(const char *rawstring, char separator)
+{
+	const char *nextp = rawstring;
+	bool		done = false;
+
+	while (scanner_isspace(*nextp))
+		nextp++;				/* skip leading whitespace */
+
+	if (*nextp == '\0')
+		return true;			/* allow empty string */
+
+	/* At the top of the loop, we are at start of a new identifier. */
+	do
+	{
+		if (*nextp == '"')
+		{
+			for (;;)
+			{
+				nextp = strchr(nextp + 1, '"');
+				if (nextp == NULL)
+					return false;	/* mismatched quotes */
+				if (nextp[1] != '"')
+					break;		/* found end of quoted name */
+				nextp++;
+			}
+
+			nextp++;
+		}
+		else
+		{
+			/* Unquoted name --- extends to separator or whitespace */
+			const char *curname = nextp;
+			while (*nextp && *nextp != separator &&
+				   !scanner_isspace(*nextp))
+				nextp++;
+			if (curname == nextp)
+				return false;	/* empty unquoted name not allowed */
+		}
+
+		while (scanner_isspace(*nextp))
+			nextp++;			/* skip trailing whitespace */
+
+		if (*nextp == separator)
+		{
+			nextp++;
+			while (scanner_isspace(*nextp))
+				nextp++;		/* skip leading whitespace for next */
+			/* we expect another name, so done remains false */
+		}
+		else if (*nextp == '\0')
+			done = true;
+		else
+			return false;		/* invalid syntax */
+
+		/* Loop back if we didn't reach end of string */
+	} while (!done);
+
+	return true;
+}
+
 /*
  * SplitIdentifierString --- parse a string containing identifiers
  *
diff --git a/src/include/utils/varlena.h b/src/include/utils/varlena.h
index 77f5b24735..4ff0cecc91 100644
--- a/src/include/utils/varlena.h
+++ b/src/include/utils/varlena.h
@@ -27,6 +27,7 @@ extern int	varstr_levenshtein_less_equal(const char *source, int slen,
 										  int ins_c, int del_c, int sub_c,
 										  int max_d, bool trusted);
 extern List *textToQualifiedNameList(text *textval);
+extern bool CheckIdentifierString(const char *rawstring, char separator);
 extern bool SplitIdentifierString(char *rawstring, char separator,
 								  List **namelist);
 extern bool SplitDirectoriesString(char *rawstring, char separator,
-- 
2.34.1

From 8f704c05d8f8538807c8430bc32fba2e043c8fb2 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v4 3/4] Add cache for recomputeNamespacePath().

When search_path is changed to something that was previously set, and
no invalidation happened in between, use the cached list of namespace
OIDs rather than recomputing them. This avoids syscache lookups and
ACL checks.

Important when the search_path changes frequently, such as when set in
proconfig.
---
 src/backend/catalog/namespace.c | 349 ++++++++++++++++++++++++++------
 1 file changed, 286 insertions(+), 63 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index f457f778cb..89a1c0bf98 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
+#include "common/hashfn.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -109,11 +110,12 @@
  * activeSearchPath is always the actually active path; it points to
  * to baseSearchPath which is the list derived from namespace_search_path.
  *
- * If baseSearchPathValid is false, then baseSearchPath (and other
- * derived variables) need to be recomputed from namespace_search_path.
- * We mark it invalid upon an assignment to namespace_search_path or receipt
- * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next lookup attempt.
+ * If baseSearchPathValid is false, then baseSearchPath (and other derived
+ * variables) need to be recomputed from namespace_search_path, or retrieved
+ * from the search path cache if there haven't been any syscache
+ * invalidations.  We mark it invalid upon an assignment to
+ * namespace_search_path or receipt of a syscache invalidation event for
+ * pg_namespace.  The recomputation is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -153,6 +155,35 @@ static Oid	namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
+static bool searchPathCacheValid = false;
+static MemoryContext SearchPathCacheContext = NULL;
+
+typedef struct SearchPathCacheKey
+{
+	const char	*searchPath;
+	Oid			 roleid;
+} SearchPathCacheKey;
+
+typedef struct SearchPathCacheEntry
+{
+	SearchPathCacheKey	*key;
+	List				*oidlist;	/* namespace OIDs that pass ACL checks */
+	List				*finalPath;	/* cached final computed search path */
+	Oid					 firstNS;	/* first explicitly-listed namespace */
+	bool				 temp_missing;
+
+	/*
+	 * If true, some namespaces were previously filtered out of finalPath by a
+	 * namespace search hook. The next time it's read from the cache, we need
+	 * to recreate finalPath from the oidlist.
+	 */
+	bool				 hook_filtered;
+
+	/* needed for simplehash */
+	char				 status;
+} SearchPathCacheEntry;
+
+static SearchPathCacheEntry *CurrentSearchPathCacheEntry = NULL;
 
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@@ -193,6 +224,45 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
 
+/*
+ * Recomputing the namespace path can be costly when done frequently, such as
+ * when a function has search_path set in proconfig. Add a cache that can be
+ * used when the search_path changes to a value that was previously set, and
+ * no invalidation intervened.
+ *
+ * The cache is a simple hash table in its own memory context, with a key of
+ * search_path and role ID.
+ */
+
+static inline uint32
+nspcachekey_hash(SearchPathCacheKey *key)
+{
+	const unsigned char	*bytes = (const unsigned char *)key->searchPath;
+	int					 blen  = strlen(key->searchPath);
+
+	return hash_combine(hash_bytes(bytes, blen),
+						hash_uint32(key->roleid));
+}
+
+static inline bool
+nspcachekey_equal(SearchPathCacheKey *a, SearchPathCacheKey *b)
+{
+	return a->roleid == b->roleid &&
+		strcmp(a->searchPath, b->searchPath) == 0;
+}
+
+#define SH_PREFIX		nsphash
+#define SH_ELEMENT_TYPE	SearchPathCacheEntry
+#define SH_KEY_TYPE		SearchPathCacheKey *
+#define	SH_KEY			key
+#define SH_HASH_KEY(tb, key)   	nspcachekey_hash(key)
+#define SH_EQUAL(tb, a, b)		nspcachekey_equal(a, b)
+#define	SH_SCOPE		static inline
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static nsphash_hash *SearchPathCache = NULL;
 
 /*
  * RangeVarGetRelidExtended
@@ -3370,6 +3440,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 	 */
 
 	baseSearchPathValid = false;	/* may need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 
@@ -3633,28 +3704,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
 }
 
 /*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Look up namespace IDs and perform ACL checks. Return newly-allocated list.
  */
-static void
-recomputeNamespacePath(void)
+static List *
+preprocessNamespacePath(const char *searchPath, Oid roleid,
+						bool *temp_missing)
 {
-	Oid			roleid = GetUserId();
 	char	   *rawname;
 	List	   *namelist;
 	List	   *oidlist;
-	List	   *newpath;
 	ListCell   *l;
-	bool		temp_missing;
-	Oid			firstNS;
-	bool		pathChanged;
-	MemoryContext oldcxt;
 
-	/* Do nothing if path is already valid. */
-	if (baseSearchPathValid && namespaceUser == roleid)
-		return;
-
-	/* Need a modifiable copy of namespace_search_path string */
-	rawname = pstrdup(namespace_search_path);
+	/* Need a modifiable copy */
+	rawname = pstrdup(searchPath);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -3671,7 +3733,7 @@ recomputeNamespacePath(void)
 	 * already been accepted.)	Don't make duplicate entries, either.
 	 */
 	oidlist = NIL;
-	temp_missing = false;
+	*temp_missing = false;
 	foreach(l, namelist)
 	{
 		char	   *curname = (char *) lfirst(l);
@@ -3691,10 +3753,8 @@ recomputeNamespacePath(void)
 				namespaceId = get_namespace_oid(rname, true);
 				ReleaseSysCache(tuple);
 				if (OidIsValid(namespaceId) &&
-					!list_member_oid(oidlist, namespaceId) &&
 					object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-									ACL_USAGE) == ACLCHECK_OK &&
-					InvokeNamespaceSearchHook(namespaceId, false))
+									ACL_USAGE) == ACLCHECK_OK)
 					oidlist = lappend_oid(oidlist, namespaceId);
 			}
 		}
@@ -3702,16 +3762,12 @@ recomputeNamespacePath(void)
 		{
 			/* pg_temp --- substitute temp namespace, if any */
 			if (OidIsValid(myTempNamespace))
-			{
-				if (!list_member_oid(oidlist, myTempNamespace) &&
-					InvokeNamespaceSearchHook(myTempNamespace, false))
-					oidlist = lappend_oid(oidlist, myTempNamespace);
-			}
+				oidlist = lappend_oid(oidlist, myTempNamespace);
 			else
 			{
 				/* If it ought to be the creation namespace, set flag */
 				if (oidlist == NIL)
-					temp_missing = true;
+					*temp_missing = true;
 			}
 		}
 		else
@@ -3719,63 +3775,218 @@ recomputeNamespacePath(void)
 			/* normal namespace reference */
 			namespaceId = get_namespace_oid(curname, true);
 			if (OidIsValid(namespaceId) &&
-				!list_member_oid(oidlist, namespaceId) &&
 				object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-								ACL_USAGE) == ACLCHECK_OK &&
-				InvokeNamespaceSearchHook(namespaceId, false))
+								ACL_USAGE) == ACLCHECK_OK)
 				oidlist = lappend_oid(oidlist, namespaceId);
 		}
 	}
 
+	pfree(rawname);
+	list_free(namelist);
+
+	return oidlist;
+}
+
+/*
+ * Remove duplicates, run namespace search hooks, and prepend
+ * implicitly-searched namespaces. Return newly-allocated list.
+ *
+ * All of this must happen after retrieving from cache, because we don't know
+ * what might invalidate the result from the last time the hook was
+ * invoked. It may seem that duplicate elimination is not dependent on the
+ * result of the hook, but if a hook returns different results on different
+ * calls for the same namespace ID, then it could affect the order in which
+ * that namespace appears in the final list.
+ */
+static List *
+finalNamespacePath(List *oidlist, Oid *firstNS, bool *hook_filtered)
+{
+	List		*finalPath = NIL;
+	ListCell	*lc;
+
+	*hook_filtered = false;
+	foreach(lc, oidlist)
+	{
+		Oid namespaceId = lfirst_oid(lc);
+		if (!list_member_oid(finalPath, namespaceId))
+		{
+			if (InvokeNamespaceSearchHook(namespaceId, false))
+				finalPath = lappend_oid(finalPath, namespaceId);
+			else
+				*hook_filtered = true;
+		}
+	}
+
 	/*
 	 * Remember the first member of the explicit list.  (Note: this is
 	 * nominally wrong if temp_missing, but we need it anyway to distinguish
 	 * explicit from implicit mention of pg_catalog.)
 	 */
-	if (oidlist == NIL)
-		firstNS = InvalidOid;
+	if (finalPath == NIL)
+		*firstNS = InvalidOid;
 	else
-		firstNS = linitial_oid(oidlist);
+		*firstNS = linitial_oid(finalPath);
 
 	/*
 	 * Add any implicitly-searched namespaces to the list.  Note these go on
 	 * the front, not the back; also notice that we do not check USAGE
 	 * permissions for these.
 	 */
-	if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
-		oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
+	if (!list_member_oid(finalPath, PG_CATALOG_NAMESPACE))
+		finalPath = lcons_oid(PG_CATALOG_NAMESPACE, finalPath);
 
 	if (OidIsValid(myTempNamespace) &&
-		!list_member_oid(oidlist, myTempNamespace))
-		oidlist = lcons_oid(myTempNamespace, oidlist);
+		!list_member_oid(finalPath, myTempNamespace))
+		finalPath = lcons_oid(myTempNamespace, finalPath);
+
+	return finalPath;
+}
+
+/*
+ * Retrieve search path information from the cache; or if not there, fill
+ * it. The returned entry is valid until the next call to this function.
+ *
+ * Invalidation takes two forms: The first is ordinary syscache invalidation
+ * clears the whole cache, and every entry must be rebuilt from the string
+ * form (finding OIDs from the namespace names and performing ACL checks). The
+ * second form of invalidation is when an object_access_hook is present, in
+ * which case we rebuild the entry by calling the hook for each OID in the
+ * oidlist, which is much faster than rebuilding from the string.
+ *
+ * We also determine if the new entry's finalPath (which is a list of OIDs) is equal
+ * to the previous one. It's more efficient to do so in this function, because
+ * we can avoid copying the previous list unless there is a syscache
+ * invalidation (in which case we must copy it before resetting
+ * SearchPathCacheContext).
+ */
+static SearchPathCacheEntry *
+cachedNamespacePath(const char *searchPath, Oid roleid, bool *prevPathEqual)
+{
+	MemoryContext			 oldcxt;
+	SearchPathCacheKey		 cachekey;
+	SearchPathCacheEntry	*entry;
+	bool					 found;
+	SearchPathCacheEntry	*prevEntry = CurrentSearchPathCacheEntry;
+	List					*prevPath = prevEntry ? prevEntry->finalPath : NIL;
+	List					*prevPathCopy = NIL;
+
+	/* clear the cache and start over when invalidated */
+	if (!searchPathCacheValid)
+	{
+		/* temporarily save prevPath for comparison after reset */
+		prevPathCopy = list_copy(prevPath);
+		prevPath = prevPathCopy;
+
+		MemoryContextReset(SearchPathCacheContext);
+
+		/* arbitrary initial starting size of 16 elements */
+		SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
+		searchPathCacheValid = true;
+		prevEntry = NULL;
+	}
+
+	if (prevEntry && roleid == prevEntry->key->roleid &&
+		strcmp(searchPath, prevEntry->key->searchPath) == 0)
+	{
+		/*
+		 * If search_path has changed and then changed back before
+		 * recomputeNamespacePath is called, then the hash lookup will just
+		 * find the very same entry that we already have. Optimize this case.
+		 */
+		entry = prevEntry;
+		found = true;
+	}
+	else
+	{
+		/* otherwise do a quick lookup */
+		cachekey.searchPath = searchPath;
+		cachekey.roleid = roleid;
+
+		entry = nsphash_insert(SearchPathCache, &cachekey, &found);
+	}
+
+	/* initialize entry, though don't set finalPath (etc.) */
+	if (!found)
+	{
+		SearchPathCacheKey	*newkey;
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+
+		newkey = palloc(sizeof(SearchPathCacheKey));
+		newkey->searchPath = pstrdup(searchPath);
+		newkey->roleid = roleid;
+
+		entry->key = newkey;
+		entry->oidlist = preprocessNamespacePath(searchPath, roleid,
+												 &entry->temp_missing);
+		entry->finalPath = finalNamespacePath(entry->oidlist,
+											  &entry->firstNS,
+											  &entry->hook_filtered);
+		MemoryContextSwitchTo(oldcxt);
+	}
 
 	/*
-	 * We want to detect the case where the effective value of the base search
-	 * path variables didn't change.  As long as we're doing so, we can avoid
-	 * copying the OID list unnecessarily.
+	 * If a hook is set now, or if a previously-set hook filtered out some
+	 * namespaces from finalPath, we must re-finalize.
 	 */
-	if (baseCreationNamespace == firstNS &&
-		baseTempCreationPending == temp_missing &&
-		equal(oidlist, baseSearchPath))
+	if (found && (object_access_hook || entry->hook_filtered))
+	{
+		/*
+		 * Don't free the existing entry->finalPath yet, as prevPath may be an
+		 * alias for it. Treat it as a copy that will be freed later.
+		 */
+		Assert(prevPathCopy == NIL);
+		prevPathCopy = entry->finalPath;
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+		entry->finalPath = finalNamespacePath(entry->oidlist,
+											  &entry->firstNS,
+											  &entry->hook_filtered);
+		MemoryContextSwitchTo(oldcxt);
+	}
+
+	*prevPathEqual = equal(prevPath, entry->finalPath);
+	list_free(prevPathCopy);
+
+	CurrentSearchPathCacheEntry = entry;
+
+	return entry;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+	Oid			roleid = GetUserId();
+	bool		prevPathEqual;
+	bool		pathChanged;
+	SearchPathCacheEntry *entry;
+
+	/* Do nothing if path is already valid. */
+	if (baseSearchPathValid && namespaceUser == roleid)
+		return;
+
+	entry = cachedNamespacePath(namespace_search_path,
+								roleid, &prevPathEqual);
+
+	if (baseCreationNamespace == entry->firstNS &&
+		baseTempCreationPending == entry->temp_missing &&
+		prevPathEqual)
 	{
 		pathChanged = false;
 	}
 	else
 	{
 		pathChanged = true;
-
-		/* Must save OID list in permanent storage. */
-		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-		newpath = list_copy(oidlist);
-		MemoryContextSwitchTo(oldcxt);
-
-		/* Now safe to assign to state variables. */
-		list_free(baseSearchPath);
-		baseSearchPath = newpath;
-		baseCreationNamespace = firstNS;
-		baseTempCreationPending = temp_missing;
 	}
 
+	/* Now safe to assign to state variables. */
+	baseSearchPath = entry->finalPath;
+	baseCreationNamespace = entry->firstNS;
+	baseTempCreationPending = entry->temp_missing;
+
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
 	namespaceUser = roleid;
@@ -3791,11 +4002,6 @@ recomputeNamespacePath(void)
 	 */
 	if (pathChanged)
 		activePathGeneration++;
-
-	/* Clean up. */
-	pfree(rawname);
-	list_free(namelist);
-	list_free(oidlist);
 }
 
 /*
@@ -3950,6 +4156,7 @@ InitTempTableNamespace(void)
 	myTempNamespaceSubID = GetCurrentSubTransactionId();
 
 	baseSearchPathValid = false;	/* need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 /*
@@ -3975,6 +4182,7 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4016,6 +4224,7 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4130,6 +4339,10 @@ assign_search_path(const char *newval, void *extra)
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
 	 * initialization, or outside a transaction.
+	 *
+	 * This does not invalidate the search path cache, so if this value had
+	 * been previously set and no syscache invalidations happened,
+	 * recomputation may not be necessary.
 	 */
 	baseSearchPathValid = false;
 }
@@ -4164,6 +4377,10 @@ InitializeSearchPath(void)
 	}
 	else
 	{
+		SearchPathCacheContext = AllocSetContextCreate(
+			TopMemoryContext, "search_path processing cache",
+			ALLOCSET_DEFAULT_SIZES);
+
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
 		 * of pg_namespace rows.
@@ -4173,6 +4390,7 @@ InitializeSearchPath(void)
 									  (Datum) 0);
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
+		searchPathCacheValid = false;
 	}
 }
 
@@ -4183,8 +4401,13 @@ InitializeSearchPath(void)
 static void
 NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
-	/* Force search path to be recomputed on next use */
+	/*
+	 * Force search path to be recomputed on next use, also invalidating the
+	 * search path cache (because namespace names, ACLs, or role names may
+	 * have changed).
+	 */
 	baseSearchPathValid = false;
+	searchPathCacheValid = false;
 }
 
 /*
-- 
2.34.1

From 08d688a4346a5bb44f702ebbd46f75d5fc7bc01d Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Mon, 7 Aug 2023 11:12:07 -0700
Subject: [PATCH v4 4/4] wip

---
 src/backend/utils/fmgr/fmgr.c |  9 +++--
 src/backend/utils/misc/guc.c  | 63 +++++++++++++++++++++++++++++++++++
 src/include/utils/guc.h       |  1 +
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 6b2d7d7be3..40c6137b01 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -703,9 +703,12 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
 		char		*name	 = lfirst(lc1);
 		char		*value	 = lfirst(lc2);
 
-		(void) set_config_option(name, value,
-								 context, source,
-								 action, true, 0, false);
+		if (!strcmp(name, "search_path"))
+			fast_set_search_path(value);
+		else
+			(void) set_config_option(name, value,
+									 context, source,
+									 action, true, 0, false);
 	}
 
 	/* function manager hook */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fa96b79192..a34a81217f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -46,6 +46,7 @@
 #include "utils/builtins.h"
 #include "utils/conffiles.h"
 #include "utils/float.h"
+#include "utils/guc_hooks.h"
 #include "utils/guc_tables.h"
 #include "utils/memutils.h"
 #include "utils/timestamp.h"
@@ -225,6 +226,7 @@ static bool reporting_enabled;	/* true to enable GUC_REPORT */
 
 static int	GUCNestLevel = 0;	/* 1 when in main transaction */
 
+struct config_string *search_path_conf = NULL;
 
 static int	guc_var_compare(const void *a, const void *b);
 static uint32 guc_name_hash(const void *key, Size keysize);
@@ -4135,6 +4137,67 @@ set_config_option_ext(const char *name, const char *value,
 	return changeVal ? 1 : -1;
 }
 
+/*
+ * optimized version of:
+ *   set_config_option("search_path", searchPath,
+ *                     superuser() ? PGC_SUSET : PGC_USERSET,
+                       PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
+ *                     
+ */
+void
+fast_set_search_path(const char *searchPath)
+{
+	char		*newval	  = guc_strdup(ERROR, searchPath);
+	void		*newextra = NULL;
+	Oid			 srole	  = GetUserId();
+	GucContext	 context  = superuser() ? PGC_SUSET : PGC_USERSET;
+	GucSource    source	  = PGC_S_SESSION;
+
+	if (search_path_conf == NULL)
+	{
+		struct config_generic *record;
+		record = find_option("search_path", true, false, ERROR);
+		Assert(record != NULL);
+		search_path_conf = (struct config_string *) record;
+	}
+
+	/* Reset variables that might be set by hook */
+	GUC_check_errcode_value = ERRCODE_INVALID_PARAMETER_VALUE;
+	GUC_check_errmsg_string = NULL;
+	GUC_check_errdetail_string = NULL;
+	GUC_check_errhint_string = NULL;
+
+	if (!check_search_path(&newval, NULL, source))
+	{
+		guc_free(newval);
+		ereport(ERROR,
+				(errcode(GUC_check_errcode_value),
+				 GUC_check_errmsg_string ?
+				 errmsg_internal("%s", GUC_check_errmsg_string) :
+				 errmsg("invalid value for parameter \"%s\": \"%s\"",
+						search_path_conf->gen.name, newval ? newval : ""),
+				 GUC_check_errdetail_string ?
+				 errdetail_internal("%s", GUC_check_errdetail_string) : 0,
+				 GUC_check_errhint_string ?
+				 errhint("%s", GUC_check_errhint_string) : 0));
+	}
+
+	assign_search_path(newval, newextra);
+	set_string_field(search_path_conf, search_path_conf->variable, newval);
+	set_extra_field(&search_path_conf->gen, &search_path_conf->gen.extra,
+					newextra);
+	set_guc_source(&search_path_conf->gen, source);
+	search_path_conf->gen.scontext = context;
+	search_path_conf->gen.srole = srole;
+
+	/* Perhaps we didn't install newval anywhere */
+	if (newval && !string_field_used(search_path_conf, newval))
+		guc_free(newval);
+	/* Perhaps we didn't install newextra anywhere */
+	if (newextra && !extra_field_used(&search_path_conf->gen, newextra))
+		guc_free(newextra);
+}
+
 
 /*
  * Set the fields for source file and line number the setting came from.
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e89083ee0e..d916ecf2ed 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -387,6 +387,7 @@ extern int	set_config_option_ext(const char *name, const char *value,
 								  Oid srole,
 								  GucAction action, bool changeVal, int elevel,
 								  bool is_reload);
+extern void fast_set_search_path(const char *searchPath);
 extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
 extern char *GetConfigOptionByName(const char *name, const char **varname,
 								   bool missing_ok);
-- 
2.34.1

From f5b055aea1bf08928de1bffcfd7b202e28847595 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v5 3/3] Add cache for recomputeNamespacePath().

When search_path is changed to something that was previously set, and
no invalidation happened in between, use the cached list of namespace
OIDs rather than recomputing them. This avoids syscache lookups and
ACL checks.

Important when the search_path changes frequently, such as when set in
proconfig.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 349 ++++++++++++++++++++++++++------
 1 file changed, 286 insertions(+), 63 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 4ceae038ec..8b97b3b529 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
+#include "common/hashfn.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -109,11 +110,12 @@
  * activeSearchPath is always the actually active path; it points to
  * to baseSearchPath which is the list derived from namespace_search_path.
  *
- * If baseSearchPathValid is false, then baseSearchPath (and other
- * derived variables) need to be recomputed from namespace_search_path.
- * We mark it invalid upon an assignment to namespace_search_path or receipt
- * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next lookup attempt.
+ * If baseSearchPathValid is false, then baseSearchPath (and other derived
+ * variables) need to be recomputed from namespace_search_path, or retrieved
+ * from the search path cache if there haven't been any syscache
+ * invalidations.  We mark it invalid upon an assignment to
+ * namespace_search_path or receipt of a syscache invalidation event for
+ * pg_namespace.  The recomputation is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -153,6 +155,35 @@ static Oid	namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
+static bool searchPathCacheValid = false;
+static MemoryContext SearchPathCacheContext = NULL;
+
+typedef struct SearchPathCacheKey
+{
+	const char	*searchPath;
+	Oid			 roleid;
+} SearchPathCacheKey;
+
+typedef struct SearchPathCacheEntry
+{
+	SearchPathCacheKey	*key;
+	List				*oidlist;	/* namespace OIDs that pass ACL checks */
+	List				*finalPath;	/* cached final computed search path */
+	Oid					 firstNS;	/* first explicitly-listed namespace */
+	bool				 temp_missing;
+
+	/*
+	 * If true, some namespaces were previously filtered out of finalPath by a
+	 * namespace search hook. The next time it's read from the cache, we need
+	 * to recreate finalPath from the oidlist.
+	 */
+	bool				 hook_filtered;
+
+	/* needed for simplehash */
+	char				 status;
+} SearchPathCacheEntry;
+
+static SearchPathCacheEntry *CurrentSearchPathCacheEntry = NULL;
 
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@@ -193,6 +224,45 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
 
+/*
+ * Recomputing the namespace path can be costly when done frequently, such as
+ * when a function has search_path set in proconfig. Add a cache that can be
+ * used when the search_path changes to a value that was previously set, and
+ * no invalidation intervened.
+ *
+ * The cache is a simple hash table in its own memory context, with a key of
+ * search_path and role ID.
+ */
+
+static inline uint32
+nspcachekey_hash(SearchPathCacheKey *key)
+{
+	const unsigned char	*bytes = (const unsigned char *)key->searchPath;
+	int					 blen  = strlen(key->searchPath);
+
+	return hash_combine(hash_bytes(bytes, blen),
+						hash_uint32(key->roleid));
+}
+
+static inline bool
+nspcachekey_equal(SearchPathCacheKey *a, SearchPathCacheKey *b)
+{
+	return a->roleid == b->roleid &&
+		strcmp(a->searchPath, b->searchPath) == 0;
+}
+
+#define SH_PREFIX		nsphash
+#define SH_ELEMENT_TYPE	SearchPathCacheEntry
+#define SH_KEY_TYPE		SearchPathCacheKey *
+#define	SH_KEY			key
+#define SH_HASH_KEY(tb, key)   	nspcachekey_hash(key)
+#define SH_EQUAL(tb, a, b)		nspcachekey_equal(a, b)
+#define	SH_SCOPE		static inline
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static nsphash_hash *SearchPathCache = NULL;
 
 /*
  * RangeVarGetRelidExtended
@@ -3370,6 +3440,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 	 */
 
 	baseSearchPathValid = false;	/* may need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 
@@ -3633,28 +3704,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
 }
 
 /*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Look up namespace IDs and perform ACL checks. Return newly-allocated list.
  */
-static void
-recomputeNamespacePath(void)
+static List *
+preprocessNamespacePath(const char *searchPath, Oid roleid,
+						bool *temp_missing)
 {
-	Oid			roleid = GetUserId();
 	char	   *rawname;
 	List	   *namelist;
 	List	   *oidlist;
-	List	   *newpath;
 	ListCell   *l;
-	bool		temp_missing;
-	Oid			firstNS;
-	bool		pathChanged;
-	MemoryContext oldcxt;
 
-	/* Do nothing if path is already valid. */
-	if (baseSearchPathValid && namespaceUser == roleid)
-		return;
-
-	/* Need a modifiable copy of namespace_search_path string */
-	rawname = pstrdup(namespace_search_path);
+	/* Need a modifiable copy */
+	rawname = pstrdup(searchPath);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -3671,7 +3733,7 @@ recomputeNamespacePath(void)
 	 * already been accepted.)	Don't make duplicate entries, either.
 	 */
 	oidlist = NIL;
-	temp_missing = false;
+	*temp_missing = false;
 	foreach(l, namelist)
 	{
 		char	   *curname = (char *) lfirst(l);
@@ -3691,10 +3753,8 @@ recomputeNamespacePath(void)
 				namespaceId = get_namespace_oid(rname, true);
 				ReleaseSysCache(tuple);
 				if (OidIsValid(namespaceId) &&
-					!list_member_oid(oidlist, namespaceId) &&
 					object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-									ACL_USAGE) == ACLCHECK_OK &&
-					InvokeNamespaceSearchHook(namespaceId, false))
+									ACL_USAGE) == ACLCHECK_OK)
 					oidlist = lappend_oid(oidlist, namespaceId);
 			}
 		}
@@ -3702,16 +3762,12 @@ recomputeNamespacePath(void)
 		{
 			/* pg_temp --- substitute temp namespace, if any */
 			if (OidIsValid(myTempNamespace))
-			{
-				if (!list_member_oid(oidlist, myTempNamespace) &&
-					InvokeNamespaceSearchHook(myTempNamespace, false))
-					oidlist = lappend_oid(oidlist, myTempNamespace);
-			}
+				oidlist = lappend_oid(oidlist, myTempNamespace);
 			else
 			{
 				/* If it ought to be the creation namespace, set flag */
 				if (oidlist == NIL)
-					temp_missing = true;
+					*temp_missing = true;
 			}
 		}
 		else
@@ -3719,63 +3775,218 @@ recomputeNamespacePath(void)
 			/* normal namespace reference */
 			namespaceId = get_namespace_oid(curname, true);
 			if (OidIsValid(namespaceId) &&
-				!list_member_oid(oidlist, namespaceId) &&
 				object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-								ACL_USAGE) == ACLCHECK_OK &&
-				InvokeNamespaceSearchHook(namespaceId, false))
+								ACL_USAGE) == ACLCHECK_OK)
 				oidlist = lappend_oid(oidlist, namespaceId);
 		}
 	}
 
+	pfree(rawname);
+	list_free(namelist);
+
+	return oidlist;
+}
+
+/*
+ * Remove duplicates, run namespace search hooks, and prepend
+ * implicitly-searched namespaces. Return newly-allocated list.
+ *
+ * All of this must happen after retrieving from cache, because we don't know
+ * what might invalidate the result from the last time the hook was
+ * invoked. It may seem that duplicate elimination is not dependent on the
+ * result of the hook, but if a hook returns different results on different
+ * calls for the same namespace ID, then it could affect the order in which
+ * that namespace appears in the final list.
+ */
+static List *
+finalNamespacePath(List *oidlist, Oid *firstNS, bool *hook_filtered)
+{
+	List		*finalPath = NIL;
+	ListCell	*lc;
+
+	*hook_filtered = false;
+	foreach(lc, oidlist)
+	{
+		Oid namespaceId = lfirst_oid(lc);
+		if (!list_member_oid(finalPath, namespaceId))
+		{
+			if (InvokeNamespaceSearchHook(namespaceId, false))
+				finalPath = lappend_oid(finalPath, namespaceId);
+			else
+				*hook_filtered = true;
+		}
+	}
+
 	/*
 	 * Remember the first member of the explicit list.  (Note: this is
 	 * nominally wrong if temp_missing, but we need it anyway to distinguish
 	 * explicit from implicit mention of pg_catalog.)
 	 */
-	if (oidlist == NIL)
-		firstNS = InvalidOid;
+	if (finalPath == NIL)
+		*firstNS = InvalidOid;
 	else
-		firstNS = linitial_oid(oidlist);
+		*firstNS = linitial_oid(finalPath);
 
 	/*
 	 * Add any implicitly-searched namespaces to the list.  Note these go on
 	 * the front, not the back; also notice that we do not check USAGE
 	 * permissions for these.
 	 */
-	if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
-		oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
+	if (!list_member_oid(finalPath, PG_CATALOG_NAMESPACE))
+		finalPath = lcons_oid(PG_CATALOG_NAMESPACE, finalPath);
 
 	if (OidIsValid(myTempNamespace) &&
-		!list_member_oid(oidlist, myTempNamespace))
-		oidlist = lcons_oid(myTempNamespace, oidlist);
+		!list_member_oid(finalPath, myTempNamespace))
+		finalPath = lcons_oid(myTempNamespace, finalPath);
+
+	return finalPath;
+}
+
+/*
+ * Retrieve search path information from the cache; or if not there, fill
+ * it. The returned entry is valid until the next call to this function.
+ *
+ * Invalidation takes two forms: The first is ordinary syscache invalidation
+ * clears the whole cache, and every entry must be rebuilt from the string
+ * form (finding OIDs from the namespace names and performing ACL checks). The
+ * second form of invalidation is when an object_access_hook is present, in
+ * which case we rebuild the entry by calling the hook for each OID in the
+ * oidlist, which is much faster than rebuilding from the string.
+ *
+ * We also determine if the new entry's finalPath (which is a list of OIDs) is equal
+ * to the previous one. It's more efficient to do so in this function, because
+ * we can avoid copying the previous list unless there is a syscache
+ * invalidation (in which case we must copy it before resetting
+ * SearchPathCacheContext).
+ */
+static SearchPathCacheEntry *
+cachedNamespacePath(const char *searchPath, Oid roleid, bool *prevPathEqual)
+{
+	MemoryContext			 oldcxt;
+	SearchPathCacheKey		 cachekey;
+	SearchPathCacheEntry	*entry;
+	bool					 found;
+	SearchPathCacheEntry	*prevEntry = CurrentSearchPathCacheEntry;
+	List					*prevPath = prevEntry ? prevEntry->finalPath : NIL;
+	List					*prevPathCopy = NIL;
+
+	/* clear the cache and start over when invalidated */
+	if (!searchPathCacheValid)
+	{
+		/* temporarily save prevPath for comparison after reset */
+		prevPathCopy = list_copy(prevPath);
+		prevPath = prevPathCopy;
+
+		MemoryContextReset(SearchPathCacheContext);
+
+		/* arbitrary initial starting size of 16 elements */
+		SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
+		searchPathCacheValid = true;
+		prevEntry = NULL;
+	}
+
+	if (prevEntry && roleid == prevEntry->key->roleid &&
+		strcmp(searchPath, prevEntry->key->searchPath) == 0)
+	{
+		/*
+		 * If search_path has changed and then changed back before
+		 * recomputeNamespacePath is called, then the hash lookup will just
+		 * find the very same entry that we already have. Optimize this case.
+		 */
+		entry = prevEntry;
+		found = true;
+	}
+	else
+	{
+		/* otherwise do a quick lookup */
+		cachekey.searchPath = searchPath;
+		cachekey.roleid = roleid;
+
+		entry = nsphash_insert(SearchPathCache, &cachekey, &found);
+	}
+
+	/* initialize entry, though don't set finalPath (etc.) */
+	if (!found)
+	{
+		SearchPathCacheKey	*newkey;
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+
+		newkey = palloc(sizeof(SearchPathCacheKey));
+		newkey->searchPath = pstrdup(searchPath);
+		newkey->roleid = roleid;
+
+		entry->key = newkey;
+		entry->oidlist = preprocessNamespacePath(searchPath, roleid,
+												 &entry->temp_missing);
+		entry->finalPath = finalNamespacePath(entry->oidlist,
+											  &entry->firstNS,
+											  &entry->hook_filtered);
+		MemoryContextSwitchTo(oldcxt);
+	}
 
 	/*
-	 * We want to detect the case where the effective value of the base search
-	 * path variables didn't change.  As long as we're doing so, we can avoid
-	 * copying the OID list unnecessarily.
+	 * If a hook is set now, or if a previously-set hook filtered out some
+	 * namespaces from finalPath, we must re-finalize.
 	 */
-	if (baseCreationNamespace == firstNS &&
-		baseTempCreationPending == temp_missing &&
-		equal(oidlist, baseSearchPath))
+	if (found && (object_access_hook || entry->hook_filtered))
+	{
+		/*
+		 * Don't free the existing entry->finalPath yet, as prevPath may be an
+		 * alias for it. Treat it as a copy that will be freed later.
+		 */
+		Assert(prevPathCopy == NIL);
+		prevPathCopy = entry->finalPath;
+
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+		entry->finalPath = finalNamespacePath(entry->oidlist,
+											  &entry->firstNS,
+											  &entry->hook_filtered);
+		MemoryContextSwitchTo(oldcxt);
+	}
+
+	*prevPathEqual = equal(prevPath, entry->finalPath);
+	list_free(prevPathCopy);
+
+	CurrentSearchPathCacheEntry = entry;
+
+	return entry;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+	Oid			roleid = GetUserId();
+	bool		prevPathEqual;
+	bool		pathChanged;
+	SearchPathCacheEntry *entry;
+
+	/* Do nothing if path is already valid. */
+	if (baseSearchPathValid && namespaceUser == roleid)
+		return;
+
+	entry = cachedNamespacePath(namespace_search_path,
+								roleid, &prevPathEqual);
+
+	if (baseCreationNamespace == entry->firstNS &&
+		baseTempCreationPending == entry->temp_missing &&
+		prevPathEqual)
 	{
 		pathChanged = false;
 	}
 	else
 	{
 		pathChanged = true;
-
-		/* Must save OID list in permanent storage. */
-		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-		newpath = list_copy(oidlist);
-		MemoryContextSwitchTo(oldcxt);
-
-		/* Now safe to assign to state variables. */
-		list_free(baseSearchPath);
-		baseSearchPath = newpath;
-		baseCreationNamespace = firstNS;
-		baseTempCreationPending = temp_missing;
 	}
 
+	/* Now safe to assign to state variables. */
+	baseSearchPath = entry->finalPath;
+	baseCreationNamespace = entry->firstNS;
+	baseTempCreationPending = entry->temp_missing;
+
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
 	namespaceUser = roleid;
@@ -3791,11 +4002,6 @@ recomputeNamespacePath(void)
 	 */
 	if (pathChanged)
 		activePathGeneration++;
-
-	/* Clean up. */
-	pfree(rawname);
-	list_free(namelist);
-	list_free(oidlist);
 }
 
 /*
@@ -3950,6 +4156,7 @@ InitTempTableNamespace(void)
 	myTempNamespaceSubID = GetCurrentSubTransactionId();
 
 	baseSearchPathValid = false;	/* need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 /*
@@ -3975,6 +4182,7 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4016,6 +4224,7 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4139,6 +4348,10 @@ assign_search_path(const char *newval, void *extra)
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
 	 * initialization, or outside a transaction.
+	 *
+	 * This does not invalidate the search path cache, so if this value had
+	 * been previously set and no syscache invalidations happened,
+	 * recomputation may not be necessary.
 	 */
 	baseSearchPathValid = false;
 }
@@ -4173,6 +4386,10 @@ InitializeSearchPath(void)
 	}
 	else
 	{
+		SearchPathCacheContext = AllocSetContextCreate(
+			TopMemoryContext, "search_path processing cache",
+			ALLOCSET_DEFAULT_SIZES);
+
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
 		 * of pg_namespace or pg_authid rows. (Changing a role name may affect
@@ -4186,6 +4403,7 @@ InitializeSearchPath(void)
 									  (Datum) 0);
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
+		searchPathCacheValid = false;
 	}
 }
 
@@ -4196,8 +4414,13 @@ InitializeSearchPath(void)
 static void
 NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
-	/* Force search path to be recomputed on next use */
+	/*
+	 * Force search path to be recomputed on next use, also invalidating the
+	 * search path cache (because namespace names, ACLs, or role names may
+	 * have changed).
+	 */
 	baseSearchPathValid = false;
+	searchPathCacheValid = false;
 }
 
 /*
-- 
2.34.1

From 955cce952e8fbc8d117d4df1876e56feaec1d944 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v7 3/6] Add cache for recomputeNamespacePath().

When search_path is changed to something that was previously set, and
no invalidation happened in between, use the cached list of namespace
OIDs rather than recomputing them. This avoids syscache lookups and
ACL checks.

Important when the search_path changes frequently, such as when set in
proconfig.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 440 ++++++++++++++++++++++++++------
 1 file changed, 365 insertions(+), 75 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index ff1bfae1a3..258c97b590 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_ts_template.h"
 #include "catalog/pg_type.h"
 #include "commands/dbcommands.h"
+#include "common/hashfn.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
@@ -109,11 +110,12 @@
  * activeSearchPath is always the actually active path; it points to
  * to baseSearchPath which is the list derived from namespace_search_path.
  *
- * If baseSearchPathValid is false, then baseSearchPath (and other
- * derived variables) need to be recomputed from namespace_search_path.
- * We mark it invalid upon an assignment to namespace_search_path or receipt
- * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next lookup attempt.
+ * If baseSearchPathValid is false, then baseSearchPath (and other derived
+ * variables) need to be recomputed from namespace_search_path, or retrieved
+ * from the search path cache if there haven't been any syscache
+ * invalidations.  We mark it invalid upon an assignment to
+ * namespace_search_path or receipt of a syscache invalidation event for
+ * pg_namespace.  The recomputation is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -153,6 +155,27 @@ static Oid	namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
+static bool searchPathCacheValid = false;
+static MemoryContext SearchPathCacheContext = NULL;
+
+typedef struct SearchPathCacheKey
+{
+	const char	*searchPath;
+	Oid			 roleid;
+} SearchPathCacheKey;
+
+typedef struct SearchPathCacheEntry
+{
+	SearchPathCacheKey	 key;
+	List				*oidlist;	/* namespace OIDs that pass ACL checks */
+	List				*finalPath;	/* cached final computed search path */
+	Oid					 firstNS;	/* first explicitly-listed namespace */
+	bool				 temp_missing;
+	bool				 forceRecompute; /* force recompute of finalPath */
+
+	/* needed for simplehash */
+	char				 status;
+} SearchPathCacheEntry;
 
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@@ -206,6 +229,143 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
 
+/*
+ * Recomputing the namespace path can be costly when done frequently, such as
+ * when a function has search_path set in proconfig. Add a search path cache
+ * that can be used by recomputeNamespacePath().
+ *
+ * The search path cache is based on a wrapper around a simplehash hash table
+ * (nsphash, defined below). The spcache wrapper deals with OOM while trying
+ * to initialize a key, and also offers a more convenient API.
+ */
+
+static inline uint32
+spcachekey_hash(SearchPathCacheKey key)
+{
+	const unsigned char	*bytes = (const unsigned char *)key.searchPath;
+	int					 blen  = strlen(key.searchPath);
+
+	return hash_combine(hash_bytes(bytes, blen),
+						hash_uint32(key.roleid));
+}
+
+static inline bool
+spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
+{
+	return a.roleid == b.roleid &&
+		strcmp(a.searchPath, b.searchPath) == 0;
+}
+
+#define SH_PREFIX		nsphash
+#define SH_ELEMENT_TYPE	SearchPathCacheEntry
+#define SH_KEY_TYPE		SearchPathCacheKey
+#define	SH_KEY			key
+#define SH_HASH_KEY(tb, key)   	spcachekey_hash(key)
+#define SH_EQUAL(tb, a, b)		spcachekey_equal(a, b)
+#define	SH_SCOPE		static inline
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+/*
+ * We only expect a small number of unique search_path strings to be used. If
+ * this cache grows to an unreasonable size, reset it to avoid steady-state
+ * memory growth. Most likely, only a few of those entries will benefit from
+ * the cache, and the cache will be quickly repopulated with such entries.
+ */
+#define SPCACHE_RESET_THRESHOLD		1024
+
+static nsphash_hash *SearchPathCache = NULL;
+
+/*
+ * Create search path cache.
+ */
+static void
+spcache_init(void)
+{
+	Assert(SearchPathCacheContext);
+
+	if (SearchPathCache)
+		return;
+
+	/* arbitrary initial starting size of 16 elements */
+	SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
+	searchPathCacheValid = true;
+}
+
+/*
+ * Reset and reinitialize search path cache.
+ */
+static void
+spcache_reset(void)
+{
+	Assert(SearchPathCacheContext);
+	Assert(SearchPathCache);
+
+	MemoryContextReset(SearchPathCacheContext);
+	SearchPathCache = NULL;
+
+	spcache_init();
+}
+
+static uint32
+spcache_members(void)
+{
+	return SearchPathCache->members;
+}
+
+/*
+ * Look up or insert entry in search path cache.
+ *
+ * Initialize key safely, so that OOM does not leave an entry without a valid
+ * key. Caller must ensure that non-key contents are properly initialized.
+ */
+static SearchPathCacheEntry *
+spcache_insert(const char *searchPath, Oid roleid)
+{
+	SearchPathCacheEntry	*entry;
+	bool					 found;
+	SearchPathCacheKey		 cachekey = {
+		.searchPath					  = searchPath,
+		.roleid						  = roleid
+	};
+
+	entry = nsphash_insert(SearchPathCache, cachekey, &found);
+
+	/* ensure that key is initialized and the rest is zeroed */
+	if (!found)
+	{
+		size_t	 size = strlen(searchPath) + 1;
+		char	*newstr;
+
+		/* do not touch entry->status, used by simplehash */
+		entry->oidlist = NIL;
+		entry->finalPath = NIL;
+		entry->firstNS = InvalidOid;
+		entry->temp_missing = false;
+
+		newstr = MemoryContextAllocExtended(SearchPathCacheContext, size,
+											MCXT_ALLOC_NO_OOM);
+
+		/* if we can't initialize the key due to OOM, delete the entry */
+		if (newstr == NULL)
+		{
+			nsphash_delete_item(SearchPathCache, entry);
+			MemoryContextStats(TopMemoryContext);
+			ereport(ERROR,
+					(errcode(ERRCODE_OUT_OF_MEMORY),
+					 errmsg("out of memory"),
+					 errdetail("Failed on request of size %zu in memory context \"%s\".",
+							   size, SearchPathCacheContext->name)));
+		}
+
+		memcpy(newstr, searchPath, size);
+		entry->key.searchPath = newstr;
+		entry->key.roleid = roleid;
+	}
+
+	return entry;
+}
 
 /*
  * RangeVarGetRelidExtended
@@ -3630,6 +3790,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 	 */
 
 	baseSearchPathValid = false;	/* may need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 
@@ -3893,28 +4054,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
 }
 
 /*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Look up namespace IDs and perform ACL checks. Return newly-allocated list.
  */
-static void
-recomputeNamespacePath(void)
+static List *
+preprocessNamespacePath(const char *searchPath, Oid roleid,
+						bool *temp_missing)
 {
-	Oid			roleid = GetUserId();
 	char	   *rawname;
 	List	   *namelist;
 	List	   *oidlist;
-	List	   *newpath;
 	ListCell   *l;
-	bool		temp_missing;
-	Oid			firstNS;
-	bool		pathChanged;
-	MemoryContext oldcxt;
 
-	/* Do nothing if path is already valid. */
-	if (baseSearchPathValid && namespaceUser == roleid)
-		return;
-
-	/* Need a modifiable copy of namespace_search_path string */
-	rawname = pstrdup(namespace_search_path);
+	/* Need a modifiable copy */
+	rawname = pstrdup(searchPath);
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -3931,7 +4083,7 @@ recomputeNamespacePath(void)
 	 * already been accepted.)	Don't make duplicate entries, either.
 	 */
 	oidlist = NIL;
-	temp_missing = false;
+	*temp_missing = false;
 	foreach(l, namelist)
 	{
 		char	   *curname = (char *) lfirst(l);
@@ -3951,10 +4103,8 @@ recomputeNamespacePath(void)
 				namespaceId = get_namespace_oid(rname, true);
 				ReleaseSysCache(tuple);
 				if (OidIsValid(namespaceId) &&
-					!list_member_oid(oidlist, namespaceId) &&
 					object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-									ACL_USAGE) == ACLCHECK_OK &&
-					InvokeNamespaceSearchHook(namespaceId, false))
+									ACL_USAGE) == ACLCHECK_OK)
 					oidlist = lappend_oid(oidlist, namespaceId);
 			}
 		}
@@ -3962,16 +4112,12 @@ recomputeNamespacePath(void)
 		{
 			/* pg_temp --- substitute temp namespace, if any */
 			if (OidIsValid(myTempNamespace))
-			{
-				if (!list_member_oid(oidlist, myTempNamespace) &&
-					InvokeNamespaceSearchHook(myTempNamespace, false))
-					oidlist = lappend_oid(oidlist, myTempNamespace);
-			}
+				oidlist = lappend_oid(oidlist, myTempNamespace);
 			else
 			{
 				/* If it ought to be the creation namespace, set flag */
 				if (oidlist == NIL)
-					temp_missing = true;
+					*temp_missing = true;
 			}
 		}
 		else
@@ -3979,62 +4125,190 @@ recomputeNamespacePath(void)
 			/* normal namespace reference */
 			namespaceId = get_namespace_oid(curname, true);
 			if (OidIsValid(namespaceId) &&
-				!list_member_oid(oidlist, namespaceId) &&
 				object_aclcheck(NamespaceRelationId, namespaceId, roleid,
-								ACL_USAGE) == ACLCHECK_OK &&
-				InvokeNamespaceSearchHook(namespaceId, false))
+								ACL_USAGE) == ACLCHECK_OK)
 				oidlist = lappend_oid(oidlist, namespaceId);
 		}
 	}
 
+	pfree(rawname);
+	list_free(namelist);
+
+	return oidlist;
+}
+
+/*
+ * Remove duplicates, run namespace search hooks, and prepend
+ * implicitly-searched namespaces. Return newly-allocated list.
+ *
+ * If an object_access_hook is present, this must always be recalculated. It
+ * may seem that duplicate elimination is not dependent on the result of the
+ * hook, but if a hook returns different results on different calls for the
+ * same namespace ID, then it could affect the order in which that namespace
+ * appears in the final list.
+ */
+static List *
+finalNamespacePath(List *oidlist, Oid *firstNS)
+{
+	List		*finalPath = NIL;
+	ListCell	*lc;
+
+	foreach(lc, oidlist)
+	{
+		Oid namespaceId = lfirst_oid(lc);
+		if (!list_member_oid(finalPath, namespaceId))
+		{
+			if (InvokeNamespaceSearchHook(namespaceId, false))
+				finalPath = lappend_oid(finalPath, namespaceId);
+		}
+	}
+
 	/*
 	 * Remember the first member of the explicit list.  (Note: this is
 	 * nominally wrong if temp_missing, but we need it anyway to distinguish
 	 * explicit from implicit mention of pg_catalog.)
 	 */
-	if (oidlist == NIL)
-		firstNS = InvalidOid;
+	if (finalPath == NIL)
+		*firstNS = InvalidOid;
 	else
-		firstNS = linitial_oid(oidlist);
+		*firstNS = linitial_oid(finalPath);
 
 	/*
 	 * Add any implicitly-searched namespaces to the list.  Note these go on
 	 * the front, not the back; also notice that we do not check USAGE
 	 * permissions for these.
 	 */
-	if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
-		oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
+	if (!list_member_oid(finalPath, PG_CATALOG_NAMESPACE))
+		finalPath = lcons_oid(PG_CATALOG_NAMESPACE, finalPath);
 
 	if (OidIsValid(myTempNamespace) &&
-		!list_member_oid(oidlist, myTempNamespace))
-		oidlist = lcons_oid(myTempNamespace, oidlist);
+		!list_member_oid(finalPath, myTempNamespace))
+		finalPath = lcons_oid(myTempNamespace, finalPath);
+
+	return finalPath;
+}
+
+/*
+ * Retrieve search path information from the cache; or if not there, fill
+ * it. The returned entry is valid only until the next call to this function.
+ *
+ * We also determine if the newly-computed finalPath is different from the
+ * prevPath passed by the caller (i.e. a no-op or a real change?). It's more
+ * efficient to check for a change in this function than the caller, because
+ * we can avoid unnecessary temporary copies of the previous path.
+ */
+static const SearchPathCacheEntry *
+cachedNamespacePath(const char *searchPath, Oid roleid, List *prevPath,
+					bool *changed)
+{
+	MemoryContext			 oldcxt;
+	SearchPathCacheEntry	*entry;
+	List					*prevPathCopy		= NIL;
+
+	spcache_init();
+
+	/* invalidate cache if necessary */
+	if (!searchPathCacheValid || spcache_members() >= SPCACHE_RESET_THRESHOLD)
+	{
+		/* prevPath will be destroyed; make temp copy for later comparison */
+		prevPathCopy = list_copy(prevPath);
+		prevPath = prevPathCopy;
+		spcache_reset();
+	}
+
+	entry = spcache_insert(searchPath, roleid);
 
 	/*
-	 * We want to detect the case where the effective value of the base search
-	 * path variables didn't change.  As long as we're doing so, we can avoid
-	 * copying the OID list unnecessarily.
+	 * An OOM may have resulted in a cache entry with mising 'oidlist' or
+	 * 'finalPath', so just compute whatever is missing.
 	 */
-	if (baseCreationNamespace == firstNS &&
-		baseTempCreationPending == temp_missing &&
-		equal(oidlist, baseSearchPath))
+
+	if (entry->oidlist == NIL)
 	{
-		pathChanged = false;
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+		entry->oidlist = preprocessNamespacePath(searchPath, roleid,
+												 &entry->temp_missing);
+		MemoryContextSwitchTo(oldcxt);
 	}
-	else
+
+	/*
+	 * If a hook is set, we must recompute finalPath from the oidlist each
+	 * time, because the hook may affect the result. This is still much faster
+	 * than recomputing from the string (and doing catalog lookups and ACL
+	 * checks).
+	 */
+	if (entry->finalPath == NIL || object_access_hook ||
+		entry->forceRecompute)
 	{
-		pathChanged = true;
+		/*
+		 * Do not free the stale value of entry->finalPath until we've
+		 * performed the comparison, in case it's aliased by prevPath (which
+		 * can only happen when recomputing due to an object_access_hook).
+		 */
+		List *finalPath;
 
-		/* Must save OID list in permanent storage. */
-		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-		newpath = list_copy(oidlist);
+		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
+		finalPath = finalNamespacePath(entry->oidlist,
+									   &entry->firstNS);
 		MemoryContextSwitchTo(oldcxt);
 
-		/* Now safe to assign to state variables. */
-		list_free(baseSearchPath);
-		baseSearchPath = newpath;
-		baseCreationNamespace = firstNS;
-		baseTempCreationPending = temp_missing;
+		*changed = !equal(prevPath, finalPath);
+
+		list_free(entry->finalPath);
+		entry->finalPath = finalPath;
+
+		/*
+		 * If an object_access_hook set when finalPath is calculated, the
+		 * result may be affected by the hook. Force recomputation of
+		 * finalPath the next time this cache entry is used, even if the
+		 * object_access_hook is not set at that time.
+		 */
+		entry->forceRecompute = object_access_hook ? true : false;
+	}
+	else
+	{
+		/* use cached version of finalPath */
+		*changed = !equal(prevPath, entry->finalPath);
+	}
+
+	list_free(prevPathCopy);
+
+	return entry;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+	Oid			roleid = GetUserId();
+	bool		newPathEqual;
+	bool		pathChanged;
+	const SearchPathCacheEntry *entry;
+
+	/* Do nothing if path is already valid. */
+	if (baseSearchPathValid && namespaceUser == roleid)
+		return;
+
+	entry = cachedNamespacePath(namespace_search_path, roleid, baseSearchPath,
+								&newPathEqual);
+
+	if (baseCreationNamespace == entry->firstNS &&
+		baseTempCreationPending == entry->temp_missing &&
+		!newPathEqual)
+	{
+		pathChanged = false;
 	}
+	else
+	{
+		pathChanged = true;
+	}
+
+	/* Now safe to assign to state variables. */
+	baseSearchPath = entry->finalPath;
+	baseCreationNamespace = entry->firstNS;
+	baseTempCreationPending = entry->temp_missing;
 
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
@@ -4051,11 +4325,6 @@ recomputeNamespacePath(void)
 	 */
 	if (pathChanged)
 		activePathGeneration++;
-
-	/* Clean up. */
-	pfree(rawname);
-	list_free(namelist);
-	list_free(oidlist);
 }
 
 /*
@@ -4210,6 +4479,7 @@ InitTempTableNamespace(void)
 	myTempNamespaceSubID = GetCurrentSubTransactionId();
 
 	baseSearchPathValid = false;	/* need to rebuild list */
+	searchPathCacheValid = false;
 }
 
 /*
@@ -4235,6 +4505,7 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4276,6 +4547,7 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
 			myTempNamespace = InvalidOid;
 			myTempToastNamespace = InvalidOid;
 			baseSearchPathValid = false;	/* need to rebuild list */
+			searchPathCacheValid = false;
 
 			/*
 			 * Reset the temporary namespace flag in MyProc.  We assume that
@@ -4361,11 +4633,23 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
-	char	   *rawname;
-	List	   *namelist;
+	const char				*searchPath = *newval;
+	char					*rawname;
+	List					*namelist;
+
+	/*
+	 * We used to try to check that the named schemas exist, but there are
+	 * many valid use-cases for having search_path settings that include
+	 * schemas that don't exist; and often, we are not inside a transaction
+	 * here and so can't consult the system catalogs anyway.  So now, the only
+	 * requirement is syntactic validity of the identifier list.
+	 */
 
-	/* Need a modifiable copy of string */
-	rawname = pstrdup(*newval);
+	/*
+	 * Ensure validity check succeeds before creating cache entry.
+	 */
+
+	rawname = pstrdup(searchPath);	/* need a modifiable copy */
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -4377,14 +4661,6 @@ check_search_path(char **newval, void **extra, GucSource source)
 		return false;
 	}
 
-	/*
-	 * We used to try to check that the named schemas exist, but there are
-	 * many valid use-cases for having search_path settings that include
-	 * schemas that don't exist; and often, we are not inside a transaction
-	 * here and so can't consult the system catalogs anyway.  So now, the only
-	 * requirement is syntactic validity of the identifier list.
-	 */
-
 	pfree(rawname);
 	list_free(namelist);
 
@@ -4399,6 +4675,10 @@ assign_search_path(const char *newval, void *extra)
 	 * We mark the path as needing recomputation, but don't do anything until
 	 * it's needed.  This avoids trying to do database access during GUC
 	 * initialization, or outside a transaction.
+	 *
+	 * This does not invalidate the search path cache, so if this value had
+	 * been previously set and no syscache invalidations happened,
+	 * recomputation may not be necessary.
 	 */
 	baseSearchPathValid = false;
 }
@@ -4433,6 +4713,10 @@ InitializeSearchPath(void)
 	}
 	else
 	{
+		SearchPathCacheContext = AllocSetContextCreate(
+			TopMemoryContext, "search_path processing cache",
+			ALLOCSET_DEFAULT_SIZES);
+
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
 		 * of pg_namespace or pg_authid rows. (Changing a role name may affect
@@ -4446,6 +4730,7 @@ InitializeSearchPath(void)
 									  (Datum) 0);
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
+		searchPathCacheValid = false;
 	}
 }
 
@@ -4456,8 +4741,13 @@ InitializeSearchPath(void)
 static void
 NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
-	/* Force search path to be recomputed on next use */
+	/*
+	 * Force search path to be recomputed on next use, also invalidating the
+	 * search path cache (because namespace names, ACLs, or role names may
+	 * have changed).
+	 */
 	baseSearchPathValid = false;
+	searchPathCacheValid = false;
 }
 
 /*
-- 
2.34.1

From 46d51755859746789aa073625fd5570f2d01be0c Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 19 Oct 2023 15:19:05 -0700
Subject: [PATCH v7 4/6] Optimize check_search_path() by using SearchPathCache.

A hash lookup is faster than re-validating the string, particularly
because we use SplitIdentifierString() for validation.

Important when search_path changes frequently.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 50 +++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 258c97b590..68deb86554 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -234,6 +234,10 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
  * when a function has search_path set in proconfig. Add a search path cache
  * that can be used by recomputeNamespacePath().
  *
+ * The cache is also used to remember already-validated strings in
+ * check_search_path() to avoid the need to call SplitIdentifierString()
+ * repeatedly. In this case, the caller uses roleid=InvalidOid.
+ *
  * The search path cache is based on a wrapper around a simplehash hash table
  * (nsphash, defined below). The spcache wrapper deals with OOM while trying
  * to initialize a key, and also offers a more convenient API.
@@ -314,6 +318,21 @@ spcache_members(void)
 	return SearchPathCache->members;
 }
 
+/*
+ * Look up entry in search path cache without inserting. Returns NULL if not
+ * present.
+ */
+static SearchPathCacheEntry *
+spcache_lookup(const char *searchPath, Oid roleid)
+{
+	SearchPathCacheKey		 cachekey = {
+		.searchPath					  = searchPath,
+		.roleid						  = roleid
+	};
+
+	return nsphash_lookup(SearchPathCache, cachekey);
+}
+
 /*
  * Look up or insert entry in search path cache.
  *
@@ -4633,9 +4652,10 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
-	const char				*searchPath = *newval;
-	char					*rawname;
-	List					*namelist;
+	Oid			 roleid		= InvalidOid;
+	const char	*searchPath = *newval;
+	char		*rawname;
+	List		*namelist;
 
 	/*
 	 * We used to try to check that the named schemas exist, but there are
@@ -4645,6 +4665,24 @@ check_search_path(char **newval, void **extra, GucSource source)
 	 * requirement is syntactic validity of the identifier list.
 	 */
 
+	/*
+	 * Checking only the syntactic validity also allows us to use the search
+	 * path cache (if available) to avoid calling SplitIdentifierString() on
+	 * the same string repeatedly.
+	 */
+	if (SearchPathCacheContext != NULL)
+	{
+		spcache_init();
+
+		roleid = GetUserId();
+
+		if (spcache_lookup(searchPath, roleid) != NULL)
+			return true;
+
+		if (spcache_members() >= SPCACHE_RESET_THRESHOLD)
+			spcache_reset();
+	}
+
 	/*
 	 * Ensure validity check succeeds before creating cache entry.
 	 */
@@ -4664,6 +4702,12 @@ check_search_path(char **newval, void **extra, GucSource source)
 	pfree(rawname);
 	list_free(namelist);
 
+	/* create empty cache entry */
+	if (SearchPathCacheContext != NULL)
+	{
+		(void) spcache_insert(searchPath, roleid);
+	}
+
 	return true;
 }
 
-- 
2.34.1

From 8b5e905cbffa9eadd1e0d4fa6af7e7b77a93b7c7 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 19 Oct 2023 15:48:16 -0700
Subject: [PATCH v7 5/6] Optimize SearchPathCache by saving the last entry.

Repeated lookups are common, so it's worth it to check the last entry
before doing another hash lookup.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 107 ++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 40 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 68deb86554..ff7169dc99 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -240,7 +240,8 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
  *
  * The search path cache is based on a wrapper around a simplehash hash table
  * (nsphash, defined below). The spcache wrapper deals with OOM while trying
- * to initialize a key, and also offers a more convenient API.
+ * to initialize a key, optimizes repeated lookups of the same key, and also
+ * offers a more convenient API.
  */
 
 static inline uint32
@@ -280,6 +281,7 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
 #define SPCACHE_RESET_THRESHOLD		1024
 
 static nsphash_hash *SearchPathCache = NULL;
+static SearchPathCacheEntry *LastSearchPathCacheEntry = NULL;
 
 /*
  * Create search path cache.
@@ -308,6 +310,7 @@ spcache_reset(void)
 
 	MemoryContextReset(SearchPathCacheContext);
 	SearchPathCache = NULL;
+	LastSearchPathCacheEntry = NULL;
 
 	spcache_init();
 }
@@ -325,12 +328,25 @@ spcache_members(void)
 static SearchPathCacheEntry *
 spcache_lookup(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheKey		 cachekey = {
-		.searchPath					  = searchPath,
-		.roleid						  = roleid
-	};
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
+	{
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry	*entry;
+		SearchPathCacheKey		 cachekey = {
+			.searchPath					  = searchPath,
+			.roleid						  = roleid
+		};
+
+		entry = nsphash_lookup(SearchPathCache, cachekey);
 
-	return nsphash_lookup(SearchPathCache, cachekey);
+		LastSearchPathCacheEntry = entry;
+		return entry;
+	}
 }
 
 /*
@@ -342,48 +358,59 @@ spcache_lookup(const char *searchPath, Oid roleid)
 static SearchPathCacheEntry *
 spcache_insert(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheEntry	*entry;
-	bool					 found;
-	SearchPathCacheKey		 cachekey = {
-		.searchPath					  = searchPath,
-		.roleid						  = roleid
-	};
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
+	{
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry	*entry;
+		bool					 found;
+		SearchPathCacheKey		 cachekey = {
+			.searchPath					  = searchPath,
+			.roleid						  = roleid
+		};
 
-	entry = nsphash_insert(SearchPathCache, cachekey, &found);
+		entry = nsphash_insert(SearchPathCache, cachekey, &found);
 
-	/* ensure that key is initialized and the rest is zeroed */
-	if (!found)
-	{
-		size_t	 size = strlen(searchPath) + 1;
-		char	*newstr;
+		/* ensure that key is initialized and the rest is zeroed */
+		if (!found)
+		{
+			size_t	 size = strlen(searchPath) + 1;
+			char	*newstr;
 
-		/* do not touch entry->status, used by simplehash */
-		entry->oidlist = NIL;
-		entry->finalPath = NIL;
-		entry->firstNS = InvalidOid;
-		entry->temp_missing = false;
+			/* do not touch entry->status, used by simplehash */
+			entry->oidlist = NIL;
+			entry->finalPath = NIL;
+			entry->firstNS = InvalidOid;
+			entry->temp_missing = false;
 
-		newstr = MemoryContextAllocExtended(SearchPathCacheContext, size,
-											MCXT_ALLOC_NO_OOM);
+			newstr = MemoryContextAllocExtended(SearchPathCacheContext, size,
+												MCXT_ALLOC_NO_OOM);
 
-		/* if we can't initialize the key due to OOM, delete the entry */
-		if (newstr == NULL)
-		{
-			nsphash_delete_item(SearchPathCache, entry);
-			MemoryContextStats(TopMemoryContext);
-			ereport(ERROR,
-					(errcode(ERRCODE_OUT_OF_MEMORY),
-					 errmsg("out of memory"),
-					 errdetail("Failed on request of size %zu in memory context \"%s\".",
-							   size, SearchPathCacheContext->name)));
+			/* if we can't initialize the key due to OOM, delete the entry */
+			if (newstr == NULL)
+			{
+				nsphash_delete_item(SearchPathCache, entry);
+				MemoryContextStats(TopMemoryContext);
+				ereport(ERROR,
+						(errcode(ERRCODE_OUT_OF_MEMORY),
+						 errmsg("out of memory"),
+						 errdetail("Failed on request of size %zu in memory context \"%s\".",
+								   size, SearchPathCacheContext->name)));
+
+			}
+
+			memcpy(newstr, searchPath, size);
+			entry->key.searchPath = newstr;
+			entry->key.roleid = roleid;
 		}
 
-		memcpy(newstr, searchPath, size);
-		entry->key.searchPath = newstr;
-		entry->key.roleid = roleid;
+		LastSearchPathCacheEntry = entry;
+		return entry;
 	}
-
-	return entry;
 }
 
 /*
-- 
2.34.1

From 6ed0785c29a43ccbd36aa3f9bd3be751131a6bf2 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 19 Oct 2023 15:48:16 -0700
Subject: [PATCH v9 5/5] Optimize SearchPathCache by saving the last entry.

Repeated lookups are common, so it's worth it to check the last entry
before doing another hash lookup.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 88 +++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 0785f2b797..f540da7ebc 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -241,7 +241,8 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
  *
  * The search path cache is based on a wrapper around a simplehash hash table
  * (nsphash, defined below). The spcache wrapper deals with OOM while trying
- * to initialize a key, and also offers a more convenient API.
+ * to initialize a key, optimizes repeated lookups of the same key, and also
+ * offers a more convenient API.
  */
 
 static inline uint32
@@ -281,6 +282,7 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
 #define SPCACHE_RESET_THRESHOLD		256
 
 static nsphash_hash * SearchPathCache = NULL;
+static SearchPathCacheEntry * LastSearchPathCacheEntry = NULL;
 
 /*
  * Create or reset search_path cache as necessary.
@@ -295,6 +297,7 @@ spcache_init(void)
 			return;
 
 	MemoryContextReset(SearchPathCacheContext);
+	LastSearchPathCacheEntry = NULL;
 	/* arbitrary initial starting size of 16 elements */
 	SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
 	searchPathCacheValid = true;
@@ -307,12 +310,25 @@ spcache_init(void)
 static SearchPathCacheEntry *
 spcache_lookup(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheKey cachekey = {
-		.searchPath = searchPath,
-		.roleid = roleid
-	};
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
+	{
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry *entry;
+		SearchPathCacheKey cachekey = {
+			.searchPath = searchPath,
+			.roleid = roleid
+		};
+
+		entry = nsphash_lookup(SearchPathCache, cachekey);
 
-	return nsphash_lookup(SearchPathCache, cachekey);
+		LastSearchPathCacheEntry = entry;
+		return entry;
+	}
 }
 
 /*
@@ -324,35 +340,45 @@ spcache_lookup(const char *searchPath, Oid roleid)
 static SearchPathCacheEntry *
 spcache_insert(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheEntry *entry;
-	SearchPathCacheKey cachekey = {
-		.searchPath = searchPath,
-		.roleid = roleid
-	};
-
-	/*
-	 * searchPath is not saved in SearchPathCacheContext. First perform a
-	 * lookup, and copy searchPath only if we need to create a new entry.
-	 */
-	entry = nsphash_lookup(SearchPathCache, cachekey);
-
-	if (!entry)
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
 	{
-		bool		found;
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry *entry;
+		SearchPathCacheKey cachekey = {
+			.searchPath = searchPath,
+			.roleid = roleid
+		};
 
-		cachekey.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
-		entry = nsphash_insert(SearchPathCache, cachekey, &found);
-		Assert(!found);
+		/*
+		 * searchPath is not saved in SearchPathCacheContext. First perform a
+		 * lookup, and copy searchPath only if we need to create a new entry.
+		 */
+		entry = nsphash_lookup(SearchPathCache, cachekey);
 
-		entry->oidlist = NIL;
-		entry->finalPath = NIL;
-		entry->firstNS = InvalidOid;
-		entry->temp_missing = false;
-		entry->forceRecompute = false;
-		/* do not touch entry->status, used by simplehash */
-	}
+		if (!entry)
+		{
+			bool		found;
+
+			cachekey.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
+			entry = nsphash_insert(SearchPathCache, cachekey, &found);
+			Assert(!found);
+
+			entry->oidlist = NIL;
+			entry->finalPath = NIL;
+			entry->firstNS = InvalidOid;
+			entry->temp_missing = false;
+			entry->forceRecompute = false;
+			/* do not touch entry->status, used by simplehash */
+		}
 
-	return entry;
+		LastSearchPathCacheEntry = entry;
+		return entry;
+	}
 }
 
 /*
-- 
2.34.1

From e7eb616cea01f575417f56615edb9bd24bfc2453 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 19 Oct 2023 15:19:05 -0700
Subject: [PATCH v9 4/5] Optimize check_search_path() by using SearchPathCache.

A hash lookup is faster than re-validating the string, particularly
because we use SplitIdentifierString() for validation.

Important when search_path changes frequently.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 55 +++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 1a75b50807..0785f2b797 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -235,6 +235,10 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
  * when a function has search_path set in proconfig. Add a search path cache
  * that can be used by recomputeNamespacePath().
  *
+ * The cache is also used to remember already-validated strings in
+ * check_search_path() to avoid the need to call SplitIdentifierString()
+ * repeatedly.
+ *
  * The search path cache is based on a wrapper around a simplehash hash table
  * (nsphash, defined below). The spcache wrapper deals with OOM while trying
  * to initialize a key, and also offers a more convenient API.
@@ -296,6 +300,21 @@ spcache_init(void)
 	searchPathCacheValid = true;
 }
 
+/*
+ * Look up entry in search path cache without inserting. Returns NULL if not
+ * present.
+ */
+static SearchPathCacheEntry *
+spcache_lookup(const char *searchPath, Oid roleid)
+{
+	SearchPathCacheKey cachekey = {
+		.searchPath = searchPath,
+		.roleid = roleid
+	};
+
+	return nsphash_lookup(SearchPathCache, cachekey);
+}
+
 /*
  * Look up or insert entry in search path cache.
  *
@@ -4577,11 +4596,39 @@ ResetTempTableNamespace(void)
 bool
 check_search_path(char **newval, void **extra, GucSource source)
 {
+	Oid			roleid = InvalidOid;
+	const char *searchPath = *newval;
 	char	   *rawname;
 	List	   *namelist;
 
-	/* Need a modifiable copy of string */
-	rawname = pstrdup(*newval);
+	/*
+	 * We used to try to check that the named schemas exist, but there are
+	 * many valid use-cases for having search_path settings that include
+	 * schemas that don't exist; and often, we are not inside a transaction
+	 * here and so can't consult the system catalogs anyway.  So now, the only
+	 * requirement is syntactic validity of the identifier list.
+	 */
+
+	/*
+	 * Checking only the syntactic validity also allows us to use the search
+	 * path cache (if available) to avoid calling SplitIdentifierString() on
+	 * the same string repeatedly.
+	 */
+	if (SearchPathCacheContext != NULL)
+	{
+		spcache_init();
+
+		roleid = GetUserId();
+
+		if (spcache_lookup(searchPath, roleid) != NULL)
+			return true;
+	}
+
+	/*
+	 * Ensure validity check succeeds before creating cache entry.
+	 */
+
+	rawname = pstrdup(searchPath);	/* need a modifiable copy */
 
 	/* Parse string into list of identifiers */
 	if (!SplitIdentifierString(rawname, ',', &namelist))
@@ -4604,6 +4651,10 @@ check_search_path(char **newval, void **extra, GucSource source)
 	pfree(rawname);
 	list_free(namelist);
 
+	/* create empty cache entry */
+	if (SearchPathCacheContext != NULL)
+		(void) spcache_insert(searchPath, roleid);
+
 	return true;
 }
 
-- 
2.34.1

From 8d870755b638a193a402292f265cb7e2f78ae432 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 16 Nov 2023 12:12:49 -0800
Subject: [PATCH v9 3/5] Be more paranoid about OOM when changing search_path.

Commit f26c2368dc introduced a search_path cache, and left some room
for potential problems with out-of-memory conditions.

This change re-introduces one list_copy(), but simplifies things
substantially in addition to better handling OOM.
---
 src/backend/catalog/namespace.c | 118 ++++++++++----------------------
 1 file changed, 35 insertions(+), 83 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 69dbe4500f..1a75b50807 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -279,42 +279,23 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
 static nsphash_hash * SearchPathCache = NULL;
 
 /*
- * Create search path cache.
+ * Create or reset search_path cache as necessary.
  */
 static void
 spcache_init(void)
 {
 	Assert(SearchPathCacheContext);
 
-	if (SearchPathCache)
-		return;
+	if (SearchPathCache && searchPathCacheValid &&
+		SearchPathCache->members < SPCACHE_RESET_THRESHOLD)
+			return;
 
+	MemoryContextReset(SearchPathCacheContext);
 	/* arbitrary initial starting size of 16 elements */
 	SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
 	searchPathCacheValid = true;
 }
 
-/*
- * Reset and reinitialize search path cache.
- */
-static void
-spcache_reset(void)
-{
-	Assert(SearchPathCacheContext);
-	Assert(SearchPathCache);
-
-	MemoryContextReset(SearchPathCacheContext);
-	SearchPathCache = NULL;
-
-	spcache_init();
-}
-
-static uint32
-spcache_members(void)
-{
-	return SearchPathCache->members;
-}
-
 /*
  * Look up or insert entry in search path cache.
  *
@@ -325,27 +306,25 @@ static SearchPathCacheEntry *
 spcache_insert(const char *searchPath, Oid roleid)
 {
 	SearchPathCacheEntry *entry;
-	bool		found;
 	SearchPathCacheKey cachekey = {
 		.searchPath = searchPath,
 		.roleid = roleid
 	};
 
 	/*
-	 * If a new entry is created, we must ensure that it's properly
-	 * initialized. Set the cache invalid temporarily, so that if the
-	 * MemoryContextStrdup() below raises an OOM, the cache will be reset on
-	 * the next use, clearing the uninitialized entry.
+	 * searchPath is not saved in SearchPathCacheContext. First perform a
+	 * lookup, and copy searchPath only if we need to create a new entry.
 	 */
-	searchPathCacheValid = false;
+	entry = nsphash_lookup(SearchPathCache, cachekey);
 
-	entry = nsphash_insert(SearchPathCache, cachekey, &found);
-
-	/* ensure that key is initialized and the rest is zeroed */
-	if (!found)
+	if (!entry)
 	{
-		entry->key.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
-		entry->key.roleid = roleid;
+		bool		found;
+
+		cachekey.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
+		entry = nsphash_insert(SearchPathCache, cachekey, &found);
+		Assert(!found);
+
 		entry->oidlist = NIL;
 		entry->finalPath = NIL;
 		entry->firstNS = InvalidOid;
@@ -354,7 +333,6 @@ spcache_insert(const char *searchPath, Oid roleid)
 		/* do not touch entry->status, used by simplehash */
 	}
 
-	searchPathCacheValid = true;
 	return entry;
 }
 
@@ -4183,31 +4161,15 @@ finalNamespacePath(List *oidlist, Oid *firstNS)
 /*
  * Retrieve search path information from the cache; or if not there, fill
  * it. The returned entry is valid only until the next call to this function.
- *
- * We also determine if the newly-computed finalPath is the same as the
- * prevPath passed by the caller (i.e. a no-op or a real change?). It's more
- * efficient to check for a change in this function than the caller, because
- * we can avoid unnecessary temporary copies of the previous path.
  */
 static const SearchPathCacheEntry *
-cachedNamespacePath(const char *searchPath, Oid roleid, List *prevPath,
-					bool *same)
+cachedNamespacePath(const char *searchPath, Oid roleid)
 {
 	MemoryContext oldcxt;
 	SearchPathCacheEntry *entry;
-	List	   *prevPathCopy = NIL;
 
 	spcache_init();
 
-	/* invalidate cache if necessary */
-	if (!searchPathCacheValid || spcache_members() >= SPCACHE_RESET_THRESHOLD)
-	{
-		/* prevPath will be destroyed; make temp copy for later comparison */
-		prevPathCopy = list_copy(prevPath);
-		prevPath = prevPathCopy;
-		spcache_reset();
-	}
-
 	entry = spcache_insert(searchPath, roleid);
 
 	/*
@@ -4232,23 +4194,14 @@ cachedNamespacePath(const char *searchPath, Oid roleid, List *prevPath,
 	if (entry->finalPath == NIL || object_access_hook ||
 		entry->forceRecompute)
 	{
-		/*
-		 * Do not free the stale value of entry->finalPath until we've
-		 * performed the comparison, in case it's aliased by prevPath (which
-		 * can only happen when recomputing due to an object_access_hook).
-		 */
-		List	   *finalPath;
+		list_free(entry->finalPath);
+		entry->finalPath = NIL;
 
 		oldcxt = MemoryContextSwitchTo(SearchPathCacheContext);
-		finalPath = finalNamespacePath(entry->oidlist,
-									   &entry->firstNS);
+		entry->finalPath = finalNamespacePath(entry->oidlist,
+											  &entry->firstNS);
 		MemoryContextSwitchTo(oldcxt);
 
-		*same = equal(prevPath, finalPath);
-
-		list_free(entry->finalPath);
-		entry->finalPath = finalPath;
-
 		/*
 		 * If an object_access_hook set when finalPath is calculated, the
 		 * result may be affected by the hook. Force recomputation of
@@ -4257,13 +4210,6 @@ cachedNamespacePath(const char *searchPath, Oid roleid, List *prevPath,
 		 */
 		entry->forceRecompute = object_access_hook ? true : false;
 	}
-	else
-	{
-		/* use cached version of finalPath */
-		*same = equal(prevPath, entry->finalPath);
-	}
-
-	list_free(prevPathCopy);
 
 	return entry;
 }
@@ -4275,7 +4221,6 @@ static void
 recomputeNamespacePath(void)
 {
 	Oid			roleid = GetUserId();
-	bool		newPathEqual;
 	bool		pathChanged;
 	const		SearchPathCacheEntry *entry;
 
@@ -4283,24 +4228,31 @@ recomputeNamespacePath(void)
 	if (baseSearchPathValid && namespaceUser == roleid)
 		return;
 
-	entry = cachedNamespacePath(namespace_search_path, roleid, baseSearchPath,
-								&newPathEqual);
+	entry = cachedNamespacePath(namespace_search_path, roleid);
 
 	if (baseCreationNamespace == entry->firstNS &&
 		baseTempCreationPending == entry->temp_missing &&
-		newPathEqual)
+		equal(entry->finalPath, baseSearchPath))
 	{
 		pathChanged = false;
 	}
 	else
 	{
+		MemoryContext oldcxt;
+		List *newpath;
+
 		pathChanged = true;
-	}
 
-	/* Now safe to assign to state variables. */
-	baseSearchPath = entry->finalPath;
-	baseCreationNamespace = entry->firstNS;
-	baseTempCreationPending = entry->temp_missing;
+		oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+		newpath = list_copy(entry->finalPath);
+		MemoryContextSwitchTo(oldcxt);
+
+		/* Now safe to assign to state variables. */
+		list_free(baseSearchPath);
+		baseSearchPath = newpath;
+		baseCreationNamespace = entry->firstNS;
+		baseTempCreationPending = entry->temp_missing;
+	}
 
 	/* Mark the path valid. */
 	baseSearchPathValid = true;
-- 
2.34.1

From 5f41c0ecc602dd183b7f6e2f23cd28c9338b3c5b Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Sun, 19 Nov 2023 14:47:04 -0800
Subject: [PATCH v10 2/2] Use SH_STORE_HASH for search_path cache.

Does not change performance in expected cases, but makes performance
more resilient in case of bucket collisions.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
Discussion: https://postgr.es/m/27a7a289d5b8f42e1b1e79b1bcaeef3a40583bd2.camel@j-davis.com
---
 src/backend/catalog/namespace.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 37a69e9023..0b6e0d711c 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -171,11 +171,10 @@ typedef struct SearchPathCacheEntry
 	List	   *oidlist;		/* namespace OIDs that pass ACL checks */
 	List	   *finalPath;		/* cached final computed search path */
 	Oid			firstNS;		/* first explicitly-listed namespace */
+	uint32		hash;			/* needed for simplehash */
 	bool		temp_missing;
 	bool		forceRecompute; /* force recompute of finalPath */
-
-	/* needed for simplehash */
-	char		status;
+	char		status;			/* needed for simplehash */
 }			SearchPathCacheEntry;
 
 /*
@@ -270,6 +269,8 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
 #define SH_EQUAL(tb, a, b)		spcachekey_equal(a, b)
 #define SH_SCOPE		static inline
 #define SH_DECLARE
+#define SH_GET_HASH(tb, a)		a->hash
+#define SH_STORE_HASH
 #define SH_DEFINE
 #include "lib/simplehash.h"
 
-- 
2.34.1

From 46e09b225217bef79a57cc4f8450ed19be8f21ba Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 19 Oct 2023 15:48:16 -0700
Subject: [PATCH v10 1/2] Optimize SearchPathCache by saving the last entry.

Repeated lookups are common, so it's worth it to check the last entry
before doing another hash lookup.

Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
---
 src/backend/catalog/namespace.c | 88 +++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 5027efc91d..37a69e9023 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -241,7 +241,8 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
  *
  * The search path cache is based on a wrapper around a simplehash hash table
  * (nsphash, defined below). The spcache wrapper deals with OOM while trying
- * to initialize a key, and also offers a more convenient API.
+ * to initialize a key, optimizes repeated lookups of the same key, and also
+ * offers a more convenient API.
  */
 
 static inline uint32
@@ -281,6 +282,7 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
 #define SPCACHE_RESET_THRESHOLD		256
 
 static nsphash_hash * SearchPathCache = NULL;
+static SearchPathCacheEntry * LastSearchPathCacheEntry = NULL;
 
 /*
  * Create or reset search_path cache as necessary.
@@ -295,6 +297,7 @@ spcache_init(void)
 		return;
 
 	MemoryContextReset(SearchPathCacheContext);
+	LastSearchPathCacheEntry = NULL;
 	/* arbitrary initial starting size of 16 elements */
 	SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
 	searchPathCacheValid = true;
@@ -307,12 +310,25 @@ spcache_init(void)
 static SearchPathCacheEntry *
 spcache_lookup(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheKey cachekey = {
-		.searchPath = searchPath,
-		.roleid = roleid
-	};
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
+	{
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry *entry;
+		SearchPathCacheKey cachekey = {
+			.searchPath = searchPath,
+			.roleid = roleid
+		};
+
+		entry = nsphash_lookup(SearchPathCache, cachekey);
 
-	return nsphash_lookup(SearchPathCache, cachekey);
+		LastSearchPathCacheEntry = entry;
+		return entry;
+	}
 }
 
 /*
@@ -324,35 +340,45 @@ spcache_lookup(const char *searchPath, Oid roleid)
 static SearchPathCacheEntry *
 spcache_insert(const char *searchPath, Oid roleid)
 {
-	SearchPathCacheEntry *entry;
-	SearchPathCacheKey cachekey = {
-		.searchPath = searchPath,
-		.roleid = roleid
-	};
-
-	/*
-	 * searchPath is not saved in SearchPathCacheContext. First perform a
-	 * lookup, and copy searchPath only if we need to create a new entry.
-	 */
-	entry = nsphash_lookup(SearchPathCache, cachekey);
-
-	if (!entry)
+	if (LastSearchPathCacheEntry &&
+		LastSearchPathCacheEntry->key.roleid == roleid &&
+		strcmp(LastSearchPathCacheEntry->key.searchPath, searchPath) == 0)
 	{
-		bool		found;
+		return LastSearchPathCacheEntry;
+	}
+	else
+	{
+		SearchPathCacheEntry *entry;
+		SearchPathCacheKey cachekey = {
+			.searchPath = searchPath,
+			.roleid = roleid
+		};
 
-		cachekey.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
-		entry = nsphash_insert(SearchPathCache, cachekey, &found);
-		Assert(!found);
+		/*
+		 * searchPath is not saved in SearchPathCacheContext. First perform a
+		 * lookup, and copy searchPath only if we need to create a new entry.
+		 */
+		entry = nsphash_lookup(SearchPathCache, cachekey);
 
-		entry->oidlist = NIL;
-		entry->finalPath = NIL;
-		entry->firstNS = InvalidOid;
-		entry->temp_missing = false;
-		entry->forceRecompute = false;
-		/* do not touch entry->status, used by simplehash */
-	}
+		if (!entry)
+		{
+			bool		found;
+
+			cachekey.searchPath = MemoryContextStrdup(SearchPathCacheContext, searchPath);
+			entry = nsphash_insert(SearchPathCache, cachekey, &found);
+			Assert(!found);
+
+			entry->oidlist = NIL;
+			entry->finalPath = NIL;
+			entry->firstNS = InvalidOid;
+			entry->temp_missing = false;
+			entry->forceRecompute = false;
+			/* do not touch entry->status, used by simplehash */
+		}
 
-	return entry;
+		LastSearchPathCacheEntry = entry;
+		return entry;
+	}
 }
 
 /*
-- 
2.34.1

From 1b24d08302e268370ebb14e732f4d5f1f1407726 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Mon, 8 Jul 2024 15:52:50 -0700
Subject: [PATCH v1] Fix missing invalidations for search_path cache.

Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240630223047.1f.nmisch@google.com
Backpatch-through: 17
---
 src/backend/catalog/namespace.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index a2510cf80c6..43b707699d7 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -230,7 +230,7 @@ static void AccessTempTableNamespace(bool force);
 static void InitTempTableNamespace(void);
 static void RemoveTempRelations(Oid tempNamespaceId);
 static void RemoveTempRelationsCallback(int code, Datum arg);
-static void NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue);
+static void InvalidationCallback(Datum arg, int cacheid, uint32 hashvalue);
 static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 						   bool include_out_arguments, int pronargs,
 						   int **argnumbers);
@@ -4749,15 +4749,29 @@ InitializeSearchPath(void)
 
 		/*
 		 * In normal mode, arrange for a callback on any syscache invalidation
-		 * of pg_namespace or pg_authid rows. (Changing a role name may affect
-		 * the meaning of the special string $user.)
+		 * that will affect the search_path cache.
 		 */
+
+		/* namespace name or ACLs may have changed */
 		CacheRegisterSyscacheCallback(NAMESPACEOID,
-									  NamespaceCallback,
+									  InvalidationCallback,
 									  (Datum) 0);
+
+		/* role name may affect the meaning of "$user" */
 		CacheRegisterSyscacheCallback(AUTHOID,
-									  NamespaceCallback,
+									  InvalidationCallback,
+									  (Datum) 0);
+
+		/* role membership may affect ACLs */
+		CacheRegisterSyscacheCallback(AUTHMEMROLEMEM,
+									  InvalidationCallback,
 									  (Datum) 0);
+
+		/* database owner may affect ACLs */
+		CacheRegisterSyscacheCallback(DATABASEOID,
+									  InvalidationCallback,
+									  (Datum) 0);
+
 		/* Force search path to be recomputed on next use */
 		baseSearchPathValid = false;
 		searchPathCacheValid = false;
@@ -4765,11 +4779,11 @@ InitializeSearchPath(void)
 }
 
 /*
- * NamespaceCallback
+ * InvalidationCallback
  *		Syscache inval callback function
  */
 static void
-NamespaceCallback(Datum arg, int cacheid, uint32 hashvalue)
+InvalidationCallback(Datum arg, int cacheid, uint32 hashvalue)
 {
 	/*
 	 * Force search path to be recomputed on next use, also invalidating the
-- 
2.34.1