Hi Alexandre,
Thanks -- and for going with this approach. You raised two things: the
comment still described the pre-patch behavior, and it needed test
cases. Both are addressed in the attached v2:
- The comment no longer narrates the pre-patch behavior. It now says,
in the present tense, that the heap has just been rewritten and the
indexes must share that fate, and leaves the detail to
ATExecSetTableSpaceNewIndexRelfilenumber.
- There is a regression test now, in the tablespace suite. It runs
the rollback-then-reinsert sequence and checks, with seqscans off,
that the index agrees with the single live heap row -- it returns
two against the bug, where the aborted transaction's index entry
survives and aliases the reused TID. check-world is green with the
patch.
The approach is the one we settled on: after the heap move, each of the
table's indexes gets a fresh relfilenumber copied within its own
tablespace, so an abort discards the new heap and index files together.
It does not change the indexes' tablespace, matching the documented
behavior that SET TABLESPACE leaves indexes in place.
Regards,
Manu
From a7758293e3bb4c36b93571435c60dfd23294f324 Mon Sep 17 00:00:00 2001
From: Manuel Reyes Bravo <[email protected]>
Date: Thu, 1 Oct 2026 21:37:04 -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 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, freeing the heap TIDs consumed by rows
inserted after the SET TABLESPACE, while the index entries for those rows
were written to the unchanged index files and survive. 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.
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. This does not change the indexes' tablespace, matching the
documented behavior that SET TABLESPACE leaves indexes in place.
Add a regression test to the tablespace suite.
Bug: #19686
Reported-by: Alexander Lakhin
---
src/backend/commands/tablecmds.c | 86 ++++++++++++++++++++++++
src/test/regress/expected/tablespace.out | 25 +++++++
src/test/regress/sql/tablespace.sql | 21 ++++++
3 files changed, 132 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fd144d783d9..2de51df9fa8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -701,6 +701,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,
@@ -17530,9 +17531,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;
/*
@@ -17550,6 +17553,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))
{
@@ -17596,6 +17600,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 */
@@ -17609,6 +17620,81 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
/* Clean up */
list_free(reltoastidxids);
+
+ /*
+ * The heap now has a new relfilenode. Give each of the table's indexes a
+ * fresh relfilenode too, so that the indexes share the heap's rewrite fate
+ * across commit and abort; otherwise a rolled-back insert can leave an
+ * index entry that a later insert's reused heap TID aliases -- index
+ * corruption (bug #19686). See ATExecSetTableSpaceNewIndexRelfilenumber.
+ */
+ 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();
}
/*
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index f0dd25cdf0c..7d7215555ad 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -951,6 +951,31 @@ ERROR: permission denied for tablespace regress_tblspace
REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- fail
ERROR: permission denied for tablespace regress_tblspace
RESET ROLE;
+-- bug #19686: rolling back ALTER TABLE SET TABLESPACE must not leave the
+-- table's indexes referencing heap TIDs that the aborted heap rewrite frees.
+-- The indexes have to share the heap's rewrite, so that a later insert cannot
+-- reuse a freed TID that a surviving index entry still points at.
+CREATE TABLE tbspace_19686 (a int);
+CREATE INDEX tbspace_19686_idx ON tbspace_19686 (a);
+BEGIN;
+ALTER TABLE tbspace_19686 SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_19686 VALUES (0);
+ROLLBACK;
+INSERT INTO tbspace_19686 VALUES (0);
+-- With seqscans disabled this count is answered from the index; it must match
+-- the single live heap row (it returns 2 against the bug, where the aborted
+-- transaction's index entry survives and aliases the reused TID).
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_19686 WHERE a = 0;
+ count
+-------
+ 1
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_19686;
ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
ALTER INDEX ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index c43a59e5957..0f79a2708a3 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -420,6 +420,27 @@ REINDEX (TABLESPACE regress_tblspace) TABLE tablespace_table; -- fail
REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- fail
RESET ROLE;
+-- bug #19686: rolling back ALTER TABLE SET TABLESPACE must not leave the
+-- table's indexes referencing heap TIDs that the aborted heap rewrite frees.
+-- The indexes have to share the heap's rewrite, so that a later insert cannot
+-- reuse a freed TID that a surviving index entry still points at.
+CREATE TABLE tbspace_19686 (a int);
+CREATE INDEX tbspace_19686_idx ON tbspace_19686 (a);
+BEGIN;
+ALTER TABLE tbspace_19686 SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_19686 VALUES (0);
+ROLLBACK;
+INSERT INTO tbspace_19686 VALUES (0);
+-- With seqscans disabled this count is answered from the index; it must match
+-- the single live heap row (it returns 2 against the bug, where the aborted
+-- transaction's index entry survives and aliases the reused TID).
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_19686 WHERE a = 0;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_19686;
+
ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default;
--
2.55.0
