Minor fixes for search path cache code.

Avoid leaving a dangling pointer in the unlikely event that
nsphash_create fails.  Improve comments, and fix formatting by
adding typedefs.list entries.

Discussion: https://postgr.es/m/3972900.1704145107@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2024-01-02 14:57:21 -05:00
parent 371b07e894
commit ae6bc39317
2 changed files with 24 additions and 23 deletions

View File

@ -156,6 +156,11 @@ static Oid namespaceUser = InvalidOid;
/* The above four values are valid only if baseSearchPathValid */ /* The above four values are valid only if baseSearchPathValid */
static bool baseSearchPathValid = true; static bool baseSearchPathValid = true;
/*
* Storage for search path cache. Clear searchPathCacheValid as a simple
* way to invalidate *all* the cache entries, not just the active one.
*/
static bool searchPathCacheValid = false; static bool searchPathCacheValid = false;
static MemoryContext SearchPathCacheContext = NULL; static MemoryContext SearchPathCacheContext = NULL;
@ -163,7 +168,7 @@ typedef struct SearchPathCacheKey
{ {
const char *searchPath; const char *searchPath;
Oid roleid; Oid roleid;
} SearchPathCacheKey; } SearchPathCacheKey;
typedef struct SearchPathCacheEntry typedef struct SearchPathCacheEntry
{ {
@ -176,7 +181,7 @@ typedef struct SearchPathCacheEntry
/* needed for simplehash */ /* needed for simplehash */
char status; char status;
} SearchPathCacheEntry; } SearchPathCacheEntry;
/* /*
* myTempNamespace is InvalidOid until and unless a TEMP namespace is set up * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
@ -281,8 +286,8 @@ spcachekey_equal(SearchPathCacheKey a, SearchPathCacheKey b)
*/ */
#define SPCACHE_RESET_THRESHOLD 256 #define SPCACHE_RESET_THRESHOLD 256
static nsphash_hash * SearchPathCache = NULL; static nsphash_hash *SearchPathCache = NULL;
static SearchPathCacheEntry * LastSearchPathCacheEntry = NULL; static SearchPathCacheEntry *LastSearchPathCacheEntry = NULL;
/* /*
* Create or reset search_path cache as necessary. * Create or reset search_path cache as necessary.
@ -296,8 +301,11 @@ spcache_init(void)
SearchPathCache->members < SPCACHE_RESET_THRESHOLD) SearchPathCache->members < SPCACHE_RESET_THRESHOLD)
return; return;
MemoryContextReset(SearchPathCacheContext); /* make sure we don't leave dangling pointers if nsphash_create fails */
SearchPathCache = NULL;
LastSearchPathCacheEntry = NULL; LastSearchPathCacheEntry = NULL;
MemoryContextReset(SearchPathCacheContext);
/* arbitrary initial starting size of 16 elements */ /* arbitrary initial starting size of 16 elements */
SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL); SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
searchPathCacheValid = true; searchPathCacheValid = true;
@ -325,8 +333,8 @@ spcache_lookup(const char *searchPath, Oid roleid)
}; };
entry = nsphash_lookup(SearchPathCache, cachekey); entry = nsphash_lookup(SearchPathCache, cachekey);
if (entry)
LastSearchPathCacheEntry = entry; LastSearchPathCacheEntry = entry;
return entry; return entry;
} }
} }
@ -4267,7 +4275,7 @@ recomputeNamespacePath(void)
{ {
Oid roleid = GetUserId(); Oid roleid = GetUserId();
bool pathChanged; bool pathChanged;
const SearchPathCacheEntry *entry; const SearchPathCacheEntry *entry;
/* Do nothing if path is already valid. */ /* Do nothing if path is already valid. */
if (baseSearchPathValid && namespaceUser == roleid) if (baseSearchPathValid && namespaceUser == roleid)
@ -4635,9 +4643,7 @@ check_search_path(char **newval, void **extra, GucSource source)
* schemas that don't exist; and often, we are not inside a transaction * 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 * here and so can't consult the system catalogs anyway. So now, the only
* requirement is syntactic validity of the identifier list. * requirement is syntactic validity of the identifier list.
*/ *
/*
* Checking only the syntactic validity also allows us to use the search * Checking only the syntactic validity also allows us to use the search
* path cache (if available) to avoid calling SplitIdentifierString() on * path cache (if available) to avoid calling SplitIdentifierString() on
* the same string repeatedly. * the same string repeatedly.
@ -4667,19 +4673,10 @@ check_search_path(char **newval, void **extra, GucSource source)
list_free(namelist); list_free(namelist);
return false; 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); pfree(rawname);
list_free(namelist); list_free(namelist);
/* create empty cache entry */ /* OK to create empty cache entry */
if (use_cache) if (use_cache)
(void) spcache_insert(searchPath, roleid); (void) spcache_insert(searchPath, roleid);
@ -4732,8 +4729,9 @@ InitializeSearchPath(void)
} }
else else
{ {
SearchPathCacheContext = AllocSetContextCreate( /* Make the context we'll keep search path cache hashtable in */
TopMemoryContext, "search_path processing cache", SearchPathCacheContext = AllocSetContextCreate(TopMemoryContext,
"search_path processing cache",
ALLOCSET_DEFAULT_SIZES); ALLOCSET_DEFAULT_SIZES);
/* /*

View File

@ -2480,6 +2480,8 @@ ScanState
ScanTypeControl ScanTypeControl
ScannerCallbackState ScannerCallbackState
SchemaQuery SchemaQuery
SearchPathCacheEntry
SearchPathCacheKey
SearchPathMatcher SearchPathMatcher
SecBuffer SecBuffer
SecBufferDesc SecBufferDesc
@ -3515,6 +3517,7 @@ needs_fmgr_hook_type
network_sortsupport_state network_sortsupport_state
nodeitem nodeitem
normal_rand_fctx normal_rand_fctx
nsphash_hash
ntile_context ntile_context
numeric numeric
object_access_hook_type object_access_hook_type