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