From 67f74e98b656261af329d36f628fbdfbf7283199 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Wed, 16 Sep 2026 19:23:08 +0100 Subject: [PATCH-v2 2/3] fix: deferred relation copy When running ALTER TABLE SET TABLESPACE on a table externally referenced by TID, e.g. having indices, outside the transaction, defer the copy of the relation fields to the commit, in order to keep the index consistent with the heap outside the transaction. --- src/backend/access/transam/xact.c | 14 +++ src/backend/commands/tablecmds.c | 181 +++++++++++++++++++++++++++++- src/include/commands/tablecmds.h | 5 + 3 files changed, 199 insertions(+), 1 deletion(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index ebb010853cf..e76d72851fb 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2351,6 +2351,12 @@ CommitTransaction(void) */ PreCommit_on_commit_actions(); + /* + * Perform the physical copy of relation files that were deferred during + * execution of SET TABLESPACE commands. + */ + PreCommit_deferred_tablespace_moves(); + /* * Synchronize files that are created and not WAL-logged during this * transaction. This must happen before AtEOXact_RelationMap(), so that we @@ -2508,6 +2514,7 @@ CommitTransaction(void) AtEOXact_SPI(true); AtEOXact_Enum(); AtEOXact_on_commit_actions(true); + AtEOXact_deferred_tablespace_moves(true); AtEOXact_Namespace(true, is_parallel_worker); AtEOXact_SMgr(); AtEOXact_Files(true); @@ -2613,6 +2620,7 @@ PrepareTransaction(void) * cursors, to avoid dangling-reference problems) */ PreCommit_on_commit_actions(); + PreCommit_deferred_tablespace_moves(); /* * Synchronize files that are created and not WAL-logged during this @@ -2804,6 +2812,7 @@ PrepareTransaction(void) AtEOXact_SPI(true); AtEOXact_Enum(); AtEOXact_on_commit_actions(true); + AtEOXact_deferred_tablespace_moves(true); AtEOXact_Namespace(true, false); AtEOXact_SMgr(); AtEOXact_Files(true); @@ -3035,6 +3044,7 @@ AbortTransaction(void) AtEOXact_SPI(false); AtEOXact_Enum(); AtEOXact_on_commit_actions(false); + AtEOXact_deferred_tablespace_moves(false); AtEOXact_Namespace(false, is_parallel_worker); AtEOXact_SMgr(); AtEOXact_Files(false); @@ -5240,6 +5250,8 @@ CommitSubTransaction(void) AtEOSubXact_SPI(true, s->subTransactionId); AtEOSubXact_on_commit_actions(true, s->subTransactionId, s->parent->subTransactionId); + AtEOSubXact_deferred_tablespace_moves(true, s->subTransactionId, + s->parent->subTransactionId); AtEOSubXact_Namespace(true, s->subTransactionId, s->parent->subTransactionId); AtEOSubXact_Files(true, s->subTransactionId, @@ -5415,6 +5427,8 @@ AbortSubTransaction(void) AtEOSubXact_SPI(false, s->subTransactionId); AtEOSubXact_on_commit_actions(false, s->subTransactionId, s->parent->subTransactionId); + AtEOSubXact_deferred_tablespace_moves(false, s->subTransactionId, + s->parent->subTransactionId); AtEOSubXact_Namespace(false, s->subTransactionId, s->parent->subTransactionId); AtEOSubXact_Files(false, s->subTransactionId, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1e6d0b1611f..1b9533230c1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -133,6 +133,16 @@ typedef struct OnCommitItem static List *on_commits = NIL; +typedef struct PendingTablespaceMove +{ + Oid relid; + Oid newTableSpace; + SubTransactionId creating_subid; +} PendingTablespaceMove; + +static List *deferred_tablespace_moves = NIL; + + /* * State information for ALTER TABLE @@ -695,6 +705,9 @@ static void ATPrepChangePersistence(AlteredTableInfo *tab, Relation rel, bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); +static bool ATRelationIsFreeToMove(Relation rel); +static void ATExecSetTableSpaceCopy(Oid tableOid, Oid newTableSpace, + LOCKMODE lockmode); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode); static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace); static void ATExecSetRelOptions(Relation rel, List *defList, @@ -17516,7 +17529,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, * rewriting to be done, so we just want to copy the data as fast as possible. */ static void -ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) +ATExecSetTableSpaceCopy(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) { Relation rel; Oid reltoastrelid; @@ -17539,6 +17552,9 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) return; } + elog(DEBUG1, "ATExecSetTableSpaceCopy: rel %d to tblspace %d (copying)", + tableOid, newTableSpace + ); reltoastrelid = rel->rd_rel->reltoastrelid; /* Fetch the list of indexes on toast relation if necessary */ if (OidIsValid(reltoastrelid)) @@ -17608,6 +17624,75 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) list_free(reltoastidxids); } + +/* + * ATRelationIsFreeToMove + * + * Determines whether a relation file can be copied immediately or it has to + * wait until the transaction commit. + * + * When the relation has indices in the old tablespace because we have to + * keep also invisible rows produced after the tablespace update must remain + * consistent in the case of a rollback. + */ +static bool +ATRelationIsFreeToMove(Relation rel) +{ + char relkind = rel->rd_rel->relkind; + + if (relkind != RELKIND_RELATION && relkind != RELKIND_MATVIEW) + return true; + + return RelationGetIndexList(rel) == NIL; +} + +/* + * ATExecSetTableSpace + * + * Either copy the relation files, or push the an item to the list of + * deferred tablespace moves, that are handled at commit. + */ +static void +ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) +{ + Relation rel; + PendingTablespaceMove *pending; + + rel = relation_open(tableOid, lockmode); + + if (ATRelationIsFreeToMove(rel)) + { + relation_close(rel, NoLock); + ATExecSetTableSpaceCopy(tableOid, newTableSpace, lockmode); + } + else + { + MemoryContext oldcxt; + + CheckRelationTableSpaceMove(rel, newTableSpace); + + elog(DEBUG1, "ATExecSetTableSpace: rel %d to tblspace %d (deferred copy)", + tableOid, newTableSpace + ); + + /* + * Keep both the list cells and entries in TopTransactionContext for + * the whole transaction, like on_commits uses CacheMemoryContext. + */ + oldcxt = MemoryContextSwitchTo(TopTransactionContext); + pending = MemoryContextAlloc(TopTransactionContext, + sizeof(PendingTablespaceMove)); + pending->relid = tableOid; + pending->newTableSpace = newTableSpace; + pending->creating_subid = GetCurrentSubTransactionId(); + deferred_tablespace_moves = lappend(deferred_tablespace_moves, pending); + + MemoryContextSwitchTo(oldcxt); + + relation_close(rel, NoLock); + } +} + /* * Special handling of ALTER TABLE SET TABLESPACE for relations with no * storage that have an interest in preserving tablespace. @@ -20113,6 +20198,100 @@ PreCommit_on_commit_actions(void) } } + +static int +deferred_tablespace_move_compare_relid(const ListCell *a, const ListCell *b) +{ + PendingTablespaceMove *ca = lfirst_node(PendingTablespaceMove, a); + PendingTablespaceMove *cb = lfirst_node(PendingTablespaceMove, b); + int t = pg_cmp_u32(ca->relid, cb->relid); + + if (t == 0) /* compare pointers for stability */ + t = (a < b) ? -1 : 1; + return t; +} + +/* + * Perform the file copies that might have been deferred during the + * transaction. + * See also: ATRelationIsFreeToMove + */ +void +PreCommit_deferred_tablespace_moves(void) +{ + ListCell *lc; + PendingTablespaceMove *prev = NULL; + PendingTablespaceMove *pending = NULL; + + /* + * Cluster moves by relation id, then for each relation apply the last + * change. + */ + list_sort(deferred_tablespace_moves, deferred_tablespace_move_compare_relid); + foreach(lc, deferred_tablespace_moves) + { + pending = (PendingTablespaceMove *) lfirst(lc); + if (prev != NULL && prev->relid != pending->relid) + { + ATExecSetTableSpaceCopy(prev->relid, prev->newTableSpace, + AccessExclusiveLock); + } + prev = pending; + } + + if (prev != NULL) + ATExecSetTableSpaceCopy(prev->relid, prev->newTableSpace, + AccessExclusiveLock); + deferred_tablespace_moves = NIL; +} + +/* + * Propagate deferred tablespace moves from sub transactions. + * The move should be performed only at the commit of the top level + * transaction. + */ +void +AtEOSubXact_deferred_tablespace_moves(bool isCommit, SubTransactionId mySubid, + SubTransactionId parentSubid) +{ + ListCell *cur_item; + + foreach(cur_item, deferred_tablespace_moves) + { + PendingTablespaceMove *pending = (PendingTablespaceMove *) lfirst(cur_item); + + if (pending->creating_subid != mySubid) + continue; + + if (isCommit) + { + pending->creating_subid = parentSubid; + continue; + } + + deferred_tablespace_moves = foreach_delete_current(deferred_tablespace_moves, + cur_item); + pfree(pending); + } +} + +/* + * Clean up deferred tablespace moves list at top level transaction + */ +void +AtEOXact_deferred_tablespace_moves(bool isCommit) +{ + if (!isCommit) + { + /* + * TopTransactionContext is about to be reset, so just drop the + * pointer. Do not pfree entries here. + */ + deferred_tablespace_moves = NIL; + } +} + + /* * Post-commit or post-abort cleanup for ON COMMIT management. * diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index c3d8518cb62..c964d0c7bf7 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -93,8 +93,13 @@ extern void check_of_type(HeapTuple typetuple); extern void register_on_commit_action(Oid relid, OnCommitAction action); extern void remove_on_commit_action(Oid relid); +extern void PreCommit_deferred_tablespace_moves(void); extern void PreCommit_on_commit_actions(void); +extern void AtEOXact_deferred_tablespace_moves(bool isCommit); extern void AtEOXact_on_commit_actions(bool isCommit); +extern void AtEOSubXact_deferred_tablespace_moves(bool isCommit, + SubTransactionId mySubid, + SubTransactionId parentSubid); extern void AtEOSubXact_on_commit_actions(bool isCommit, SubTransactionId mySubid, SubTransactionId parentSubid); -- 2.53.0