From 3c5662ff268ac2b2e803130b7199517160f5a4e9 Mon Sep 17 00:00:00 2001 From: Manu Date: Sun, 27 Sep 2026 13:08:36 -0300 Subject: [PATCH] Fix index corruption after rolling back ALTER TABLE SET TABLESPACE ALTER TABLE ... SET TABLESPACE rewrites a table's heap to a new relfilenode but deliberately leaves the table's indexes on their existing relfilenodes. The two then roll back by different mechanisms: on abort the heap's new file is discarded, so the heap TIDs consumed by rows inserted after the SET TABLESPACE become free again, while the index entries for those rows were written to the unchanged index files and survive the abort. A later insert can reuse a freed heap TID, leaving two index entries that point at the same live heap tuple. This surfaces as a _bt_posting_valid assertion failure in nbtree deduplication, and as duplicate rows through an index-only scan in gist. Make the indexes share the heap's rewrite: after moving the heap, give each of the table's indexes a new relfilenumber by copying it within its own tablespace, so that an abort discards the new heap and index files together and a commit keeps them together. Bug: #19686 Reported-by: Alexander Lakhin --- src/backend/commands/tablecmds.c | 93 ++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0274d892f2e..918d517ac23 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -695,6 +695,7 @@ static void ATPrepChangePersistence(AlteredTableInfo *tab, Relation rel, static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode); +static void ATExecSetTableSpaceNewIndexRelfilenumber(Oid indexOid, LOCKMODE lockmode); static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace); static void ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, @@ -17520,9 +17521,11 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) { Relation rel; Oid reltoastrelid; + char relkind; RelFileNumber newrelfilenumber; RelFileLocator newrlocator; List *reltoastidxids = NIL; + List *reltabidxids = NIL; ListCell *lc; /* @@ -17540,6 +17543,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) } reltoastrelid = rel->rd_rel->reltoastrelid; + relkind = rel->rd_rel->relkind; /* Fetch the list of indexes on toast relation if necessary */ if (OidIsValid(reltoastrelid)) { @@ -17586,6 +17590,13 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) RelationAssumeNewRelfilelocator(rel); + /* + * If this is a table, collect its index list now, while the relation is + * still open, so we can give each index a fresh relfilenumber below. + */ + if (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW) + reltabidxids = RelationGetIndexList(rel); + relation_close(rel, NoLock); /* Make sure the reltablespace change is visible */ @@ -17599,6 +17610,88 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) /* Clean up */ list_free(reltoastidxids); + + /* + * Moving a table's heap assigns it a new relfilenode, but its indexes are + * deliberately left in place with their existing relfilenodes. That mix + * is unsafe across a rollback: if the transaction inserts into the table + * after this and then aborts, the heap's new relfilenode is discarded and + * its file reverts to the pre-move contents, freeing the TIDs used by the + * aborted rows; but the matching index entries were written to the + * unchanged index files and survive the abort. A later insert can reuse a + * freed heap TID, leaving two index entries pointing at the same live heap + * tuple -- index corruption (bug #19686). Give each index a fresh + * relfilenumber, copied within its own tablespace, so it shares the heap's + * new-relfilenode fate: on abort the new heap and index files are all + * discarded together, and on commit they are all kept. + */ + foreach(lc, reltabidxids) + ATExecSetTableSpaceNewIndexRelfilenumber(lfirst_oid(lc), lockmode); + list_free(reltabidxids); +} + +/* + * Give one of a table's indexes a fresh relfilenumber within its existing + * tablespace, copying the current index file to the new relfilenumber. + * + * ATExecSetTableSpace() calls this for each index of a table whose heap it has + * just rewritten to a new relfilenode. The indexes must share that rewrite's + * transactional fate, otherwise an abort discards the heap's new file (freeing + * its TIDs) while leaving behind index entries that were written during the + * transaction, which a later insert can then alias -- index corruption (bug + * #19686). Unlike a full reindex this merely copies the existing index file, + * so the added cost stays close to that of the heap move itself. + */ +static void +ATExecSetTableSpaceNewIndexRelfilenumber(Oid indexOid, LOCKMODE lockmode) +{ + Relation ind; + RelFileNumber newrelfilenumber; + RelFileLocator newrlocator; + Relation pg_class; + HeapTuple tuple; + ItemPointerData otid; + Form_pg_class rd_rel; + + ind = relation_open(indexOid, lockmode); + + /* Only plain indexes have storage that can hold the stale entries. */ + if (ind->rd_rel->relkind != RELKIND_INDEX || + !RELKIND_HAS_STORAGE(ind->rd_rel->relkind)) + { + relation_close(ind, NoLock); + return; + } + + /* Allocate a new relfilenumber in the index's current tablespace. */ + newrelfilenumber = GetNewRelFileNumber(ind->rd_rel->reltablespace, NULL, + ind->rd_rel->relpersistence); + newrlocator = ind->rd_locator; + newrlocator.relNumber = newrelfilenumber; + + /* Copy the index into the new file and schedule the old one for cleanup. */ + index_copy_data(ind, newrlocator); + + /* Update the pg_class row; only the relfilenode changes. */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + tuple = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(indexOid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for index %u", indexOid); + otid = tuple->t_self; + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + rd_rel->relfilenode = newrelfilenumber; + CatalogTupleUpdate(pg_class, &otid, tuple); + UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock); + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + + InvokeObjectPostAlterHook(RelationRelationId, indexOid, 0); + RelationAssumeNewRelfilelocator(ind); + + relation_close(ind, NoLock); + + /* Make the relfilenode change visible. */ + CommandCounterIncrement(); } /* -- 2.55.0