From 83bb52375630ce93660bde938ffba93c6dc7fc63 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Sat, 28 Dec 2024 07:16:22 -0800 Subject: In REASSIGN OWNED of a database, lock the tuple as mandated. Commit aac2c9b4fde889d13f859c233c2523345e72d32b mandated such locking and attempted to fulfill that mandate, but it missed REASSIGN OWNED. Hence, it remained possible to lose VACUUM's inplace update of datfrozenxid if a REASSIGN OWNED processed that database at the same time. This didn't affect the other inplace-updated catalog, pg_class. For pg_class, REASSIGN OWNED calls ATExecChangeOwner() instead of the generic AlterObjectOwner_internal(), and ATExecChangeOwner() fulfills the locking mandate. Like in GRANT, implement this by following the locking protocol for any catalog subject to the generic AlterObjectOwner_internal(). It would suffice to do this for IsInplaceUpdateOid() catalogs only. Back-patch to v13 (all supported versions). Kirill Reshke. Reported by Alexander Kukushkin. Discussion: https://postgr.es/m/CAFh8B=mpKjAy4Cuun-HP-f_vRzh2HSvYFG3rhVfYbfEBUhBAGg@mail.gmail.com --- src/backend/commands/alter.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/backend/commands/alter.c') diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 6f27a492d09..feb5517a288 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -60,6 +60,7 @@ #include "miscadmin.h" #include "parser/parse_func.h" #include "rewrite/rewriteDefine.h" +#include "storage/lmgr.h" #include "tcop/utility.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -947,7 +948,9 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) Oid old_ownerId; Oid namespaceId = InvalidOid; - oldtup = get_catalog_object_by_oid(rel, Anum_oid, objectId); + /* Search tuple and lock it. */ + oldtup = + get_catalog_object_by_oid_extended(rel, Anum_oid, objectId, true); if (oldtup == NULL) elog(ERROR, "cache lookup failed for object %u of catalog \"%s\"", objectId, RelationGetRelationName(rel)); @@ -1046,6 +1049,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) /* Perform actual update */ CatalogTupleUpdate(rel, &newtup->t_self, newtup); + UnlockTuple(rel, &oldtup->t_self, InplaceUpdateTupleLock); + /* * Update owner dependency reference. When working on a large object, * we have to translate back to the OID conventionally used for LOs' @@ -1063,6 +1068,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) } else { + UnlockTuple(rel, &oldtup->t_self, InplaceUpdateTupleLock); + /* * No need to change anything. But when working on a large object, we * have to translate back to the OID conventionally used for LOs' -- cgit v1.2.3