Author: Noah Misch Commit: Noah Misch Cope with inplace updates making catcache entries stale during detoasting. This extends ad98fb14226ae6456fbaed7990ee7591cbe5efd2 to invals of inplace updates. PgDatabaseToastTable is the only one affected. 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 by FIXME. Discussion: https://postgr.es/m/FIXME diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 3217008..c7cb9bc 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)); +} + +/* + * IsInplaceUpdateRelation + * 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 569f51c..a734ab6 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -2008,6 +2008,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 +2074,45 @@ 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. + * must recheck its visibility afterwards. The recheck shall + * distinguish two cases: + * + * - Inval is not yet queued or not yet processed. This implies + * the current cache searcher accepts staleness in this tuple. A + * future searcher that can't accept staleness will take a lock + * conflicting with one the inval issuer held, so we won't miss + * the inval. + * + * - Inval message processed during toast_flatten_tuple(). We + * didn't get the memo. The inval message processing did do + * InvalidateCatalogSnapshot(), so systable_recheck_tuple() will + * return false where needed. + * + * AcceptInvalidationMessages could have processed an inval for an + * inplace update. 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 data change. If the change reaches + * this process just after we look, 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 1fd326e..a8dd304 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);