diff options
author | Noah Misch <noah@leadboat.com> | 2024-09-24 15:25:18 -0700 |
---|---|---|
committer | Noah Misch <noah@leadboat.com> | 2024-09-24 15:25:18 -0700 |
commit | aac2c9b4fde889d13f859c233c2523345e72d32b (patch) | |
tree | 65d5e1b73059d4c423dee459bcf58bc0c47b8a9a /src/backend/commands/tablecmds.c | |
parent | a07e03fd8fa7daf4d1356f7cb501ffe784ea6257 (diff) | |
download | postgresql-aac2c9b4fde889d13f859c233c2523345e72d32b.tar.gz postgresql-aac2c9b4fde889d13f859c233c2523345e72d32b.zip |
For inplace update durability, make heap_update() callers wait.
The previous commit fixed some ways of losing an inplace update. It
remained possible to lose one when a backend working toward a
heap_update() copied a tuple into memory just before inplace update of
that tuple. In catalogs eligible for inplace update, use LOCKTAG_TUPLE
to govern admission to the steps of copying an old tuple, modifying it,
and issuing heap_update(). This includes MERGE commands. To avoid
changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when
holding a relation lock sufficient to exclude inplace updaters.
Back-patch to v12 (all supported versions). In v13 and v12, "UPDATE
pg_class" or "UPDATE pg_database" can still lose an inplace update. The
v14+ UPDATE fix needs commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35,
and it wasn't worth reimplementing that fix without such infrastructure.
Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas.
Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com
Diffstat (limited to 'src/backend/commands/tablecmds.c')
-rw-r--r-- | src/backend/commands/tablecmds.c | 28 |
1 files changed, 21 insertions, 7 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d27e6cf3451..87232e01376 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3593,6 +3593,7 @@ SetRelationTableSpace(Relation rel, { Relation pg_class; HeapTuple tuple; + ItemPointerData otid; Form_pg_class rd_rel; Oid reloid = RelationGetRelid(rel); @@ -3601,9 +3602,10 @@ SetRelationTableSpace(Relation rel, /* Get a modifiable copy of the relation's pg_class row. */ pg_class = table_open(RelationRelationId, RowExclusiveLock); - tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid)); + tuple = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(reloid)); if (!HeapTupleIsValid(tuple)) elog(ERROR, "cache lookup failed for relation %u", reloid); + otid = tuple->t_self; rd_rel = (Form_pg_class) GETSTRUCT(tuple); /* Update the pg_class row. */ @@ -3611,7 +3613,8 @@ SetRelationTableSpace(Relation rel, InvalidOid : newTableSpaceId; if (RelFileNumberIsValid(newRelFilenumber)) rd_rel->relfilenode = newRelFilenumber; - CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + CatalogTupleUpdate(pg_class, &otid, tuple); + UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock); /* * Record dependency on tablespace. This is only required for relations @@ -4105,6 +4108,7 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo { Relation targetrelation; Relation relrelation; /* for RELATION relation */ + ItemPointerData otid; HeapTuple reltup; Form_pg_class relform; Oid namespaceId; @@ -4127,7 +4131,8 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo */ relrelation = table_open(RelationRelationId, RowExclusiveLock); - reltup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid)); + reltup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(myrelid)); + otid = reltup->t_self; if (!HeapTupleIsValid(reltup)) /* shouldn't happen */ elog(ERROR, "cache lookup failed for relation %u", myrelid); relform = (Form_pg_class) GETSTRUCT(reltup); @@ -4154,7 +4159,8 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo */ namestrcpy(&(relform->relname), newrelname); - CatalogTupleUpdate(relrelation, &reltup->t_self, reltup); + CatalogTupleUpdate(relrelation, &otid, reltup); + UnlockTuple(relrelation, &otid, InplaceUpdateTupleLock); InvokeObjectPostAlterHookArg(RelationRelationId, myrelid, 0, InvalidOid, is_internal); @@ -15072,7 +15078,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, /* Fetch heap tuple */ relid = RelationGetRelid(rel); - tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + tuple = SearchSysCacheLocked1(RELOID, ObjectIdGetDatum(relid)); if (!HeapTupleIsValid(tuple)) elog(ERROR, "cache lookup failed for relation %u", relid); @@ -15176,6 +15182,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, repl_val, repl_null, repl_repl); CatalogTupleUpdate(pgclass, &newtuple->t_self, newtuple); + UnlockTuple(pgclass, &tuple->t_self, InplaceUpdateTupleLock); InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); @@ -17329,7 +17336,8 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid, ObjectAddress thisobj; bool already_done = false; - classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relOid)); + /* no rel lock for relkind=c so use LOCKTAG_TUPLE */ + classTup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relOid)); if (!HeapTupleIsValid(classTup)) elog(ERROR, "cache lookup failed for relation %u", relOid); classForm = (Form_pg_class) GETSTRUCT(classTup); @@ -17348,6 +17356,8 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid, already_done = object_address_present(&thisobj, objsMoved); if (!already_done && oldNspOid != newNspOid) { + ItemPointerData otid = classTup->t_self; + /* check for duplicate name (more friendly than unique-index failure) */ if (get_relname_relid(NameStr(classForm->relname), newNspOid) != InvalidOid) @@ -17360,7 +17370,9 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid, /* classTup is a copy, so OK to scribble on */ classForm->relnamespace = newNspOid; - CatalogTupleUpdate(classRel, &classTup->t_self, classTup); + CatalogTupleUpdate(classRel, &otid, classTup); + UnlockTuple(classRel, &otid, InplaceUpdateTupleLock); + /* Update dependency on schema if caller said so */ if (hasDependEntry && @@ -17372,6 +17384,8 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid, elog(ERROR, "could not change schema dependency for relation \"%s\"", NameStr(classForm->relname)); } + else + UnlockTuple(classRel, &classTup->t_self, InplaceUpdateTupleLock); if (!already_done) { add_exact_object_address(&thisobj, objsMoved); |