Improve performance of "SET search_path".
Motivation:
Creating functions with a "SET search_path" clause is safer and more
secure because the function behavior doesn't change based on the
caller's search_path setting.
Setting search_path in the function declaration is especially important
for SECURITY DEFINER functions[1], but even SECURITY INVOKER functions
can be executed more like SECURITY DEFINER in some contexts (e.g.
REINDEX executing an index function). Also, it's just error-prone to
depend on the caller's search_path unless there's a specific reason you
want to do that.
Unfortunately, adding a "SET search_path" clause to functions slows
them down. The attached patches close the performance gap
substantially.
Changes:
0001: Transform the settings in proconfig into a List for faster
processing. This is simple and speeds up any proconfig setting.
0002: Introduce CheckIdentifierString(), which is a faster version of
SplitIdentifierString() that only validates, and can be used in
check_search_path().
0003: Cache of previous search_path settings. The key is the raw
namespace_search_path string and the role OID, and it caches the
computed OID list. Changes to the search_path setting or the role can
retrieve the cached OID list as long as nothing else invalidates the
cache (changes to the temp schema or a syscache invalidation of
pg_namespace or pg_role).
One behavior change in 0003 is that retrieving a cached OID list
doesn't call InvokeNamespaceSearchHook(). It would be easy enough to
disable caching when a hook exists, but I didn't see a reason to expect
that "SET search_path" must invoke that hook each time. Invoking it
when computing for the first time, or after a real invalidation, seemed
fine to me. Feedback on that is welcome.
Test:
CREATE SCHEMA a;
CREATE SCHEMA b;
CREATE TABLE big(i) AS SELECT generate_series(1,20000000);
VACUUM big; CHECKPOINT;
CREATE FUNCTION inc(int) RETURNS INT
LANGUAGE plpgsql
AS $$ begin return $1+1; end; $$;
CREATE FUNCTION inc_ab(int) RETURNS INT
LANGUAGE plpgsql SET search_path = a, b
AS $$ begin return $1+1; end; $$;
-- baseline
EXPLAIN ANALYZE SELECT inc(i) FROM big;
-- test query
EXPLAIN ANALYZE SELECT inc_ab(i) FROM big;
Results:
baseline: 4.3s
test query:
without patch: 14.7s
0001: 13.6s
0001,0002: 10.4s
0001,0002,0003: 8.6s
Timings were inconsistent for me so I took the middle of three runs.
It's a lot faster than without the patch. It's still 2X worse than not
specifying any search_path (baseline), but I think it brings it into
"usable" territory for more use cases.
Regards,
Jeff Davis
[1]
https://www.postgresql.org/docs/current/sql-createfunction.html#SQL-CREATEFUNCTION-SECURITY
From c05ee846d6d9099642eeeed58933924a00d80262 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Thu, 27 Jul 2023 15:11:42 -0700
Subject: [PATCH v1 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 | 58 +++++++++++++++++++++++++++++++++++
src/include/utils/guc.h | 1 +
3 files changed, 77 insertions(+), 9 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..bb69ebc4d8 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6213,6 +6213,64 @@ ParseLongOption(const char *string, char **name, char **value)
*cp = '_';
}
+/*
+ * 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).
+ */
+List *
+TransformGUCArray(ArrayType *array)
+{
+ List *result = NIL;
+ int i;
+
+ Assert(array != NULL);
+ Assert(ARR_ELEMTYPE(array) == TEXTOID);
+ Assert(ARR_NDIM(array) == 1);
+ Assert(ARR_LBOUND(array)[0] == 1);
+
+ for (i = 1; i <= ARR_DIMS(array)[0]; i++)
+ {
+ Datum d;
+ bool isnull;
+ char *s;
+ char *name;
+ char *value;
+ List *pair;
+
+ d = array_ref(array, 1, &i,
+ -1 /* varlenarray */ ,
+ -1 /* TEXT's typlen */ ,
+ false /* TEXT's typbyval */ ,
+ TYPALIGN_INT /* TEXT's typalign */ ,
+ &isnull);
+
+ if (isnull)
+ continue;
+
+ s = TextDatumGetCString(d);
+
+ ParseLongOption(s, &name, &value);
+ if (!value)
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("could not parse setting for parameter \"%s\"",
+ name)));
+ pfree(name);
+ 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,
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 8f7ff4cbf0697144dd6f73d964abc2e589fc5020 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Thu, 27 Jul 2023 15:23:51 -0700
Subject: [PATCH v1 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 69ab1b8e4b..cb87f3385e 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -4322,19 +4322,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;
}
@@ -4346,9 +4340,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 8152b3f279a6ca2f43c38633d4ff80afec0c6a8a Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 26 Jul 2023 12:55:09 -0700
Subject: [PATCH v1 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 | 218 +++++++++++++++---
.../expected/test_oat_hooks.out | 6 -
2 files changed, 183 insertions(+), 41 deletions(-)
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index cb87f3385e..147b4308f9 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"
@@ -115,12 +116,14 @@
* topmost stack entry, or 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 non-overridden lookup attempt. Note that an
- * override spec is never subject to recomputation.
+ * 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 non-overridden
+ * lookup attempt. Note that an override spec is never subject to
+ * recomputation.
*
* Any namespaces mentioned in namespace_search_path that are not readable
* by the current user ID are simply left out of baseSearchPath; so
@@ -160,6 +163,23 @@ 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;
+ Oid firstNS;
+ bool temp_missing;
+ char status;
+} SearchPathCacheEntry;
/* Override requests are remembered in a stack of OverrideStackEntry structs */
@@ -211,6 +231,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
@@ -3388,6 +3446,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
*/
baseSearchPathValid = false; /* may need to rebuild list */
+ searchPathCacheValid = false;
}
@@ -3778,32 +3837,19 @@ FindDefaultConversionProc(int32 for_encoding, int32 to_encoding)
}
/*
- * recomputeNamespacePath - recompute path derived variables if needed.
+ * Full recomputation of search_path.
*/
-static void
-recomputeNamespacePath(void)
+static List *
+processNamespacePath(const char *searchPath, Oid roleid, Oid *firstNS,
+ 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 an override search spec is active. */
- if (overrideStack)
- return;
- /* 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))
@@ -3820,7 +3866,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);
@@ -3860,7 +3906,7 @@ recomputeNamespacePath(void)
{
/* If it ought to be the creation namespace, set flag */
if (oidlist == NIL)
- temp_missing = true;
+ *temp_missing = true;
}
}
else
@@ -3882,9 +3928,9 @@ recomputeNamespacePath(void)
* explicit from implicit mention of pg_catalog.)
*/
if (oidlist == NIL)
- firstNS = InvalidOid;
+ *firstNS = InvalidOid;
else
- firstNS = linitial_oid(oidlist);
+ *firstNS = linitial_oid(oidlist);
/*
* Add any implicitly-searched namespaces to the list. Note these go on
@@ -3898,6 +3944,95 @@ recomputeNamespacePath(void)
!list_member_oid(oidlist, myTempNamespace))
oidlist = lcons_oid(myTempNamespace, oidlist);
+ pfree(rawname);
+ list_free(namelist);
+
+ return oidlist;
+}
+
+/*
+ * Retrieve previously-computed search path information from the cache; or if
+ * not present, fully recompute it and add it to the cache.
+ */
+static List *
+cachedNamespacePath(const char *searchPath, Oid roleid, Oid *pfirstNS,
+ bool *p_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;
+ Oid firstNS;
+ bool temp_missing;
+
+ oidlist = processNamespacePath(searchPath, roleid, &firstNS,
+ &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->temp_missing = temp_missing;
+ entry->firstNS = firstNS;
+
+ MemoryContextSwitchTo(oldcxt);
+
+ /* oidlist already copied into cache entry */
+ list_free(oidlist);
+ }
+
+ *pfirstNS = entry->firstNS;
+ *p_temp_missing = entry->temp_missing;
+ return entry->oidlist;
+}
+
+/*
+ * recomputeNamespacePath - recompute path derived variables if needed.
+ */
+static void
+recomputeNamespacePath(void)
+{
+ Oid roleid = GetUserId();
+ List *oidlist;
+ List *newpath;
+ bool temp_missing;
+ Oid firstNS;
+ bool pathChanged;
+
+ /* Do nothing if an override search spec is active. */
+ if (overrideStack)
+ return;
+
+ /* Do nothing if path is already valid. */
+ if (baseSearchPathValid && namespaceUser == roleid)
+ return;
+
+ oidlist = cachedNamespacePath(namespace_search_path, roleid, &firstNS,
+ &temp_missing);
+
/*
* 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
@@ -3911,6 +4046,8 @@ recomputeNamespacePath(void)
}
else
{
+ MemoryContext oldcxt;
+
pathChanged = true;
/* Must save OID list in permanent storage. */
@@ -3943,11 +4080,6 @@ recomputeNamespacePath(void)
*/
if (pathChanged)
activePathGeneration++;
-
- /* Clean up. */
- pfree(rawname);
- list_free(namelist);
- list_free(oidlist);
}
/*
@@ -4102,6 +4234,7 @@ InitTempTableNamespace(void)
myTempNamespaceSubID = GetCurrentSubTransactionId();
baseSearchPathValid = false; /* need to rebuild list */
+ searchPathCacheValid = false;
}
/*
@@ -4127,6 +4260,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
@@ -4192,6 +4326,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
@@ -4351,6 +4486,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;
}
@@ -4385,6 +4524,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.
@@ -4394,6 +4536,7 @@ InitializeSearchPath(void)
(Datum) 0);
/* Force search path to be recomputed on next use */
baseSearchPathValid = false;
+ searchPathCacheValid = false;
}
}
@@ -4404,8 +4547,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;
}
/*
diff --git a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
index f80373aecc..7f76ebecf9 100644
--- a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
+++ b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
@@ -249,12 +249,6 @@ NOTICE: in process utility: superuser attempting SET
NOTICE: in object_access_hook_str: non-superuser attempting alter (subId=0x1000, set) [session_authorization]
ERROR: permission denied: set session_authorization
SELECT * FROM regress_test_table;
-NOTICE: in object access: superuser attempting namespace search (subId=0x0) [no report on violation, allowed]
-LINE 1: SELECT * FROM regress_test_table;
- ^
-NOTICE: in object access: superuser finished namespace search (subId=0x0) [no report on violation, allowed]
-LINE 1: SELECT * FROM regress_test_table;
- ^
NOTICE: in executor check perms: superuser attempting execute
NOTICE: in executor check perms: superuser finished execute
t
--
2.34.1
