diff options
author | Noah Misch <noah@leadboat.com> | 2024-06-27 19:21:06 -0700 |
---|---|---|
committer | Noah Misch <noah@leadboat.com> | 2024-06-27 19:21:06 -0700 |
commit | f9f47f0d93d1a493a3365625f96026c7b18d7cf5 (patch) | |
tree | a35c51f21bddc89825a64df1cca17ee5b255300b | |
parent | 5b823b179e5e8ab32f140658698ca08f8c83f06e (diff) | |
download | postgresql-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.c | 21 | ||||
-rw-r--r-- | src/backend/utils/cache/catcache.c | 44 | ||||
-rw-r--r-- | src/include/catalog/catalog.h | 2 |
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); |