From bcc70ec04556c8570de7db1d3b122ac88711f83b Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 28 Jul 2026 13:47:36 +0800 Subject: [PATCH v10 1/2] tablecmds: preserve partition index names during ALTER TYPE ALTER TABLE ... ALTER COLUMN ... TYPE rebuilds dependent partitioned indexes. When DefineIndex() recursively recreated the partition indexes, generateClonedIndexStmt() left their names unset, causing default names to be generated. As a result, custom names of attached partition indexes were not preserved. Before dropping the index hierarchy, remember each partition index name together with its table OID. Pass the saved name to generateClonedIndexStmt() when recursively recreating the corresponding partition index. Other callers continue to let DefineIndex() choose a name. Reported-by: Sami Imseih Author: Chao Li Reviewed-by: Sami Imseih Discussion: https://postgr.es/m/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com --- src/backend/commands/indexcmds.c | 29 ++++++++++++++- src/backend/commands/tablecmds.c | 44 +++++++++++++++++++++-- src/backend/parser/parse_utilcmd.c | 18 +++++----- src/include/nodes/parsenodes.h | 2 ++ src/include/parser/parse_utilcmd.h | 3 +- src/test/regress/expected/alter_table.out | 16 +++++++++ src/test/regress/sql/alter_table.sql | 12 +++++++ 7 files changed, 112 insertions(+), 12 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 3790b8e1252..d9e5d74bebf 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -109,6 +109,8 @@ static List *ChooseIndexColumnNames(Relation rel, const List *indexElems); static char *ChooseIndexExpressionName(Relation rel, Node *indexExpr); static bool ChooseIndexExpressionName_walker(Node *node, CIEN_context *context); +static const char *GetRebuildPartitionIndexName(const IndexStmt *stmt, + Oid relid); static void ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel); static void RangeVarCallbackForReindexIndex(const RangeVar *relation, @@ -1517,6 +1519,7 @@ DefineIndex(ParseState *pstate, { IndexStmt *childStmt; ObjectAddress childAddr; + const char *childIndexName; /* * Build an IndexStmt describing the desired child index @@ -1525,10 +1528,15 @@ DefineIndex(ParseState *pstate, * a search-path-independent representation, which the * original IndexStmt might not be. */ + childIndexName = + GetRebuildPartitionIndexName(stmt, childRelid); childStmt = generateClonedIndexStmt(NULL, parentIndex, attmap, - NULL); + NULL, + childIndexName); + childStmt->oldPartIndexRelids = stmt->oldPartIndexRelids; + childStmt->oldPartIndexNames = stmt->oldPartIndexNames; /* * Recurse as the starting user ID. Callee will use that @@ -2979,6 +2987,25 @@ ChooseIndexExpressionName_walker(Node *node, context); } +/* + * Find the old name of a partition index that is being rebuilt. + */ +static const char * +GetRebuildPartitionIndexName(const IndexStmt *stmt, Oid relid) +{ + ListCell *relid_item; + ListCell *name_item; + + forboth(relid_item, stmt->oldPartIndexRelids, + name_item, stmt->oldPartIndexNames) + { + if (lfirst_oid(relid_item) == relid) + return strVal(lfirst(name_item)); + } + + return NULL; +} + /* * ExecReindex * diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6d4c457b820..bbb42e3c3b0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -705,6 +705,8 @@ static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, static void RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, Oid objid, Relation rel, List *domname, const char *conname); +static void RememberPartitionIndexNamesForRebuilding(Oid indoid, + IndexStmt *stmt); static void TryReuseIndex(Oid oldId, IndexStmt *stmt); static void TryReuseForeignKey(Oid oldId, Constraint *con); static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName, @@ -1353,7 +1355,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, false); idxstmt = generateClonedIndexStmt(NULL, idxRel, - attmap, &constraintOid); + attmap, &constraintOid, + NULL); DefineIndex(NULL, RelationGetRelid(rel), idxstmt, @@ -16402,6 +16405,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, IndexStmt *stmt = (IndexStmt *) stm; AlterTableCmd *newcmd; + RememberPartitionIndexNamesForRebuilding(oldId, stmt); if (!rewrite) TryReuseIndex(oldId, stmt); stmt->reset_default_tblspc = true; @@ -16431,6 +16435,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, indstmt = castNode(IndexStmt, cmd->def); indoid = get_constraint_index(oldId); + RememberPartitionIndexNamesForRebuilding(indoid, indstmt); if (!rewrite) TryReuseIndex(indoid, indstmt); /* keep any comment on the index */ @@ -16583,6 +16588,40 @@ RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, Oid objid, tab->subcmds[pass] = lappend(tab->subcmds[pass], newcmd); } +/* + * Save the names of child indexes before dropping a partitioned index. + * DefineIndex() will use them when it recursively rebuilds the hierarchy. + */ +static void +RememberPartitionIndexNamesForRebuilding(Oid indoid, IndexStmt *stmt) +{ + List *indexOids; + + if (get_rel_relkind(indoid) != RELKIND_PARTITIONED_INDEX) + return; + + /* + * ALTER TABLE has already locked all the partition tables, so the index + * hierarchy cannot change underneath us. + */ + indexOids = find_all_inheritors(indoid, NoLock, NULL); + foreach_oid(indexOid, indexOids) + { + Oid relid; + + if (indexOid == indoid) + continue; + + relid = IndexGetRelation(indexOid, false); + stmt->oldPartIndexRelids = + lappend_oid(stmt->oldPartIndexRelids, relid); + stmt->oldPartIndexNames = + lappend(stmt->oldPartIndexNames, + makeString(get_rel_name(indexOid))); + } + list_free(indexOids); +} + /* * Subroutine for ATPostAlterTypeParse(). Calls out to CheckIndexCompatible() * for the real analysis, then mutates the IndexStmt based on that verdict. @@ -21513,7 +21552,8 @@ AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel) stmt = generateClonedIndexStmt(NULL, idxRel, attmap, - &conOid); + &conOid, + NULL); DefineIndex(NULL, RelationGetRelid(attachrel), stmt, InvalidOid, RelationGetRelid(idxRel), diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index ccf6ee55310..8ce6b6a5458 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -1562,6 +1562,7 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) index_stmt = generateClonedIndexStmt(heapRel, parent_index, attmap, + NULL, NULL); /* Copy comment on index, if requested */ @@ -1681,6 +1682,12 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename) * If constraintOid isn't NULL, we store the OID of any constraint associated * with the index there. * + * If indexName isn't NULL, use it as the name of the cloned index; otherwise, + * let DefineIndex() choose a name. Usually the source index name cannot be + * used for a clone because it would conflict in the same schema. A caller + * rebuilding a previously existing index can provide the name that should be + * preserved. + * * Unlike transformIndexConstraint, we don't make any effort to force primary * key columns to be not-null. The larger cloning process this is part of * should have cloned their not-null status separately (and DefineIndex will @@ -1689,7 +1696,8 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename) IndexStmt * generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx, const AttrMap *attmap, - Oid *constraintOid) + Oid *constraintOid, + const char *indexName) { Oid source_relid = RelationGetRelid(source_idx); HeapTuple ht_idxrel; @@ -1765,13 +1773,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx, index->if_not_exists = false; index->reset_default_tblspc = false; - /* - * We don't try to preserve the name of the source index; instead, just - * let DefineIndex() choose a reasonable name. (If we tried to preserve - * the name, we'd get duplicate-relation-name failures unless the source - * table was in a different schema.) - */ - index->idxname = NULL; + index->idxname = indexName ? pstrdup(indexName) : NULL; /* * If the index is marked PRIMARY or has an exclusion condition, it's diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 5c8f9a07b62..ff7f23098d9 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3646,6 +3646,8 @@ typedef struct IndexStmt SubTransactionId oldCreateSubid; /* rd_createSubid of oldNumber */ SubTransactionId oldFirstRelfilelocatorSubid; /* rd_firstRelfilelocatorSubid * of oldNumber */ + List *oldPartIndexRelids; /* partition relids during index rebuild */ + List *oldPartIndexNames; /* partition index names during rebuild */ bool unique; /* is index unique? */ bool nulls_not_distinct; /* null treatment for UNIQUE constraints */ bool primary; /* is index a primary key? */ diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h index 34c98e5122f..da6f5835b5f 100644 --- a/src/include/parser/parse_utilcmd.h +++ b/src/include/parser/parse_utilcmd.h @@ -40,6 +40,7 @@ extern List *expandTableLikeClause(RangeVar *heapRel, extern IndexStmt *generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx, const AttrMap *attmap, - Oid *constraintOid); + Oid *constraintOid, + const char *indexName); #endif /* PARSE_UTILCMD_H */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index e167a41ce79..44d08bc4d0e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4836,6 +4836,22 @@ select indexrelid::regclass, indisclustered from pg_index (2 rows) drop table alttype_cluster; +-- A partition index keeps its name when rebuilt through the parent. +create table alttype_part (a int not null, b int) partition by range (a); +create table alttype_part_child (a int not null, b int); +create index alttype_part_child_custom_idx on alttype_part_child (a, b); +create index alttype_part_idx on alttype_part (a, b); +alter table alttype_part attach partition alttype_part_child + for values from (0) to (10); +alter table alttype_part alter column b type bigint; +select indexrelid::regclass + from pg_index where indrelid = 'alttype_part_child'::regclass; + indexrelid +------------------------------- + alttype_part_child_custom_idx +(1 row) + +drop table alttype_part; -- -- Check that attaching or detaching a partitioned partition correctly leads -- to its partitions' constraint being updated to reflect the parent's diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index f5f13bbd3e7..48f5f0b8bbc 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3128,6 +3128,18 @@ select indexrelid::regclass, indisclustered from pg_index order by indexrelid::regclass::text; drop table alttype_cluster; +-- A partition index keeps its name when rebuilt through the parent. +create table alttype_part (a int not null, b int) partition by range (a); +create table alttype_part_child (a int not null, b int); +create index alttype_part_child_custom_idx on alttype_part_child (a, b); +create index alttype_part_idx on alttype_part (a, b); +alter table alttype_part attach partition alttype_part_child + for values from (0) to (10); +alter table alttype_part alter column b type bigint; +select indexrelid::regclass + from pg_index where indrelid = 'alttype_part_child'::regclass; +drop table alttype_part; + -- -- Check that attaching or detaching a partitioned partition correctly leads -- to its partitions' constraint being updated to reflect the parent's -- 2.50.1 (Apple Git-155)