[go: up one dir, main page]
projects / postgresql.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 0f2835e)
Cope with inplace update making catcache stale during TOAST fetch.
authorNoah Misch <noah@leadboat.com>
Fri, 28 Jun 2024 02:21:06 +0000 (19:21 -0700)
committerNoah Misch <noah@leadboat.com>
Fri, 28 Jun 2024 02:21:11 +0000 (19:21 -0700)
This extends ad98fb14226ae6456fbaed7990ee7591cbe5efd2 to invals of
inplace updates.  Trouble requires an inplace update of a catalog having
a TOAST table, so only pg_database was at risk.  (The other catalog on
which core code performs inplace updates, pg_class, has no TOAST table.)
Trouble would require something like the inplace-inval.spec test.
Consider GRANT ... ON DATABASE fetching a stale row from cache and
discarding a datfrozenxid update that vac_truncate_clog() has already
relied upon.  Back-patch to v12 (all supported versions).

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240114201411.d0@rfd.leadboat.com
Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com

src/backend/catalog/catalog.c patch | blob | blame | history
src/backend/utils/cache/catcache.c patch | blob | blame | history
src/include/catalog/catalog.h patch | blob | blame | history

diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index e784538aaea49490be489ca6496cbcd0a30b9529..1cf998ee97591ac90b2a2f21ba81510919508a45 100644 (file)
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -137,6 +137,27 @@ IsCatalogRelationOid(Oid relid)
    return (relid < (Oid) FirstUnpinnedObjectId);
 }
 
+/*
+ * IsInplaceUpdateRelation
+ *     True iff core code performs inplace updates on the relation.
+ */
+bool
+IsInplaceUpdateRelation(Relation relation)
+{
+   return IsInplaceUpdateOid(RelationGetRelid(relation));
+}
+
+/*
+ * IsInplaceUpdateOid
+ *     Like the above, but takes an OID as argument.
+ */
+bool
+IsInplaceUpdateOid(Oid relid)
+{
+   return (relid == RelationRelationId ||
+           relid == DatabaseRelationId);
+}
+
 /*
  * IsToastRelation
  *     True iff relation is a TOAST support relation (or index).
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index d22bc07de955b93cb647523f1b391e807ccf808b..3b9434c324e6a318f2e20f54d4d4de96eed0a77d 100644 (file)
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -21,6 +21,7 @@
 #include "access/table.h"
 #include "access/valid.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
@@ -1843,6 +1844,23 @@ ReleaseCatCacheList(CatCList *list)
 }
 
 
+/*
+ * equalTuple
+ *     Are these tuples memcmp()-equal?
+ */
+static bool
+equalTuple(HeapTuple a, HeapTuple b)
+{
+   uint32      alen;
+   uint32      blen;
+
+   alen = a->t_len;
+   blen = b->t_len;
+   return (alen == blen &&
+           memcmp((char *) a->t_data,
+                  (char *) b->t_data, blen) == 0);
+}
+
 /*
  * CatalogCacheCreateEntry
  *     Create a new CatCTup entry, copying the given HeapTuple and other
@@ -1893,14 +1911,34 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, SysScanDesc scandesc,
         */
        if (HeapTupleHasExternal(ntp))
        {
+           bool        need_cmp = IsInplaceUpdateOid(cache->cc_reloid);
+           HeapTuple   before = NULL;
+           bool        matches = true;
+
+           if (need_cmp)
+               before = heap_copytuple(ntp);
            dtp = toast_flatten_tuple(ntp, cache->cc_tupdesc);
 
            /*
             * The tuple could become stale while we are doing toast table
-            * access (since AcceptInvalidationMessages can run then), so we
-            * must recheck its visibility afterwards.
+            * access (since AcceptInvalidationMessages can run then).
+            * equalTuple() detects staleness from inplace updates, while
+            * systable_recheck_tuple() detects staleness from normal updates.
+            *
+            * While this equalTuple() follows the usual rule of reading with
+            * a pin and no buffer lock, it warrants suspicion since an
+            * inplace update could appear at any moment.  It's safe because
+            * the inplace update sends an invalidation that can't reorder
+            * before the inplace heap change.  If the heap change reaches
+            * this process just after equalTuple() looks, we've not missed
+            * its inval.
             */
-           if (!systable_recheck_tuple(scandesc, ntp))
+           if (need_cmp)
+           {
+               matches = equalTuple(before, ntp);
+               heap_freetuple(before);
+           }
+           if (!matches || !systable_recheck_tuple(scandesc, ntp))
            {
                heap_freetuple(dtp);
                return NULL;
diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h
index 60c1215362a7318f50d59b3cfc5535b718078b51..3b9494781e94af6ce77b3f1b638ae6bdafbfa3f7 100644 (file)
--- a/src/include/catalog/catalog.h
+++ b/src/include/catalog/catalog.h
@@ -21,11 +21,13 @@
 extern bool IsSystemRelation(Relation relation);
 extern bool IsToastRelation(Relation relation);
 extern bool IsCatalogRelation(Relation relation);
+extern bool IsInplaceUpdateRelation(Relation relation);
 
 extern bool IsSystemClass(Oid relid, Form_pg_class reltuple);
 extern bool IsToastClass(Form_pg_class reltuple);
 
 extern bool IsCatalogRelationOid(Oid relid);
+extern bool IsInplaceUpdateOid(Oid relid);
 
 extern bool IsCatalogNamespace(Oid namespaceId);
 extern bool IsToastNamespace(Oid namespaceId);
This is the main PostgreSQL git repository.