aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoah Misch <noah@leadboat.com>2024-06-27 19:21:06 -0700
committerNoah Misch <noah@leadboat.com>2024-06-27 19:21:06 -0700
commitf9f47f0d93d1a493a3365625f96026c7b18d7cf5 (patch)
treea35c51f21bddc89825a64df1cca17ee5b255300b
parent5b823b179e5e8ab32f140658698ca08f8c83f06e (diff)
downloadpostgresql-f9f47f0d93d1a493a3365625f96026c7b18d7cf5.tar.gz
postgresql-f9f47f0d93d1a493a3365625f96026c7b18d7cf5.zip
Cope with inplace update making catcache stale during TOAST fetch.
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
-rw-r--r--src/backend/catalog/catalog.c21
-rw-r--r--src/backend/utils/cache/catcache.c44
-rw-r--r--src/include/catalog/catalog.h2
3 files changed, 64 insertions, 3 deletions
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index 32170083fa3..6c39434a306 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -136,6 +136,27 @@ IsCatalogRelationOid(Oid relid)
}
/*
+ * 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 569f51cb334..111d8a280a0 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -19,6 +19,7 @@
#include "access/relscan.h"
#include "access/table.h"
#include "access/xact.h"
+#include "catalog/catalog.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
#include "common/hashfn.h"
@@ -2008,6 +2009,23 @@ ReleaseCatCacheListWithOwner(CatCList *list, ResourceOwner resowner)
/*
+ * 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
* supplied data into it. The new entry initially has refcount 0.
@@ -2057,14 +2075,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 1fd326e48c2..a8dd304b1ad 100644
--- 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);