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

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 3f777693ae9b9b817ecd641406ed5c497172a1af..dcb0d479e04b25c14262f260b31ea89b89b9aa95 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -156,6 +156,11 @@ static Oid namespaceUser = InvalidOid;
 
 /* The above four values are valid only if baseSearchPathValid */
 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 MemoryContext SearchPathCacheContext = NULL;
 
@@ -163,7 +168,7 @@ typedef struct SearchPathCacheKey
 {
        const char *searchPath;
        Oid                     roleid;
-}                      SearchPathCacheKey;
+} SearchPathCacheKey;
 
 typedef struct SearchPathCacheEntry
 {
@@ -176,7 +181,7 @@ typedef struct SearchPathCacheEntry
 
        /* needed for simplehash */
        char            status;
-}                      SearchPathCacheEntry;
+} SearchPathCacheEntry;
 
 /*
  * 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
 
-static nsphash_hash * SearchPathCache = NULL;
-static SearchPathCacheEntry * LastSearchPathCacheEntry = NULL;
+static nsphash_hash *SearchPathCache = NULL;
+static SearchPathCacheEntry *LastSearchPathCacheEntry = NULL;
 
 /*
  * Create or reset search_path cache as necessary.
@@ -296,8 +301,11 @@ spcache_init(void)
                SearchPathCache->members < SPCACHE_RESET_THRESHOLD)
                return;
 
-       MemoryContextReset(SearchPathCacheContext);
+       /* make sure we don't leave dangling pointers if nsphash_create fails */
+       SearchPathCache = NULL;
        LastSearchPathCacheEntry = NULL;
+
+       MemoryContextReset(SearchPathCacheContext);
        /* arbitrary initial starting size of 16 elements */
        SearchPathCache = nsphash_create(SearchPathCacheContext, 16, NULL);
        searchPathCacheValid = true;
@@ -325,8 +333,8 @@ spcache_lookup(const char *searchPath, Oid roleid)
                };
 
                entry = nsphash_lookup(SearchPathCache, cachekey);
-
-               LastSearchPathCacheEntry = entry;
+               if (entry)
+                       LastSearchPathCacheEntry = entry;
                return entry;
        }
 }
@@ -4267,7 +4275,7 @@ recomputeNamespacePath(void)
 {
        Oid                     roleid = GetUserId();
        bool            pathChanged;
-       const           SearchPathCacheEntry *entry;
+       const SearchPathCacheEntry *entry;
 
        /* Do nothing if path is already valid. */
        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
         * 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.
@@ -4667,19 +4673,10 @@ check_search_path(char **newval, void **extra, GucSource source)
                list_free(namelist);
                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);
 
-       /* create empty cache entry */
+       /* OK to create empty cache entry */
        if (use_cache)
                (void) spcache_insert(searchPath, roleid);
 
@@ -4732,8 +4729,9 @@ InitializeSearchPath(void)
        }
        else
        {
-               SearchPathCacheContext = AllocSetContextCreate(
-                                                                                                          TopMemoryContext, "search_path processing cache",
+               /* Make the context we'll keep search path cache hashtable in */
+               SearchPathCacheContext = AllocSetContextCreate(TopMemoryContext,
+                                                                                                          "search_path processing cache",
                                                                                                           ALLOCSET_DEFAULT_SIZES);
 
                /*

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 5f1a017d2dc2faf18c581a6ac99f44c16298c097..5fd46b7bd1f668e9de93acc18b46ad7cb5c0c01e 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2480,6 +2480,8 @@ ScanState
 ScanTypeControl
 ScannerCallbackState
 SchemaQuery
+SearchPathCacheEntry
+SearchPathCacheKey
 SearchPathMatcher
 SecBuffer
 SecBufferDesc
@@ -3515,6 +3517,7 @@ needs_fmgr_hook_type
 network_sortsupport_state
 nodeitem
 normal_rand_fctx
+nsphash_hash
 ntile_context
 numeric
 object_access_hook_type