From 8bd97da9fcfe6587978697bff48b700723eab8b3 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 21 Aug 2026 12:59:21 -0400 Subject: [PATCH v14 1/2] Preserve index per-column statistics targets across ALTER COLUMN TYPE A per-column statistics target set on an index (ALTER INDEX ... ALTER COLUMN n SET STATISTICS) was silently lost when ALTER TABLE ... ALTER COLUMN TYPE rebuilt the index. A statistics target is not expressible as a CREATE INDEX clause, so it is not reproduced by the pg_get_indexdef_string() that recreates the index. Capture the stats target before dropping the index and then reapply it after creating the new index. Only the statistics target, not the collected statistics data, which would be a compatibility concern with the new column type. Author: Zsolt Parragi Co-authored-by: Melanie Plageman Discussion: https://postgr.es/m/CAN4CZFNZwcCgi-igaD=LH1ubxMBqJJS+p4ZnOKKdCi9duaMu_w@mail.gmail.com Discussion: https://postgr.es/m/DB533C25-C6BA-4C0F-8046-96168E9CDD72@gmail.com --- src/backend/commands/indexcmds.c | 50 ++++++++++++++++++++++- src/backend/commands/tablecmds.c | 43 +++++++++++++++++++ src/include/nodes/parsenodes.h | 20 ++++++++- src/test/regress/expected/alter_table.out | 20 +++++++++ src/test/regress/sql/alter_table.sql | 14 +++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 146 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5a0312fe772..4e592ea7785 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -83,6 +83,7 @@ typedef struct CIEN_context /* non-export function prototypes */ static bool CompareOpclassOptions(const Datum *opts1, const Datum *opts2, int natts); static void CheckPredicate(Expr *predicate); +static void SetIndexStatTargets(Oid indexRelationId, List *stattargets); static void ComputeIndexAttrs(ParseState *pstate, IndexInfo *indexInfo, Oid *typeOids, @@ -510,6 +511,46 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress) } +/* + * Update the required catalog entries to restore the list of statistics + * targets to the index passed in as indexRelationId. stattargets is a list of + * IndexStatTarget nodes, one per column that had a target set. + */ +static void +SetIndexStatTargets(Oid indexRelationId, List *stattargets) +{ + Relation attrelation = table_open(AttributeRelationId, RowExclusiveLock); + + foreach_node(IndexStatTarget, st, stattargets) + { + HeapTuple attup; + HeapTuple newtuple; + Datum repl_val[Natts_pg_attribute]; + bool repl_null[Natts_pg_attribute]; + bool repl_repl[Natts_pg_attribute]; + + attup = SearchSysCacheCopy2(ATTNUM, + ObjectIdGetDatum(indexRelationId), + Int16GetDatum(st->attnum)); + if (!HeapTupleIsValid(attup)) + continue; + memset(repl_null, false, sizeof(repl_null)); + memset(repl_repl, false, sizeof(repl_repl)); + repl_val[Anum_pg_attribute_attstattarget - 1] = + Int16GetDatum(st->stattarget); + repl_repl[Anum_pg_attribute_attstattarget - 1] = true; + newtuple = heap_modify_tuple(attup, + RelationGetDescr(attrelation), + repl_val, repl_null, repl_repl); + CatalogTupleUpdate(attrelation, &newtuple->t_self, newtuple); + heap_freetuple(newtuple); + heap_freetuple(attup); + } + + table_close(attrelation, RowExclusiveLock); +} + + /* * DefineIndex * Creates a new index. @@ -1319,11 +1360,18 @@ DefineIndex(ParseState *pstate, root_save_nestlevel = NewGUCNestLevel(); RestrictSearchPath(); - /* Add any requested comment */ + /* + * Restore index properties that were not able to be recreated as part of + * CREATE INDEX. These were saved in the IndexStmt so we could restore + * them now by updating the correct catalog tables. + */ if (stmt->idxcomment != NULL) CreateComments(indexRelationId, RelationRelationId, 0, stmt->idxcomment); + if (stmt->idxstattargets != NIL) + SetIndexStatTargets(indexRelationId, stmt->idxstattargets); + if (partitioned) { PartitionDesc partdesc; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9b911310f05..f9b1b0aa82d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -707,6 +707,7 @@ static void RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, Oid objid, Relation rel, List *domname, const char *conname); static void TryReuseIndex(Oid oldId, IndexStmt *stmt); +static List *GetIndexStatTargets(Oid indexOid); static void TryReuseForeignKey(Oid oldId, Constraint *con); static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName, List *options, LOCKMODE lockmode); @@ -16439,6 +16440,8 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, stmt->reset_default_tblspc = true; /* keep the index's comment */ stmt->idxcomment = GetComment(oldId, RelationRelationId, 0); + /* keep the index's per-column statistics targets */ + stmt->idxstattargets = GetIndexStatTargets(oldId); newcmd = makeNode(AlterTableCmd); newcmd->subtype = AT_ReAddIndex; @@ -16468,6 +16471,8 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, /* keep any comment on the index */ indstmt->idxcomment = GetComment(indoid, RelationRelationId, 0); + /* keep the index's per-column statistics targets */ + indstmt->idxstattargets = GetIndexStatTargets(indoid); indstmt->reset_default_tblspc = true; cmd->subtype = AT_ReAddIndex; @@ -16615,6 +16620,44 @@ RebuildConstraintComment(AlteredTableInfo *tab, AlterTablePass pass, Oid objid, tab->subcmds[pass] = lappend(tab->subcmds[pass], newcmd); } +/* + * Collect the per-column statistics targets of an index into a list of + * IndexStatTarget nodes. Returns NIL if none are set. + */ +static List * +GetIndexStatTargets(Oid indexOid) +{ + List *result = NIL; + Relation irel; + + irel = index_open(indexOid, AccessShareLock); + for (int i = 1; i <= IndexRelationGetNumberOfAttributes(irel); i++) + { + HeapTuple atup; + Datum d; + bool isnull; + + atup = SearchSysCache2(ATTNUM, ObjectIdGetDatum(indexOid), + Int16GetDatum(i)); + if (!HeapTupleIsValid(atup)) + continue; + d = SysCacheGetAttr(ATTNUM, atup, + Anum_pg_attribute_attstattarget, &isnull); + if (!isnull) + { + IndexStatTarget *st = makeNode(IndexStatTarget); + + st->attnum = i; + st->stattarget = DatumGetInt16(d); + result = lappend(result, st); + } + ReleaseSysCache(atup); + } + index_close(irel, AccessShareLock); + + return result; +} + /* * Subroutine for ATPostAlterTypeParse(). Calls out to CheckIndexCompatible() * for the real analysis, then mutates the IndexStmt based on that verdict. diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 8a9df884276..84f7f0e7d3b 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3640,7 +3640,6 @@ typedef struct IndexStmt List *options; /* WITH clause options: a list of DefElem */ Node *whereClause; /* qualification (partial-index predicate) */ List *excludeOpNames; /* exclusion operator names, or NIL if none */ - char *idxcomment; /* comment to apply to index, or NULL */ Oid indexOid; /* OID of an existing index, if any */ RelFileNumber oldNumber; /* relfilenumber of existing storage, if any */ SubTransactionId oldCreateSubid; /* rd_createSubid of oldNumber */ @@ -3658,8 +3657,27 @@ typedef struct IndexStmt bool if_not_exists; /* just do nothing if index already exists? */ bool reset_default_tblspc; /* reset default_tablespace prior to * executing */ + + /* + * When doing an operation on the index that causes it to be dropped and + * recreated, these properties are not automatically cloned from the old + * index to the new and must be explicitly saved before dropping the old + * index and restored after creating the new index. + */ + char *idxcomment; /* comment to apply to index, or NULL */ + List *idxstattargets; /* list of IndexStatTarget to restore */ } IndexStmt; +/* one per-column statistics target carried across an index rebuild */ +typedef struct IndexStatTarget +{ + pg_node_attr(no_equal, no_query_jumble) + + NodeTag type; + int attnum; /* index column number (1-based) */ + int stattarget; /* attstattarget value to restore */ +} IndexStatTarget; + /* ---------------------- * Create Statistics Statement * ---------------------- diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index e167a41ce79..8b6d85461f9 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2326,6 +2326,26 @@ select conname, obj_description(oid, 'pg_constraint') as desc -- Don't remove this DROP, it exposes bug #15672 drop table at_partitioned; +-- Per-column statistics targets should still exist after an ALTER COLUMN TYPE +create table at_reb_plain (id int not null, val int not null); +create index at_reb_plain_expr on at_reb_plain ((val + 1)); +create unique index at_reb_plain_u on at_reb_plain ((id + 0), (val + 0)); +alter index at_reb_plain_expr alter column 1 set statistics 321; +alter index at_reb_plain_u alter column 1 set statistics 111; +alter index at_reb_plain_u alter column 2 set statistics 222; +alter table at_reb_plain alter column val type bigint; +select c.relname, a.attnum, a.attstattarget + from pg_attribute a join pg_class c on c.oid = a.attrelid + where c.relname in ('at_reb_plain_expr', 'at_reb_plain_u') and a.attnum > 0 + order by c.relname, a.attnum; + relname | attnum | attstattarget +-------------------+--------+--------------- + at_reb_plain_expr | 1 | 321 + at_reb_plain_u | 1 | 111 + at_reb_plain_u | 2 | 222 +(3 rows) + +drop table at_reb_plain; -- disallow recursive containment of row types create temp table recur1 (f1 int); alter table recur1 add column f2 recur1; -- fails diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9f6c2a4bb08..8bdfc4262ef 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1531,6 +1531,20 @@ select conname, obj_description(oid, 'pg_constraint') as desc -- Don't remove this DROP, it exposes bug #15672 drop table at_partitioned; +-- Per-column statistics targets should still exist after an ALTER COLUMN TYPE +create table at_reb_plain (id int not null, val int not null); +create index at_reb_plain_expr on at_reb_plain ((val + 1)); +create unique index at_reb_plain_u on at_reb_plain ((id + 0), (val + 0)); +alter index at_reb_plain_expr alter column 1 set statistics 321; +alter index at_reb_plain_u alter column 1 set statistics 111; +alter index at_reb_plain_u alter column 2 set statistics 222; +alter table at_reb_plain alter column val type bigint; +select c.relname, a.attnum, a.attstattarget + from pg_attribute a join pg_class c on c.oid = a.attrelid + where c.relname in ('at_reb_plain_expr', 'at_reb_plain_u') and a.attnum > 0 + order by c.relname, a.attnum; +drop table at_reb_plain; + -- disallow recursive containment of row types create temp table recur1 (f1 int); alter table recur1 add column f2 recur1; -- fails diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 6c366d3a523..6a464f80de1 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1350,6 +1350,7 @@ IndexScanDesc IndexScanDescData IndexScanInstrumentation IndexScanState +IndexStatTarget IndexStateFlagsAction IndexStmt IndexTuple -- 2.47.3