From e7769b6ea5aa7d8f4d1d4ae1ec288f6fdce92324 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Tue, 28 Jul 2026 13:47:36 +0800 Subject: [PATCH v12 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 | 19 ++--- src/include/nodes/parsenodes.h | 4 ++ src/include/parser/parse_utilcmd.h | 3 +- src/test/regress/expected/alter_table.out | 84 +++++++++++++++++++++++ src/test/regress/sql/alter_table.sql | 67 ++++++++++++++++++ 7 files changed, 238 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..fab8e820a6e 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,13 @@ 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. Most callers pass NULL because using the + * source index name would cause a duplicate-name failure when both indexes + * are in the same schema. Callers rebuilding a partition index (such as + * ALTER COLUMN TYPE) can pass the old name because the old index has already + * been dropped, making the name available in the target namespace. + * * 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 +1697,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 +1774,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..a0c51ebb910 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3646,6 +3646,10 @@ typedef struct IndexStmt SubTransactionId oldCreateSubid; /* rd_createSubid of oldNumber */ SubTransactionId oldFirstRelfilelocatorSubid; /* rd_firstRelfilelocatorSubid * of oldNumber */ + List *oldPartIndexRelids; /* table OIDs of partitions owning old + * indexes */ + List *oldPartIndexNames; /* corresponding old index names to + * preserve */ 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..656ea8b56ee 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4836,6 +4836,90 @@ 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; +-- Names are preserved throughout a multi-level partition index hierarchy. +create table alttype_part_multi (a int, b int) partition by range (a); +create index alttype_part_multi_idx on alttype_part_multi (a, b); +create table alttype_part_multi_mid (a int, b int) + partition by range (a); +create index alttype_part_multi_mid_custom_idx + on alttype_part_multi_mid (a, b); +create table alttype_part_multi_leaf (a int, b int); +create index alttype_part_multi_leaf_custom_idx + on alttype_part_multi_leaf (a, b); +alter table alttype_part_multi_mid attach partition alttype_part_multi_leaf + for values from (0) to (10); +alter table alttype_part_multi attach partition alttype_part_multi_mid + for values from (0) to (100); +alter table alttype_part_multi alter column b type bigint; +select indexrelid::regclass + from pg_index + where indrelid in ('alttype_part_multi_mid'::regclass, + 'alttype_part_multi_leaf'::regclass) + order by indexrelid::regclass::text; + indexrelid +------------------------------------ + alttype_part_multi_leaf_custom_idx + alttype_part_multi_mid_custom_idx +(2 rows) + +drop table alttype_part_multi; +-- Custom names of constraint indexes are preserved. +create table alttype_part_constr (a int, b int) + partition by range (a); +alter table alttype_part_constr + add constraint alttype_part_constr_key unique (a, b); +create table alttype_part_constr_child (a int, b int); +alter table alttype_part_constr_child + add constraint alttype_part_constr_child_custom_key unique (a, b); +alter table alttype_part_constr attach partition alttype_part_constr_child + for values from (0) to (10); +alter table alttype_part_constr alter column b type bigint; +select conname, conindid::regclass + from pg_constraint + where conrelid = 'alttype_part_constr_child'::regclass + and contype = 'u'; + conname | conindid +--------------------------------------+-------------------------------------- + alttype_part_constr_child_custom_key | alttype_part_constr_child_custom_key +(1 row) + +drop table alttype_part_constr; +-- SET EXPRESSION preserves custom partition index names too. +create table alttype_part_expr + (a int, b int generated always as (a * 2) stored) + partition by range (a); +create table alttype_part_expr_child + (a int, b int generated always as (a * 2) stored); +create index alttype_part_expr_child_custom_idx + on alttype_part_expr_child (b); +create index alttype_part_expr_idx on alttype_part_expr (b); +alter table alttype_part_expr attach partition alttype_part_expr_child + for values from (0) to (10); +alter table alttype_part_expr alter column b set expression as (a * 3); +select indexrelid::regclass + from pg_index where indrelid = 'alttype_part_expr_child'::regclass; + indexrelid +------------------------------------ + alttype_part_expr_child_custom_idx +(1 row) + +drop table alttype_part_expr; -- -- 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..947ae5a5a8f 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -3128,6 +3128,73 @@ 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; + +-- Names are preserved throughout a multi-level partition index hierarchy. +create table alttype_part_multi (a int, b int) partition by range (a); +create index alttype_part_multi_idx on alttype_part_multi (a, b); +create table alttype_part_multi_mid (a int, b int) + partition by range (a); +create index alttype_part_multi_mid_custom_idx + on alttype_part_multi_mid (a, b); +create table alttype_part_multi_leaf (a int, b int); +create index alttype_part_multi_leaf_custom_idx + on alttype_part_multi_leaf (a, b); +alter table alttype_part_multi_mid attach partition alttype_part_multi_leaf + for values from (0) to (10); +alter table alttype_part_multi attach partition alttype_part_multi_mid + for values from (0) to (100); +alter table alttype_part_multi alter column b type bigint; +select indexrelid::regclass + from pg_index + where indrelid in ('alttype_part_multi_mid'::regclass, + 'alttype_part_multi_leaf'::regclass) + order by indexrelid::regclass::text; +drop table alttype_part_multi; + +-- Custom names of constraint indexes are preserved. +create table alttype_part_constr (a int, b int) + partition by range (a); +alter table alttype_part_constr + add constraint alttype_part_constr_key unique (a, b); +create table alttype_part_constr_child (a int, b int); +alter table alttype_part_constr_child + add constraint alttype_part_constr_child_custom_key unique (a, b); +alter table alttype_part_constr attach partition alttype_part_constr_child + for values from (0) to (10); +alter table alttype_part_constr alter column b type bigint; +select conname, conindid::regclass + from pg_constraint + where conrelid = 'alttype_part_constr_child'::regclass + and contype = 'u'; +drop table alttype_part_constr; + +-- SET EXPRESSION preserves custom partition index names too. +create table alttype_part_expr + (a int, b int generated always as (a * 2) stored) + partition by range (a); +create table alttype_part_expr_child + (a int, b int generated always as (a * 2) stored); +create index alttype_part_expr_child_custom_idx + on alttype_part_expr_child (b); +create index alttype_part_expr_idx on alttype_part_expr (b); +alter table alttype_part_expr attach partition alttype_part_expr_child + for values from (0) to (10); +alter table alttype_part_expr alter column b set expression as (a * 3); +select indexrelid::regclass + from pg_index where indrelid = 'alttype_part_expr_child'::regclass; +drop table alttype_part_expr; + -- -- Check that attaching or detaching a partitioned partition correctly leads -- to its partitions' constraint being updated to reflect the parent's -- 2.47.3