From 262ab55900577fc64bdeb56d657e79e1214fb196 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?=
 <alvarorodriguez.garcia@datadoghq.com>
Date: Thu, 14 May 2026 15:51:26 +0200
Subject: [PATCH v1 2/3] Skip index rewriting when possible on ALTER TABLE
 ALTER COLUMN TYPE for partitioned tables

During ALTER TABLE ALTER COLUMN TYPE for a partitioned table with an index, the old deletion behavior:
1. Processes the parent table in ATRewriteCatalogs before processing any of its child tables
2. Runs ATPostAlterTypeCleanup for the table, which drops the partitioned index at the end. That
drop cascades to the child indexes.
3. Processes the child tables in ATRewriteCatalogs. However, the child indexes were already dropped,
so we never remember them in RememberAllDependentForRebuilding.
4. When indexes are eventually recreated, they always need to be reindexed.

This change batches the deletions in ATRewriteCatalogs so that they happen at the end of a processing
step and not in each call to ATPostAlterTypeCleanup (which happens for each processed table). This
ensures that child indexes are tracked when ATExecAlterColumnType calls RememberAllDependentForRebuilding
for each child table.

After the change, child indexes are now recreated twice: once when the parent index is created, and once
by themselves. We need for the child indexes to be created first. That way, they use the remembered
information to decide whether a reindex is needed. When the parent index is created, it will automatically
use existing child indexes. To enforce this behavior, we add a new AT_PASS_OLD_PARTITIONED_INDEX that
recreates the parent indexes.
---
 src/backend/commands/tablecmds.c          | 79 +++++++++++++++--------
 src/test/regress/expected/alter_table.out |  7 +-
 src/test/regress/sql/alter_table.sql      |  3 +-
 3 files changed, 55 insertions(+), 34 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d8d7969bf30..9bd2936476f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -157,6 +157,7 @@ typedef enum AlterTablePass
 	AT_PASS_ADD_COL,			/* ADD COLUMN */
 	AT_PASS_SET_EXPRESSION,		/* ALTER SET EXPRESSION */
 	AT_PASS_OLD_INDEX,			/* re-add existing indexes */
+	AT_PASS_OLD_PARTITIONED_INDEX,	/* re-add existing partitioned indexes */
 	AT_PASS_OLD_CONSTR,			/* re-add existing constraints */
 	/* We could support a RENAME COLUMN pass here, but not currently used */
 	AT_PASS_ADD_CONSTR,			/* ADD constraints (initial examination) */
@@ -689,8 +690,7 @@ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableT
 static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
 static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
 static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab);
-static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
-								   LOCKMODE lockmode);
+static void ATPostAlterTypeCleanup(List **wqueue, ObjectAddresses *objects, AlteredTableInfo *tab, LOCKMODE lockmode);
 static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
 								 char *cmd, List **wqueue, LOCKMODE lockmode,
 								 bool rewrite);
@@ -5381,6 +5381,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
 	 */
 	for (AlterTablePass pass = 0; pass < AT_NUM_PASSES; pass++)
 	{
+		ObjectAddresses *todelete = new_object_addresses();
+
 		/* Go through each table that needs to be processed */
 		foreach(ltab, *wqueue)
 		{
@@ -5406,10 +5408,14 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
 			/*
 			 * After the ALTER TYPE or SET EXPRESSION pass, do cleanup work
 			 * (this is not done in ATExecAlterColumnType since it should be
-			 * done only once if multiple columns of a table are altered).
+			 * done only once if multiple columns of a table are altered). The
+			 * objects pending deletion will be batched in the todelete list,
+			 * to be deleted at the end of the pass. Otherwise, we will drop
+			 * dependent objects before they are properly procesed. This is a
+			 * problem when we try to avoid rewrites for child indexes.
 			 */
 			if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION)
-				ATPostAlterTypeCleanup(wqueue, tab, lockmode);
+				ATPostAlterTypeCleanup(wqueue, todelete, tab, lockmode);
 
 			if (tab->rel)
 			{
@@ -5417,6 +5423,20 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
 				tab->rel = NULL;
 			}
 		}
+
+		/*
+		 * After the full pass, we drop all the objects that we accumulated,
+		 * then free the address list. It should be okay to use DROP_RESTRICT
+		 * here, since nothing else should be depending on these objects.
+		 */
+		performMultipleDeletions(todelete, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+
+		free_object_addresses(todelete);
+
+		/*
+		 * The objects will get recreated during subsequent passes over the
+		 * work queue.
+		 */
 	}
 
 	/* Check to see if a toast table must be added. */
@@ -15700,25 +15720,18 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab)
 /*
  * Cleanup after we've finished all the ALTER TYPE or SET EXPRESSION
  * operations for a particular relation.  We have to drop and recreate all the
- * indexes and constraints that depend on the altered columns.  We do the
- * actual dropping here, but re-creation is managed by adding work queue
- * entries to do those steps later.
+ * indexes and constraints that depend on the altered columns. We record the 
+ * objects that need to be deleted here, so they can be dropped by the caller 
+ * in one go after this method has been called for each table. The re-creation,
+ * however, is managed by adding work queue entries to do those steps later.
  */
 static void
-ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
+ATPostAlterTypeCleanup(List **wqueue, ObjectAddresses *objects, AlteredTableInfo *tab, LOCKMODE lockmode)
 {
 	ObjectAddress obj;
-	ObjectAddresses *objects;
 	ListCell   *def_item;
 	ListCell   *oid_item;
 
-	/*
-	 * Collect all the constraints and indexes to drop so we can process them
-	 * in a single call.  That way we don't have to worry about dependencies
-	 * among them.
-	 */
-	objects = new_object_addresses();
-
 	/*
 	 * Re-parse the index and constraint definitions, and attach them to the
 	 * appropriate work queue entries.  We do this before dropping because in
@@ -15875,16 +15888,10 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
 	}
 
 	/*
-	 * It should be okay to use DROP_RESTRICT here, since nothing else should
-	 * be depending on these objects.
-	 */
-	performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
-
-	free_object_addresses(objects);
-
-	/*
-	 * The objects will get recreated during subsequent passes over the work
-	 * queue.
+	 * The caller will take care of deleting the objects. We need to batch
+	 * the deletions together instead of dropping here. Otherwise, child
+	 * indexes will be dropped when dropping their parent partitioned index,
+	 * which means they will never be tracked for rebuilding.
 	 */
 }
 
@@ -15968,6 +15975,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
 		{
 			IndexStmt  *stmt = (IndexStmt *) stm;
 			AlterTableCmd *newcmd;
+			char		relkind = get_rel_relkind(oldId);
 
 			if (!rewrite)
 				TryReuseIndex(oldId, stmt);
@@ -15978,8 +15986,23 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
 			newcmd = makeNode(AlterTableCmd);
 			newcmd->subtype = AT_ReAddIndex;
 			newcmd->def = (Node *) stmt;
-			tab->subcmds[AT_PASS_OLD_INDEX] =
-				lappend(tab->subcmds[AT_PASS_OLD_INDEX], newcmd);
+
+			/*
+			 * We process partitioned indexes at a later stage. This ensures
+			 * that we won't create duplicate indexes. We rely on postgres 
+			 * automatically attaching existing child indexes to a new 
+			 * partitioned index. 
+			 * 
+			 * We could choose to skip child indexes and let them be recreated
+			 * by the parent, but that would force a reindex that we can
+			 * potentially avoid. 
+			 */
+			if (relkind == RELKIND_PARTITIONED_INDEX)
+				tab->subcmds[AT_PASS_OLD_PARTITIONED_INDEX] =
+					lappend(tab->subcmds[AT_PASS_OLD_PARTITIONED_INDEX], newcmd);
+			else
+				tab->subcmds[AT_PASS_OLD_INDEX] =
+					lappend(tab->subcmds[AT_PASS_OLD_INDEX], newcmd);
 		}
 		else if (IsA(stm, AlterTableStmt))
 		{
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index b43392316bb..40beb57c08f 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2327,8 +2327,7 @@ select conname, obj_description(oid, 'pg_constraint') as desc
 -- Don't remove this DROP, it exposes bug #15672
 drop table at_partitioned;
 -- Alter column type when no index rewrite is required
--- for partitioned tables. Reindex is required although
--- it shouldn't be.
+-- for partitioned tables.
 create table at_idx_part (id int, code varchar(10),
                           primary key (id, code))
   partition by range (id);
@@ -2358,10 +2357,10 @@ select relname,
  at_idx_part             | t        | none
  at_idx_part_code_idx    | f        | none
  at_idx_part_p1          | t        | own
- at_idx_part_p1_code_idx | f        | own
+ at_idx_part_p1_code_idx | f        | orig
  at_idx_part_p1_pkey     | f        | own
  at_idx_part_p2          | t        | own
- at_idx_part_p2_code_idx | f        | own
+ at_idx_part_p2_code_idx | f        | orig
  at_idx_part_p2_pkey     | f        | own
  at_idx_part_pkey        | f        | none
 (9 rows)
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 54dc9a85b8b..c78598308e4 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1530,8 +1530,7 @@ select conname, obj_description(oid, 'pg_constraint') as desc
 drop table at_partitioned;
 
 -- Alter column type when no index rewrite is required
--- for partitioned tables. Reindex is required although
--- it shouldn't be.
+-- for partitioned tables.
 create table at_idx_part (id int, code varchar(10),
                           primary key (id, code))
   partition by range (id);
-- 
2.52.0

