Fix up flushing of composite-type typcache entries to be driven directly by

SI invalidation events, rather than indirectly through the relcache.

In the previous coding, we had to flush a composite-type typcache entry
whenever we discarded the corresponding relcache entry.  This caused problems
at least when testing with RELCACHE_FORCE_RELEASE, as shown in recent report
from Jeff Davis, and might result in real-world problems given the kind of
unexpected relcache flush that that test mechanism is intended to model.

The new coding decouples relcache and typcache management, which is a good
thing anyway from a structural perspective.  The cost is that we have to
search the typcache linearly to find entries that need to be flushed.  There
are a couple of ways we could avoid that, but at the moment it's not clear
it's worth any extra trouble, because the typcache contains very few entries
in typical operation.

Back-patch to 8.2, the same as some other recent fixes in this general area.
The patch could be carried back to 8.0 with some additional work, but given
that it's only hypothetical whether we're fixing any problem observable in
the field, it doesn't seem worth the work now.

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 166beb25b15c5b3cbecb0b463ce5ea5ddd22cffc..8ad11417c4145ccdaf44197df0406cb52aae5e0c 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.312 2010/08/13 20:10:52 rhaas Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.313 2010/09/02 03:16:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -77,7 +77,6 @@
 #include "utils/resowner.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
-#include "utils/typcache.h"
 
 
 /*
@@ -1854,8 +1853,6 @@ RelationDestroyRelation(Relation relation)
 static void
 RelationClearRelation(Relation relation, bool rebuild)
 {
-   Oid         old_reltype = relation->rd_rel->reltype;
-
    /*
     * As per notes above, a rel to be rebuilt MUST have refcnt > 0; while of
     * course it would be a bad idea to blow away one with nonzero refcnt.
@@ -1925,9 +1922,6 @@ RelationClearRelation(Relation relation, bool rebuild)
     */
    if (!rebuild)
    {
-       /* Flush any rowtype cache entry */
-       flush_rowtype_cache(old_reltype);
-
        /* Remove it from the hash table */
        RelationCacheDelete(relation);
 
@@ -1975,7 +1969,6 @@ RelationClearRelation(Relation relation, bool rebuild)
        if (newrel == NULL)
        {
            /* Should only get here if relation was deleted */
-           flush_rowtype_cache(old_reltype);
            RelationCacheDelete(relation);
            RelationDestroyRelation(relation);
            elog(ERROR, "relation %u deleted while still in use", save_relid);
@@ -1983,8 +1976,6 @@ RelationClearRelation(Relation relation, bool rebuild)
 
        keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att);
        keep_rules = equalRuleLocks(relation->rd_rules, newrel->rd_rules);
-       if (!keep_tupdesc)
-           flush_rowtype_cache(old_reltype);
 
        /*
         * Perform swapping of the relcache entry contents.  Within this

diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 35b635163623008d9bfcb468af3a13b4f16d0931..8d0d753a3e00a1e5fb75ce629f81ada3de69dbdb 100644 (file)
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -4,14 +4,14 @@
  *   POSTGRES type cache code
  *
  * The type cache exists to speed lookup of certain information about data
- * types that is not directly available from a type's pg_type row.  In
- * particular, we use a type's default btree opclass, or the default hash
+ * types that is not directly available from a type's pg_type row.  For
+ * example, we use a type's default btree opclass, or the default hash
  * opclass if no btree opclass exists, to determine which operators should
  * be used for grouping and sorting the type (GROUP BY, ORDER BY ASC/DESC).
  *
  * Several seemingly-odd choices have been made to support use of the type
  * cache by the generic array comparison routines array_eq() and array_cmp().
- * Because these routines are used as index support operations, they cannot
+ * Because those routines are used as index support operations, they cannot
  * leak memory.  To allow them to execute efficiently, all information that
  * either of them would like to re-use across calls is made available in the
  * type cache.
@@ -36,7 +36,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/cache/typcache.c,v 1.32 2010/02/14 18:42:17 rhaas Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/cache/typcache.c,v 1.33 2010/09/02 03:16:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,6 +48,7 @@
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "utils/builtins.h"
+#include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
@@ -86,6 +87,8 @@ static TupleDesc *RecordCacheArray = NULL;
 static int32 RecordCacheArrayLen = 0;  /* allocated length of array */
 static int32 NextRecordTypmod = 0;     /* number of entries used */
 
+static void TypeCacheRelCallback(Datum arg, Oid relid);
+
 
 /*
  * lookup_type_cache
@@ -116,6 +119,9 @@ lookup_type_cache(Oid type_id, int flags)
        TypeCacheHash = hash_create("Type information cache", 64,
                                    &ctl, HASH_ELEM | HASH_FUNCTION);
 
+       /* Also set up a callback for relcache SI invalidations */
+       CacheRegisterRelcacheCallback(TypeCacheRelCallback, (Datum) 0);
+
        /* Also make sure CacheMemoryContext exists */
        if (!CacheMemoryContext)
            CreateCacheMemoryContext();
@@ -497,36 +503,51 @@ assign_record_type_typmod(TupleDesc tupDesc)
 }
 
 /*
- * flush_rowtype_cache
+ * TypeCacheRelCallback
+ *     Relcache inval callback function
+ *
+ * Delete the cached tuple descriptor (if any) for the given rel's composite
+ * type, or for all composite types if relid == InvalidOid.
  *
- * If a typcache entry exists for a rowtype, delete the entry's cached
- * tuple descriptor link.  This is called from relcache.c when a cached
- * relation tupdesc is about to be dropped.
+ * This is called when a relcache invalidation event occurs for the given
+ * relid.  We must scan the whole typcache hash since we don't know the
+ * type OID corresponding to the relid.  We could do a direct search if this
+ * were a syscache-flush callback on pg_type, but then we would need all
+ * ALTER-TABLE-like commands that could modify a rowtype to issue syscache
+ * invals against the rel's pg_type OID.  The extra SI signaling could very
+ * well cost more than we'd save, since in most usages there are not very
+ * many entries in a backend's typcache.  The risk of bugs-of-omission seems
+ * high, too.
+ *
+ * Another possibility, with only localized impact, is to maintain a second
+ * hashtable that indexes composite-type typcache entries by their typrelid.
+ * But it's still not clear it's worth the trouble.
  */
-void
-flush_rowtype_cache(Oid type_id)
+static void
+TypeCacheRelCallback(Datum arg, Oid relid)
 {
+   HASH_SEQ_STATUS status;
    TypeCacheEntry *typentry;
 
-   if (TypeCacheHash == NULL)
-       return;                 /* no table, so certainly no entry */
-
-   typentry = (TypeCacheEntry *) hash_search(TypeCacheHash,
-                                             (void *) &type_id,
-                                             HASH_FIND, NULL);
-   if (typentry == NULL)
-       return;                 /* no matching entry */
-   if (typentry->tupDesc == NULL)
-       return;                 /* tupdesc hasn't been requested */
-
-   /*
-    * Release our refcount and free the tupdesc if none remain. (Can't use
-    * DecrTupleDescRefCount because this reference is not logged in current
-    * resource owner.)
-    */
-   Assert(typentry->tupDesc->tdrefcount > 0);
-   if (--typentry->tupDesc->tdrefcount == 0)
-       FreeTupleDesc(typentry->tupDesc);
+   /* TypeCacheHash must exist, else this callback wouldn't be registered */
+   hash_seq_init(&status, TypeCacheHash);
+   while ((typentry = (TypeCacheEntry *) hash_seq_search(&status)) != NULL)
+   {
+       if (typentry->tupDesc == NULL)
+           continue;   /* not composite, or tupdesc hasn't been requested */
 
-   typentry->tupDesc = NULL;
+       /* Delete if match, or if we're zapping all composite types */
+       if (relid == typentry->typrelid || relid == InvalidOid)
+       {
+           /*
+            * Release our refcount, and free the tupdesc if none remain.
+            * (Can't use DecrTupleDescRefCount because this reference is not
+            * logged in current resource owner.)
+            */
+           Assert(typentry->tupDesc->tdrefcount > 0);
+           if (--typentry->tupDesc->tdrefcount == 0)
+               FreeTupleDesc(typentry->tupDesc);
+           typentry->tupDesc = NULL;
+       }
+   }
 }

diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h
index 45d71bf882472567b593bdce54c3342c76344205..05b773245410c3bf05d5a7c9da569a180dd3f279 100644 (file)
--- a/src/include/utils/typcache.h
+++ b/src/include/utils/typcache.h
@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/typcache.h,v 1.18 2010/01/02 16:58:10 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/typcache.h,v 1.19 2010/09/02 03:16:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -86,6 +86,4 @@ extern TupleDesc lookup_rowtype_tupdesc_copy(Oid type_id, int32 typmod);
 
 extern void assign_record_type_typmod(TupleDesc tupDesc);
 
-extern void flush_rowtype_cache(Oid type_id);
-
 #endif   /* TYPCACHE_H */