Re: BUG #19686: Rolling back SET TABLESPACE

[Manu]

Hi Shihao,

Thanks for catching this.

> A sql_drop event trigger also runs after the ALTER, and v5 only
> checks ddl_command_end.

Right, and it is not the only one.  An ALTER that also adds a CHECK
constraint validates it on the table's inheritance children after the
table itself was moved, so a volatile check function can write to the
table and then fail.  That needs no event trigger, so no superuser: an
ordinary role that owns the table leaves 50 stale index entries with
v5, and your diff does not cover it.

Rather than list every such place, v6 keeps the shortcut only for a
plain ALTER TABLE ... SET TABLESPACE of a single table, with no other
subcommand.  Then nothing else is in the work queue, nothing is dropped
and nothing is validated, so a ddl_command_end trigger and a pipeline
are all that remain, as in v5.  Any combined ALTER copies the indexes.

Stale entries after the abort, for v5, v5 with your diff, and v6:

  sql_drop trigger: 50, 0, 0
  CHECK on an inheritance child: 50, 50, 0
  ddl_command_end trigger, pipeline: 0, 0, 0

A plain SET TABLESPACE still leaves the index files alone.  The
regression test gains the child CHECK case.  It passes on master,
REL_15 and REL_14 (243, 217 and 216 tests), and the standby, crash
and wal_level=minimal checks stay clean.

> Also, ALTER TABLE ALL IN TABLESPACE always copies the indexes. Is
> that on purpose?

Yes.  It moves each table through AlterTableInternal(), which has no
utility context, and moves them all in one transaction, which can
also be a transaction block, so the shortcut's condition cannot hold
there.

Attached are v6 for master, REL_15 and REL_14, and the change from v5.

Regards,
Manu

From 601ecf002d25427a7081d506df162c105a10fb92 Mon Sep 17 00:00:00 2001
From: Manuel Reyes Bravo <manuelreyesbr...@gmail.com>
Date: Thu, 1 Oct 2026 21:37:04 -0300
Subject: [PATCH v6] 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.  A TRUNCATE after the move in the same
transaction truncates the old index files in place, which a rollback
cannot undo either.

Fix by giving each index of the table a new relfilenumber in the same
ALTER, copying it within its own tablespace (the indexes stay where they
are, as documented), so that an abort discards the new heap and index
files together.  Two cases need no copy:

- The ALTER is a plain ALTER TABLE ... SET TABLESPACE of a single
  table, with no other subcommand, issued as a top-level statement
  outside a transaction block, so nothing can modify the table before
  the transaction ends.  Any other subcommand could run user code after
  the move: the validation of an inheritance child's new CHECK
  constraint, a sql_drop event trigger, and so on.  This also requires
  that no ddl_command_end event trigger runs after it, and the commit is
  forced right after the statement, as for PreventInTransactionBlock, so
  that a later statement of an extended-protocol pipeline cannot share
  the transaction.

- The index's current file was created in the current subtransaction,
  for example an index created, or moved, earlier in it.  An abort
  discards that file together with the heap's new one.  This also lets
  a transaction move the indexes first and then the table without
  copying the indexes twice.

The regression test checks that the ALTER on its own leaves the index
files untouched; that combined with a CHECK constraint whose validation
on an inheritance child writes to the table after the move and fails,
it leaves no stale index entry; that in a transaction block a rollback
after inserting or truncating leaves the index file at its
pre-transaction size; that an index created or moved earlier in the
subtransaction is not copied; and that after a rollback to a savepoint,
an index created, reindexed or already copied before the savepoint
returns no row for the rolled-back values.

Bug: #19686
Reported-by: Alexander Lakhin
Suggested-by: Andres Freund
Reviewed-by: Alexandre Felipe
Reviewed-by: Shihao Zhong
---
 src/backend/commands/tablecmds.c         | 145 ++++++++++++++++++++-
 src/backend/tcop/utility.c               |   1 +
 src/include/tcop/utility.h               |   1 +
 src/test/regress/expected/tablespace.out | 153 +++++++++++++++++++++++
 src/test/regress/sql/tablespace.sql      | 102 +++++++++++++++
 5 files changed, 397 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fd144d783d9..a59581cad9a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -98,6 +98,7 @@
 #include "tcop/utility.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
+#include "utils/evtcache.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
@@ -700,7 +701,11 @@ static void ATPrepChangePersistence(AlteredTableInfo *tab, 
Relation rel,
                                                                        bool 
toLogged);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                                                                const char 
*tablespacename, LOCKMODE lockmode);
-static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode);
+static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode,
+                                                               bool 
copyIndexes);
+static bool ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                                          
AlterTableUtilityContext *context);
+static void ATExecSetTableSpaceNewIndexRelfilenumber(Oid indexOid, LOCKMODE 
lockmode);
 static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
 static void ATExecSetRelOptions(Relation rel, List *defList,
                                                                AlterTableType 
operation,
@@ -6097,7 +6102,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, 
LOCKMODE lockmode,
                         * just do a block-by-block copy.
                         */
                        if (tab->newTableSpace)
-                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode);
+                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode,
+                                                                       
ATSetTableSpaceCopyIndexes(parsetree, *wqueue,
+                                                                               
                                           context));
                }
 
                /*
@@ -17526,13 +17533,16 @@ 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)
+ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode,
+                                       bool copyIndexes)
 {
        Relation        rel;
        Oid                     reltoastrelid;
+       char            relkind;
        RelFileNumber newrelfilenumber;
        RelFileLocator newrlocator;
        List       *reltoastidxids = NIL;
+       List       *reltabidxids = NIL;
        ListCell   *lc;
 
        /*
@@ -17550,6 +17560,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 +17607,14 @@ 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 (copyIndexes &&
+               (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW))
+               reltabidxids = RelationGetIndexList(rel);
+
        relation_close(rel, NoLock);
 
        /* Make sure the reltablespace change is visible */
@@ -17603,12 +17622,128 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode)
 
        /* Move associated toast relation and/or indexes, too */
        if (OidIsValid(reltoastrelid))
-               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode);
+               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode, 
copyIndexes);
        foreach(lc, reltoastidxids)
-               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode);
+               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode, 
copyIndexes);
 
        /* 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.  See 
ATExecSetTableSpaceNewIndexRelfilenumber.
+        */
+       foreach(lc, reltabidxids)
+               ATExecSetTableSpaceNewIndexRelfilenumber(lfirst_oid(lc), 
lockmode);
+       list_free(reltabidxids);
+}
+
+/*
+ * Does ALTER TABLE SET TABLESPACE need to give the table's indexes new files?
+ *
+ * Only if something can still modify the table in the same transaction after
+ * the move.  Rather than listing every place where user code can run after
+ * it -- the validation of other tables in the work queue, such as an
+ * inheritance child getting a new CHECK constraint, foreign key validation,
+ * sql_drop and table_rewrite event triggers, and more -- the files are kept
+ * only for the one case where none of them exists: a plain
+ * "ALTER TABLE ... SET TABLESPACE" of a single table, with no other
+ * subcommand, issued as a top-level statement outside any transaction block.
+ * What can still run after it is a ddl_command_end event trigger, and a
+ * further statement of an extended-protocol pipeline, which would share the
+ * transaction until the next Sync.  Rule out the former by looking for such
+ * triggers, and the latter by forcing the commit right after the ALTER, as
+ * PreventInTransactionBlock does.
+ */
+static bool
+ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                  AlterTableUtilityContext 
*context)
+{
+       if (context == NULL || IsInTransactionBlock(context->isTopLevel))
+               return true;
+       if (parsetree == NULL || list_length(parsetree->cmds) != 1 ||
+               castNode(AlterTableCmd, linitial(parsetree->cmds))->subtype != 
AT_SetTableSpace ||
+               list_length(wqueue) != 1)
+               return true;
+       if (EventCacheLookup(EVT_DDLCommandEnd) != NIL)
+               return true;
+
+       MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT;
+       return false;
+}
+
+/*
+ * 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 the heap's
+ * transactional fate: an abort has to discard the index entries written during
+ * the transaction together with the heap's new file, since the heap TIDs those
+ * entries point at are freed by the abort and can be reused by later inserts.
+ * Copying the existing index file keeps the added cost close to that of the
+ * heap move itself; the index stays in its own tablespace, as documented.
+ */
+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.  An
+        * index whose current file was created in this subtransaction (the 
index
+        * itself, or a new file for it) is discarded together with the heap's 
new
+        * file on abort, so it needs no copy.  rd_newRelfilelocatorSubid can 
be zero after
+        * a rollback to a savepoint even though the file is new; then this 
falls
+        * back to rd_createSubid, and copies if that does not match either.
+        */
+       if (ind->rd_rel->relkind != RELKIND_INDEX ||
+               !RELKIND_HAS_STORAGE(ind->rd_rel->relkind) ||
+               Max(ind->rd_newRelfilelocatorSubid, ind->rd_createSubid) ==
+               GetCurrentSubTransactionId())
+       {
+               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/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 4d33fcb5e9d..57d88f371ef 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1315,6 +1315,7 @@ ProcessUtilitySlow(ParseState *pstate,
                                                atcontext.relid = relid;
                                                atcontext.params = params;
                                                atcontext.queryEnv = queryEnv;
+                                               atcontext.isTopLevel = 
isTopLevel;
 
                                                /* ... ensure we have an event 
trigger context ... */
                                                
EventTriggerAlterTableStart(parsetree);
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index abbdb401bf6..0357550da71 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -34,6 +34,7 @@ typedef struct AlterTableUtilityContext
        Oid                     relid;                  /* OID of ALTER's 
target table */
        ParamListInfo params;           /* any parameters available to ALTER 
TABLE */
        QueryEnvironment *queryEnv; /* execution environment for ALTER TABLE */
+       bool            isTopLevel;             /* ALTER TABLE is a top-level 
statement */
 } AlterTableUtilityContext;
 
 /*
diff --git a/src/test/regress/expected/tablespace.out 
b/src/test/regress/expected/tablespace.out
index f0dd25cdf0c..aec469aee9f 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -951,6 +951,159 @@ 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;
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+ idx_file_kept 
+---------------
+ t
+(1 row)
+
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+ idx_file_new 
+--------------
+ t
+(1 row)
+
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ idx_grew 
+----------
+ t
+(1 row)
+
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+SELECT count(*) FROM tbspace_rollback;
+ count 
+-------
+   100
+(1 row)
+
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ idx2_not_copied | idx_not_copied_again 
+-----------------+----------------------
+ t               | t
+(1 row)
+
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tbspace_subxact;
+ count 
+-------
+   301
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+ERROR:  abort after writing
+CONTEXT:  PL/pgSQL function tbspace_combined_chk(integer) line 5 at RAISE
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
 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..52cf5d5a09e 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -420,6 +420,108 @@ REINDEX (TABLESPACE regress_tblspace) TABLE 
tablespace_table; -- fail
 REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- 
fail
 RESET ROLE;
 
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+SELECT count(*) FROM tbspace_rollback;
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+SELECT count(*) FROM tbspace_subxact;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
+
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
 
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE 
pg_default;
-- 
2.55.0


From 33a6523c8a5e694281b09228584bad846ff01766 Mon Sep 17 00:00:00 2001
From: Manuel Reyes Bravo <manuelreyesbr...@gmail.com>
Date: Fri, 2 Oct 2026 10:47:53 -0300
Subject: [PATCH v6] 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.  A TRUNCATE after the move in the same
transaction truncates the old index files in place, which a rollback
cannot undo either.

Fix by giving each index of the table a new relfilenumber in the same
ALTER, copying it within its own tablespace (the indexes stay where they
are, as documented), so that an abort discards the new heap and index
files together.  Two cases need no copy:

- The ALTER is a plain ALTER TABLE ... SET TABLESPACE of a single
  table, with no other subcommand, issued as a top-level statement
  outside a transaction block, so nothing can modify the table before
  the transaction ends.  Any other subcommand could run user code after
  the move: the validation of an inheritance child's new CHECK
  constraint, a sql_drop event trigger, and so on.  This also requires
  that no ddl_command_end event trigger runs after it, and the commit is
  forced right after the statement, as for PreventInTransactionBlock, so
  that a later statement of an extended-protocol pipeline cannot share
  the transaction.

- The index's current file was created in the current subtransaction,
  for example an index created, or moved, earlier in it.  An abort
  discards that file together with the heap's new one.  This also lets
  a transaction move the indexes first and then the table without
  copying the indexes twice.

The regression test checks that the ALTER on its own leaves the index
files untouched; that combined with a CHECK constraint whose validation
on an inheritance child writes to the table after the move and fails,
it leaves no stale index entry; that in a transaction block a rollback
after inserting or truncating leaves the index file at its
pre-transaction size; that an index created or moved earlier in the
subtransaction is not copied; and that after a rollback to a savepoint,
an index created, reindexed or already copied before the savepoint
returns no row for the rolled-back values.

Bug: #19686
Reported-by: Alexander Lakhin
Suggested-by: Andres Freund
Reviewed-by: Alexandre Felipe
Reviewed-by: Shihao Zhong
---
 src/backend/commands/tablecmds.c         | 145 ++++++++++++++++++++-
 src/backend/tcop/utility.c               |   1 +
 src/include/tcop/utility.h               |   1 +
 src/test/regress/expected/tablespace.out | 153 +++++++++++++++++++++++
 src/test/regress/sql/tablespace.sql      | 102 +++++++++++++++
 5 files changed, 397 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3fd375feffc..3e2f8e54dda 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -92,6 +92,7 @@
 #include "tcop/utility.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
+#include "utils/evtcache.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
@@ -593,7 +594,11 @@ static void ATPrepSetAccessMethod(AlteredTableInfo *tab, 
Relation rel, const cha
 static bool ATPrepChangePersistence(Relation rel, bool toLogged);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                                                                const char 
*tablespacename, LOCKMODE lockmode);
-static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode);
+static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode,
+                                                               bool 
copyIndexes);
+static bool ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                                          
AlterTableUtilityContext *context);
+static void ATExecSetTableSpaceNewIndexRelfilenode(Oid indexOid, LOCKMODE 
lockmode);
 static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
 static void ATExecSetRelOptions(Relation rel, List *defList,
                                                                AlterTableType 
operation,
@@ -5670,7 +5675,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, 
LOCKMODE lockmode,
                         * just do a block-by-block copy.
                         */
                        if (tab->newTableSpace)
-                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode);
+                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode,
+                                                                       
ATSetTableSpaceCopyIndexes(parsetree, *wqueue,
+                                                                               
                                           context));
                }
 
                /*
@@ -14791,13 +14798,16 @@ 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)
+ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode,
+                                       bool copyIndexes)
 {
        Relation        rel;
        Oid                     reltoastrelid;
+       char            relkind;
        Oid                     newrelfilenode;
        RelFileNode newrnode;
        List       *reltoastidxids = NIL;
+       List       *reltabidxids = NIL;
        ListCell   *lc;
 
        /*
@@ -14815,6 +14825,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))
        {
@@ -14861,6 +14872,14 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode)
 
        RelationAssumeNewRelfilenode(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 relfilenode below.
+        */
+       if (copyIndexes &&
+               (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW))
+               reltabidxids = RelationGetIndexList(rel);
+
        relation_close(rel, NoLock);
 
        /* Make sure the reltablespace change is visible */
@@ -14868,12 +14887,128 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode)
 
        /* Move associated toast relation and/or indexes, too */
        if (OidIsValid(reltoastrelid))
-               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode);
+               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode, 
copyIndexes);
        foreach(lc, reltoastidxids)
-               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode);
+               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode, 
copyIndexes);
 
        /* 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.  See ATExecSetTableSpaceNewIndexRelfilenode.
+        */
+       foreach(lc, reltabidxids)
+               ATExecSetTableSpaceNewIndexRelfilenode(lfirst_oid(lc), 
lockmode);
+       list_free(reltabidxids);
+}
+
+/*
+ * Does ALTER TABLE SET TABLESPACE need to give the table's indexes new files?
+ *
+ * Only if something can still modify the table in the same transaction after
+ * the move.  Rather than listing every place where user code can run after
+ * it -- the validation of other tables in the work queue, such as an
+ * inheritance child getting a new CHECK constraint, foreign key validation,
+ * sql_drop and table_rewrite event triggers, and more -- the files are kept
+ * only for the one case where none of them exists: a plain
+ * "ALTER TABLE ... SET TABLESPACE" of a single table, with no other
+ * subcommand, issued as a top-level statement outside any transaction block.
+ * What can still run after it is a ddl_command_end event trigger, and a
+ * further statement of an extended-protocol pipeline, which would share the
+ * transaction until the next Sync.  Rule out the former by looking for such
+ * triggers, and the latter by forcing the commit right after the ALTER, as
+ * PreventInTransactionBlock does.
+ */
+static bool
+ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                  AlterTableUtilityContext 
*context)
+{
+       if (context == NULL || IsInTransactionBlock(context->isTopLevel))
+               return true;
+       if (parsetree == NULL || list_length(parsetree->cmds) != 1 ||
+               castNode(AlterTableCmd, linitial(parsetree->cmds))->subtype != 
AT_SetTableSpace ||
+               list_length(wqueue) != 1)
+               return true;
+       if (EventCacheLookup(EVT_DDLCommandEnd) != NIL)
+               return true;
+
+       MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT;
+       return false;
+}
+
+/*
+ * Give one of a table's indexes a fresh relfilenode within its existing
+ * tablespace, copying the current index file to the new relfilenode.
+ *
+ * ATExecSetTableSpace() calls this for each index of a table whose heap it has
+ * just rewritten to a new relfilenode.  The indexes must share the heap's
+ * transactional fate: an abort has to discard the index entries written during
+ * the transaction together with the heap's new file, since the heap TIDs those
+ * entries point at are freed by the abort and can be reused by later inserts.
+ * Copying the existing index file keeps the added cost close to that of the
+ * heap move itself; the index stays in its own tablespace, as documented.
+ */
+static void
+ATExecSetTableSpaceNewIndexRelfilenode(Oid indexOid, LOCKMODE lockmode)
+{
+       Relation        ind;
+       Oid                     newrelfilenode;
+       RelFileNode newrnode;
+       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.  An
+        * index whose current file was created in this subtransaction (the 
index
+        * itself, or a new file for it) is discarded together with the heap's 
new
+        * file on abort, so it needs no copy.  rd_newRelfilenodeSubid can be 
zero after
+        * a rollback to a savepoint even though the file is new; then this 
falls
+        * back to rd_createSubid, and copies if that does not match either.
+        */
+       if (ind->rd_rel->relkind != RELKIND_INDEX ||
+               !RELKIND_HAS_STORAGE(ind->rd_rel->relkind) ||
+               Max(ind->rd_newRelfilenodeSubid, ind->rd_createSubid) ==
+               GetCurrentSubTransactionId())
+       {
+               relation_close(ind, NoLock);
+               return;
+       }
+
+       /* Allocate a new relfilenode in the index's current tablespace. */
+       newrelfilenode = GetNewRelFileNode(ind->rd_rel->reltablespace, NULL,
+                                                                               
   ind->rd_rel->relpersistence);
+       newrnode = ind->rd_node;
+       newrnode.relNode = newrelfilenode;
+
+       /* Copy the index into the new file and schedule the old one for 
cleanup. */
+       index_copy_data(ind, newrnode);
+
+       /* 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 = newrelfilenode;
+       CatalogTupleUpdate(pg_class, &otid, tuple);
+       UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
+       heap_freetuple(tuple);
+       table_close(pg_class, RowExclusiveLock);
+
+       InvokeObjectPostAlterHook(RelationRelationId, indexOid, 0);
+       RelationAssumeNewRelfilenode(ind);
+
+       relation_close(ind, NoLock);
+
+       /* Make the relfilenode change visible. */
+       CommandCounterIncrement();
 }
 
 /*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index dbbe5904256..478fb5c0071 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1316,6 +1316,7 @@ ProcessUtilitySlow(ParseState *pstate,
                                                atcontext.relid = relid;
                                                atcontext.params = params;
                                                atcontext.queryEnv = queryEnv;
+                                               atcontext.isTopLevel = 
isTopLevel;
 
                                                /* ... ensure we have an event 
trigger context ... */
                                                
EventTriggerAlterTableStart(parsetree);
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index f9daf5b744c..3123d4aa8dd 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -34,6 +34,7 @@ typedef struct AlterTableUtilityContext
        Oid                     relid;                  /* OID of ALTER's 
target table */
        ParamListInfo params;           /* any parameters available to ALTER 
TABLE */
        QueryEnvironment *queryEnv; /* execution environment for ALTER TABLE */
+       bool            isTopLevel;             /* ALTER TABLE is a top-level 
statement */
 } AlterTableUtilityContext;
 
 /*
diff --git a/src/test/regress/expected/tablespace.out 
b/src/test/regress/expected/tablespace.out
index 3873e3cd7fc..1fd24491262 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -951,6 +951,159 @@ 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;
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+ idx_file_kept 
+---------------
+ t
+(1 row)
+
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+ idx_file_new 
+--------------
+ t
+(1 row)
+
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ idx_grew 
+----------
+ t
+(1 row)
+
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+SELECT count(*) FROM tbspace_rollback;
+ count 
+-------
+   100
+(1 row)
+
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ idx2_not_copied | idx_not_copied_again 
+-----------------+----------------------
+ t               | t
+(1 row)
+
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tbspace_subxact;
+ count 
+-------
+   301
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+ERROR:  abort after writing
+CONTEXT:  PL/pgSQL function tbspace_combined_chk(integer) line 5 at RAISE
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
 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 060698072e6..39669a5a14b 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -422,6 +422,108 @@ REINDEX (TABLESPACE regress_tblspace) TABLE 
tablespace_table; -- fail
 REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- 
fail
 RESET ROLE;
 
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+SELECT count(*) FROM tbspace_rollback;
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+SELECT count(*) FROM tbspace_subxact;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
+
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
 
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE 
pg_default;
-- 
2.55.0


From 6ae6c3233f5c259e864e024104f42fd7c5229a4b Mon Sep 17 00:00:00 2001
From: Manuel Reyes Bravo <manuelreyesbr...@gmail.com>
Date: Fri, 2 Oct 2026 10:50:40 -0300
Subject: [PATCH v6] 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.  A TRUNCATE after the move in the same
transaction truncates the old index files in place, which a rollback
cannot undo either.

Fix by giving each index of the table a new relfilenumber in the same
ALTER, copying it within its own tablespace (the indexes stay where they
are, as documented), so that an abort discards the new heap and index
files together.  Two cases need no copy:

- The ALTER is a plain ALTER TABLE ... SET TABLESPACE of a single
  table, with no other subcommand, issued as a top-level statement
  outside a transaction block, so nothing can modify the table before
  the transaction ends.  Any other subcommand could run user code after
  the move: the validation of an inheritance child's new CHECK
  constraint, a sql_drop event trigger, and so on.  This also requires
  that no ddl_command_end event trigger runs after it, and the commit is
  forced right after the statement, as for PreventInTransactionBlock, so
  that a later statement of an extended-protocol pipeline cannot share
  the transaction.

- The index's current file was created in the current subtransaction,
  for example an index created, or moved, earlier in it.  An abort
  discards that file together with the heap's new one.  This also lets
  a transaction move the indexes first and then the table without
  copying the indexes twice.

The regression test checks that the ALTER on its own leaves the index
files untouched; that combined with a CHECK constraint whose validation
on an inheritance child writes to the table after the move and fails,
it leaves no stale index entry; that in a transaction block a rollback
after inserting or truncating leaves the index file at its
pre-transaction size; that an index created or moved earlier in the
subtransaction is not copied; and that after a rollback to a savepoint,
an index created, reindexed or already copied before the savepoint
returns no row for the rolled-back values.

Bug: #19686
Reported-by: Alexander Lakhin
Suggested-by: Andres Freund
Reviewed-by: Alexandre Felipe
Reviewed-by: Shihao Zhong
---
 src/backend/commands/tablecmds.c          | 145 +++++++++++++++++++-
 src/backend/tcop/utility.c                |   1 +
 src/include/tcop/utility.h                |   1 +
 src/test/regress/input/tablespace.source  | 102 +++++++++++++++
 src/test/regress/output/tablespace.source | 153 ++++++++++++++++++++++
 5 files changed, 397 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e445b801a6b..dad0fb0b77a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -89,6 +89,7 @@
 #include "tcop/utility.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
+#include "utils/evtcache.h"
 #include "utils/fmgroids.h"
 #include "utils/inval.h"
 #include "utils/lsyscache.h"
@@ -549,7 +550,11 @@ static void ATExecDropCluster(Relation rel, LOCKMODE 
lockmode);
 static bool ATPrepChangePersistence(Relation rel, bool toLogged);
 static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
                                                                const char 
*tablespacename, LOCKMODE lockmode);
-static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode);
+static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode,
+                                                               bool 
copyIndexes);
+static bool ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                                          
AlterTableUtilityContext *context);
+static void ATExecSetTableSpaceNewIndexRelfilenode(Oid indexOid, LOCKMODE 
lockmode);
 static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
 static void ATExecSetRelOptions(Relation rel, List *defList,
                                                                AlterTableType 
operation,
@@ -5582,7 +5587,9 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, 
LOCKMODE lockmode,
                         * just do a block-by-block copy.
                         */
                        if (tab->newTableSpace)
-                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode);
+                               ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode,
+                                                                       
ATSetTableSpaceCopyIndexes(parsetree, *wqueue,
+                                                                               
                                           context));
                }
        }
 
@@ -14198,13 +14205,16 @@ 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)
+ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode,
+                                       bool copyIndexes)
 {
        Relation        rel;
        Oid                     reltoastrelid;
+       char            relkind;
        Oid                     newrelfilenode;
        RelFileNode newrnode;
        List       *reltoastidxids = NIL;
+       List       *reltabidxids = NIL;
        ListCell   *lc;
 
        /*
@@ -14222,6 +14232,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))
        {
@@ -14270,6 +14281,14 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode)
 
        RelationAssumeNewRelfilenode(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 relfilenode below.
+        */
+       if (copyIndexes &&
+               (relkind == RELKIND_RELATION || relkind == RELKIND_MATVIEW))
+               reltabidxids = RelationGetIndexList(rel);
+
        relation_close(rel, NoLock);
 
        /* Make sure the reltablespace change is visible */
@@ -14277,12 +14296,128 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode)
 
        /* Move associated toast relation and/or indexes, too */
        if (OidIsValid(reltoastrelid))
-               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode);
+               ATExecSetTableSpace(reltoastrelid, newTableSpace, lockmode, 
copyIndexes);
        foreach(lc, reltoastidxids)
-               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode);
+               ATExecSetTableSpace(lfirst_oid(lc), newTableSpace, lockmode, 
copyIndexes);
 
        /* 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.  See ATExecSetTableSpaceNewIndexRelfilenode.
+        */
+       foreach(lc, reltabidxids)
+               ATExecSetTableSpaceNewIndexRelfilenode(lfirst_oid(lc), 
lockmode);
+       list_free(reltabidxids);
+}
+
+/*
+ * Does ALTER TABLE SET TABLESPACE need to give the table's indexes new files?
+ *
+ * Only if something can still modify the table in the same transaction after
+ * the move.  Rather than listing every place where user code can run after
+ * it -- the validation of other tables in the work queue, such as an
+ * inheritance child getting a new CHECK constraint, foreign key validation,
+ * sql_drop and table_rewrite event triggers, and more -- the files are kept
+ * only for the one case where none of them exists: a plain
+ * "ALTER TABLE ... SET TABLESPACE" of a single table, with no other
+ * subcommand, issued as a top-level statement outside any transaction block.
+ * What can still run after it is a ddl_command_end event trigger, and a
+ * further statement of an extended-protocol pipeline, which would share the
+ * transaction until the next Sync.  Rule out the former by looking for such
+ * triggers, and the latter by forcing the commit right after the ALTER, as
+ * PreventInTransactionBlock does.
+ */
+static bool
+ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                  AlterTableUtilityContext 
*context)
+{
+       if (context == NULL || IsInTransactionBlock(context->isTopLevel))
+               return true;
+       if (parsetree == NULL || list_length(parsetree->cmds) != 1 ||
+               castNode(AlterTableCmd, linitial(parsetree->cmds))->subtype != 
AT_SetTableSpace ||
+               list_length(wqueue) != 1)
+               return true;
+       if (EventCacheLookup(EVT_DDLCommandEnd) != NIL)
+               return true;
+
+       MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT;
+       return false;
+}
+
+/*
+ * Give one of a table's indexes a fresh relfilenode within its existing
+ * tablespace, copying the current index file to the new relfilenode.
+ *
+ * ATExecSetTableSpace() calls this for each index of a table whose heap it has
+ * just rewritten to a new relfilenode.  The indexes must share the heap's
+ * transactional fate: an abort has to discard the index entries written during
+ * the transaction together with the heap's new file, since the heap TIDs those
+ * entries point at are freed by the abort and can be reused by later inserts.
+ * Copying the existing index file keeps the added cost close to that of the
+ * heap move itself; the index stays in its own tablespace, as documented.
+ */
+static void
+ATExecSetTableSpaceNewIndexRelfilenode(Oid indexOid, LOCKMODE lockmode)
+{
+       Relation        ind;
+       Oid                     newrelfilenode;
+       RelFileNode newrnode;
+       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.  An
+        * index whose current file was created in this subtransaction (the 
index
+        * itself, or a new file for it) is discarded together with the heap's 
new
+        * file on abort, so it needs no copy.  rd_newRelfilenodeSubid can be 
zero after
+        * a rollback to a savepoint even though the file is new; then this 
falls
+        * back to rd_createSubid, and copies if that does not match either.
+        */
+       if (ind->rd_rel->relkind != RELKIND_INDEX ||
+               !RELKIND_HAS_STORAGE(ind->rd_rel->relkind) ||
+               Max(ind->rd_newRelfilenodeSubid, ind->rd_createSubid) ==
+               GetCurrentSubTransactionId())
+       {
+               relation_close(ind, NoLock);
+               return;
+       }
+
+       /* Allocate a new relfilenode in the index's current tablespace. */
+       newrelfilenode = GetNewRelFileNode(ind->rd_rel->reltablespace, NULL,
+                                                                               
   ind->rd_rel->relpersistence);
+       newrnode = ind->rd_node;
+       newrnode.relNode = newrelfilenode;
+
+       /* Copy the index into the new file and schedule the old one for 
cleanup. */
+       index_copy_data(ind, newrnode);
+
+       /* 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 = newrelfilenode;
+       CatalogTupleUpdate(pg_class, &otid, tuple);
+       UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
+       heap_freetuple(tuple);
+       table_close(pg_class, RowExclusiveLock);
+
+       InvokeObjectPostAlterHook(RelationRelationId, indexOid, 0);
+       RelationAssumeNewRelfilenode(ind);
+
+       relation_close(ind, NoLock);
+
+       /* Make the relfilenode change visible. */
+       CommandCounterIncrement();
 }
 
 /*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 51509efbfc1..cbfd800ad35 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1308,6 +1308,7 @@ ProcessUtilitySlow(ParseState *pstate,
                                                atcontext.relid = relid;
                                                atcontext.params = params;
                                                atcontext.queryEnv = queryEnv;
+                                               atcontext.isTopLevel = 
isTopLevel;
 
                                                /* ... ensure we have an event 
trigger context ... */
                                                
EventTriggerAlterTableStart(parsetree);
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index 212e9b32806..7cd9a891d2b 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -34,6 +34,7 @@ typedef struct AlterTableUtilityContext
        Oid                     relid;                  /* OID of ALTER's 
target table */
        ParamListInfo params;           /* any parameters available to ALTER 
TABLE */
        QueryEnvironment *queryEnv; /* execution environment for ALTER TABLE */
+       bool            isTopLevel;             /* ALTER TABLE is a top-level 
statement */
 } AlterTableUtilityContext;
 
 /*
diff --git a/src/test/regress/input/tablespace.source 
b/src/test/regress/input/tablespace.source
index fd003d805e7..a6d699f4b65 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -400,6 +400,108 @@ REINDEX (TABLESPACE regress_tblspace) TABLE 
tablespace_table; -- fail
 REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE tablespace_table; -- 
fail
 RESET ROLE;
 
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+SELECT count(*) FROM tbspace_rollback;
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+SELECT count(*) FROM tbspace_subxact;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
+
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
 
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE 
pg_default;
diff --git a/src/test/regress/output/tablespace.source 
b/src/test/regress/output/tablespace.source
index 1b60b99ff3e..5369b98c8de 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -921,6 +921,159 @@ 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;
+-- ALTER TABLE SET TABLESPACE moves the heap and leaves the indexes in their
+-- tablespace.  Run on its own, as here, it leaves their files alone too.
+CREATE TABLE tbspace_rollback (a int);
+INSERT INTO tbspace_rollback SELECT generate_series(1, 100);
+CREATE INDEX tbspace_rollback_idx ON tbspace_rollback (a);
+SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS 
idx_file_kept;
+ idx_file_kept 
+---------------
+ t
+(1 row)
+
+-- In a transaction block the indexes get new files, so that a rollback
+-- discards the entries added after the move together with the heap's file.
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS 
idx_file_new;
+ idx_file_new 
+--------------
+ t
+(1 row)
+
+INSERT INTO tbspace_rollback SELECT generate_series(101, 2000);
+SELECT pg_relation_size('tbspace_rollback_idx') > :idx_size_before AS idx_grew;
+ idx_grew 
+----------
+ t
+(1 row)
+
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+-- the same for a TRUNCATE after the move, which is done in place
+BEGIN;
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+TRUNCATE tbspace_rollback;
+ROLLBACK;
+SELECT pg_relation_size('tbspace_rollback_idx') = :idx_size_before AS 
idx_size_restored;
+ idx_size_restored 
+-------------------
+ t
+(1 row)
+
+SELECT count(*) FROM tbspace_rollback;
+ count 
+-------
+   100
+(1 row)
+
+-- An index created in the same subtransaction is discarded with the heap
+-- anyway and is not copied.  Moving the indexes first and then the table
+-- copies each index only once.
+BEGIN;
+CREATE INDEX tbspace_rollback_idx2 ON tbspace_rollback (a);
+SELECT pg_relation_filepath('tbspace_rollback_idx2') AS idx2_path \gset
+ALTER INDEX tbspace_rollback_idx SET TABLESPACE regress_tblspace;
+SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_moved_path \gset
+ALTER TABLE tbspace_rollback SET TABLESPACE pg_default;
+SELECT pg_relation_filepath('tbspace_rollback_idx2') = :'idx2_path' AS 
idx2_not_copied,
+  pg_relation_filepath('tbspace_rollback_idx') = :'idx_moved_path' AS 
idx_not_copied_again;
+ idx2_not_copied | idx_not_copied_again 
+-----------------+----------------------
+ t               | t
+(1 row)
+
+ROLLBACK;
+DROP TABLE tbspace_rollback;
+-- An index file that is new in this transaction but older than a savepoint
+-- survives a rollback to it, so it must still follow a move made after the
+-- savepoint: an index created or reindexed before it, or already copied by
+-- an earlier move.  Rows inserted afterwards reuse the TIDs freed by the
+-- rollbacks; a stale index entry would match them.
+CREATE TABLE tbspace_subxact (a int);
+INSERT INTO tbspace_subxact SELECT generate_series(1, 100);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+BEGIN;
+CREATE INDEX tbspace_subxact_idx ON tbspace_subxact (a);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+REINDEX INDEX tbspace_subxact_idx;
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+BEGIN;
+ALTER TABLE tbspace_subxact SET TABLESPACE regress_tblspace;
+INSERT INTO tbspace_subxact VALUES (0);
+SAVEPOINT s;
+ALTER TABLE tbspace_subxact SET TABLESPACE pg_default;
+INSERT INTO tbspace_subxact SELECT -generate_series(1, 50);
+ROLLBACK TO s;
+COMMIT;
+INSERT INTO tbspace_subxact SELECT generate_series(101, 300);
+SELECT count(*) FROM tbspace_subxact WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM tbspace_subxact;
+ count 
+-------
+   301
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+ERROR:  abort after writing
+CONTEXT:  PL/pgSQL function tbspace_combined_chk(integer) line 5 at RAISE
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
 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;
-- 
2.55.0


diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dcfbb362c73..a59581cad9a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -703,7 +703,8 @@ static void ATPrepSetTableSpace(AlteredTableInfo *tab, 
Relation rel,
                                                                const char 
*tablespacename, LOCKMODE lockmode);
 static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE 
lockmode,
                                                                bool 
copyIndexes);
-static bool ATSetTableSpaceCopyIndexes(AlterTableUtilityContext *context);
+static bool ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                                          
AlterTableUtilityContext *context);
 static void ATExecSetTableSpaceNewIndexRelfilenumber(Oid indexOid, LOCKMODE 
lockmode);
 static void ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace);
 static void ATExecSetRelOptions(Relation rel, List *defList,
@@ -6102,7 +6103,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, 
LOCKMODE lockmode,
                         */
                        if (tab->newTableSpace)
                                ATExecSetTableSpace(tab->relid, 
tab->newTableSpace, lockmode,
-                                                                       
ATSetTableSpaceCopyIndexes(context));
+                                                                       
ATSetTableSpaceCopyIndexes(parsetree, *wqueue,
+                                                                               
                                           context));
                }
 
                /*
@@ -17641,18 +17643,29 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, 
LOCKMODE lockmode,
  * Does ALTER TABLE SET TABLESPACE need to give the table's indexes new files?
  *
  * Only if something can still modify the table in the same transaction after
- * the move.  Nothing can when the ALTER is a top-level statement outside any
- * transaction block and nothing runs after it before the commit: no
- * ddl_command_end event trigger, and no further statement of an
- * extended-protocol pipeline, which would otherwise share the transaction
- * until the next Sync.  To rule out the latter, force the commit right after
- * the ALTER, as PreventInTransactionBlock does.
+ * the move.  Rather than listing every place where user code can run after
+ * it -- the validation of other tables in the work queue, such as an
+ * inheritance child getting a new CHECK constraint, foreign key validation,
+ * sql_drop and table_rewrite event triggers, and more -- the files are kept
+ * only for the one case where none of them exists: a plain
+ * "ALTER TABLE ... SET TABLESPACE" of a single table, with no other
+ * subcommand, issued as a top-level statement outside any transaction block.
+ * What can still run after it is a ddl_command_end event trigger, and a
+ * further statement of an extended-protocol pipeline, which would share the
+ * transaction until the next Sync.  Rule out the former by looking for such
+ * triggers, and the latter by forcing the commit right after the ALTER, as
+ * PreventInTransactionBlock does.
  */
 static bool
-ATSetTableSpaceCopyIndexes(AlterTableUtilityContext *context)
+ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue,
+                                                  AlterTableUtilityContext 
*context)
 {
        if (context == NULL || IsInTransactionBlock(context->isTopLevel))
                return true;
+       if (parsetree == NULL || list_length(parsetree->cmds) != 1 ||
+               castNode(AlterTableCmd, linitial(parsetree->cmds))->subtype != 
AT_SetTableSpace ||
+               list_length(wqueue) != 1)
+               return true;
        if (EventCacheLookup(EVT_DDLCommandEnd) != NIL)
                return true;
 
diff --git a/src/test/regress/expected/tablespace.out 
b/src/test/regress/expected/tablespace.out
index ccc43061aad..aec469aee9f 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -1071,6 +1071,39 @@ SELECT count(*) FROM tbspace_subxact;
 RESET enable_seqscan;
 RESET enable_bitmapscan;
 DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+ERROR:  abort after writing
+CONTEXT:  PL/pgSQL function tbspace_combined_chk(integer) line 5 at RAISE
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+ count 
+-------
+     0
+(1 row)
+
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
 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 8fc94894491..52cf5d5a09e 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -495,6 +495,32 @@ SELECT count(*) FROM tbspace_subxact;
 RESET enable_seqscan;
 RESET enable_bitmapscan;
 DROP TABLE tbspace_subxact;
+-- Combined with another subcommand, the ALTER can run user code after the
+-- move, such as a CHECK constraint validated on an inheritance child after
+-- the parent was moved, so the indexes get new files even outside a
+-- transaction block.
+CREATE TABLE tbspace_combined (a int);
+INSERT INTO tbspace_combined SELECT generate_series(1, 100);
+CREATE INDEX tbspace_combined_idx ON tbspace_combined (a);
+CREATE TABLE tbspace_combined_child () INHERITS (tbspace_combined);
+INSERT INTO tbspace_combined_child VALUES (5000);
+CREATE FUNCTION tbspace_combined_chk(v int) RETURNS bool LANGUAGE plpgsql AS $$
+BEGIN
+  IF v < 5000 THEN RETURN true; END IF;
+  INSERT INTO tbspace_combined SELECT -generate_series(1, 50);
+  RAISE EXCEPTION 'abort after writing';
+END $$;
+ALTER TABLE tbspace_combined SET TABLESPACE regress_tblspace,
+  ADD CONSTRAINT tbspace_combined_k CHECK (tbspace_combined_chk(a));
+DROP TABLE tbspace_combined_child;
+INSERT INTO tbspace_combined SELECT generate_series(101, 300);
+SET enable_seqscan = off;
+SET enable_bitmapscan = off;
+SELECT count(*) FROM tbspace_combined WHERE a < 0;
+RESET enable_seqscan;
+RESET enable_bitmapscan;
+DROP TABLE tbspace_combined;
+DROP FUNCTION tbspace_combined_chk(int);
 
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;