Improve test coverage of CLOBBER_CACHE_ALWAYS by having it also force

reloading of operator class information on each use of LookupOpclassInfo.
Had this been in place a year ago, it would have helped me find a bug
in the then-new 'operator family' code.  Now that we have a build farm
member testing CLOBBER_CACHE_ALWAYS on a regular basis, it seems worth
expending a little bit of effort here.

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index e28a79134e89f467e570f0d2e7a6b76eccbd6866..d0fe1e06ac8b97d8ed867da36eb304a07754b24b 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.264 2007/11/15 21:14:40 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.265 2007/11/28 20:44:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1133,9 +1133,13 @@ IndexSupportInitialize(oidvector *indclass,
  * numbers is passed in, rather than being looked up, mainly because the
  * caller will have it already.
  *
- * XXX There isn't any provision for flushing the cache.  However, there
- * isn't any provision for flushing relcache entries when opclass info
- * changes, either :-(
+ * Note there is no provision for flushing the cache.  This is OK at the
+ * moment because there is no way to ALTER any interesting properties of an
+ * existing opclass --- all you can do is drop it, which will result in
+ * a useless but harmless dead entry in the cache.  To support altering
+ * opclass membership (not the same as opfamily membership!), we'd need to
+ * be able to flush this cache as well as the contents of relcache entries
+ * for indexes.
  */
 static OpClassCacheEnt *
 LookupOpclassInfo(Oid operatorClassOid,
@@ -1170,34 +1174,50 @@ LookupOpclassInfo(Oid operatorClassOid,
                                                                                           (void *) &operatorClassOid,
                                                                                           HASH_ENTER, &found);
 
-       if (found && opcentry->valid)
+       if (!found)
+       {
+               /* Need to allocate memory for new entry */
+               opcentry->valid = false;        /* until known OK */
+               opcentry->numStrats = numStrats;
+               opcentry->numSupport = numSupport;
+
+               if (numStrats > 0)
+                       opcentry->operatorOids = (Oid *)
+                               MemoryContextAllocZero(CacheMemoryContext,
+                                                                          numStrats * sizeof(Oid));
+               else
+                       opcentry->operatorOids = NULL;
+
+               if (numSupport > 0)
+                       opcentry->supportProcs = (RegProcedure *)
+                               MemoryContextAllocZero(CacheMemoryContext,
+                                                                          numSupport * sizeof(RegProcedure));
+               else
+                       opcentry->supportProcs = NULL;
+       }
+       else
        {
-               /* Already made an entry for it */
                Assert(numStrats == opcentry->numStrats);
                Assert(numSupport == opcentry->numSupport);
-               return opcentry;
        }
 
-       /* Need to fill in new entry */
-       opcentry->valid = false;        /* until known OK */
-       opcentry->numStrats = numStrats;
-       opcentry->numSupport = numSupport;
-
-       if (numStrats > 0)
-               opcentry->operatorOids = (Oid *)
-                       MemoryContextAllocZero(CacheMemoryContext,
-                                                                  numStrats * sizeof(Oid));
-       else
-               opcentry->operatorOids = NULL;
+       /*
+        * When testing for cache-flush hazards, we intentionally disable the
+        * operator class cache and force reloading of the info on each call.
+        * This is helpful because we want to test the case where a cache flush
+        * occurs while we are loading the info, and it's very hard to provoke
+        * that if this happens only once per opclass per backend.
+        */
+#if defined(CLOBBER_CACHE_ALWAYS)
+       opcentry->valid = false;
+#endif
 
-       if (numSupport > 0)
-               opcentry->supportProcs = (RegProcedure *)
-                       MemoryContextAllocZero(CacheMemoryContext,
-                                                                  numSupport * sizeof(RegProcedure));
-       else
-               opcentry->supportProcs = NULL;
+       if (opcentry->valid)
+               return opcentry;
 
        /*
+        * Need to fill in new entry.
+        *
         * To avoid infinite recursion during startup, force heap scans if we're
         * looking up info for the opclasses used by the indexes we would like to
         * reference here.