From 045d5b32c4d416ca0ed10c4517f027881ddd4b60 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 21 Aug 2026 12:59:21 -0400 Subject: [PATCH v18 2/3] 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 | 48 +++++++++++++++++++++++ src/backend/commands/tablecmds.c | 44 +++++++++++++++++++++ src/include/nodes/parsenodes.h | 22 ++++++++++- src/test/regress/expected/alter_table.out | 35 +++++++++++++++++ src/test/regress/sql/alter_table.sql | 24 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 172 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 620359a7a32..f23dc6ee2e6 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -29,6 +29,7 @@ #include "catalog/index.h" #include "catalog/indexing.h" #include "catalog/namespace.h" +#include "catalog/objectaccess.h" #include "catalog/pg_am.h" #include "catalog/pg_authid.h" #include "catalog/pg_collation.h" @@ -85,6 +86,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, @@ -512,6 +514,49 @@ 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)) + elog(ERROR, "cache lookup failed for attribute %d of relation %u", + st->attnum, indexRelationId); + 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); + InvokeObjectPostAlterHook(RelationRelationId, indexRelationId, + st->attnum); + heap_freetuple(newtuple); + heap_freetuple(attup); + } + + table_close(attrelation, RowExclusiveLock); +} + + /* * DefineIndex * Creates a new index. @@ -1341,6 +1386,9 @@ DefineIndex(ParseState *pstate, } } + 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 d25a7cc16ce..b65ab6e0615 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -685,6 +685,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); @@ -16381,6 +16382,8 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, /* keep any DEPENDS ON EXTENSION links */ stmt->idxextensionOids = getAutoExtensionsOfObject(RelationRelationId, oldId); + /* keep the index's per-column statistics targets */ + stmt->idxstattargets = GetIndexStatTargets(oldId); newcmd = makeNode(AlterTableCmd); newcmd->subtype = AT_ReAddIndex; @@ -16413,6 +16416,8 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, Oid ownerId, /* keep any DEPENDS ON EXTENSION links */ indstmt->idxextensionOids = getAutoExtensionsOfObject(RelationRelationId, indoid); + /* keep the index's per-column statistics targets */ + indstmt->idxstattargets = GetIndexStatTargets(indoid); indstmt->reset_default_tblspc = true; cmd->subtype = AT_ReAddIndex; @@ -16560,6 +16565,45 @@ 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)) + elog(ERROR, "cache lookup failed for attribute %d of relation %u", + i, indexOid); + 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 78a393b79fb..c9421084208 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3612,8 +3612,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 */ - List *idxextensionOids; /* extensions to depend on after a rebuild */ Oid indexOid; /* OID of an existing index, if any */ RelFileNumber oldNumber; /* relfilenumber of existing storage, if any */ SubTransactionId oldCreateSubid; /* rd_createSubid of oldNumber */ @@ -3631,8 +3629,28 @@ 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 */ + List *idxextensionOids; /* extensions to depend on */ } 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) */ + int16 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 62bdf95860f..2e7d194a477 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2347,6 +2347,41 @@ select c.relname, d.deptype, e.extname (2 rows) drop table at_reb_extdep; +-- 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; +-- Per-column statistics targets should still exist after SET EXPRESSION +create table at_reb_set (val int not null, + gen int generated always as (val + 1) stored); +create index at_reb_set_idx on at_reb_set ((gen + 1)); +alter index at_reb_set_idx alter column 1 set statistics 654; +alter table at_reb_set alter column gen set expression as (val + 2); +select attstattarget + from pg_attribute + where attrelid = 'at_reb_set_idx'::regclass and attnum = 1; + attstattarget +--------------- + 654 +(1 row) + +drop table at_reb_set; -- 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 4cdf476395d..21abfcdcc41 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1546,6 +1546,30 @@ select c.relname, d.deptype, e.extname and d.refclassid = 'pg_extension'::regclass order by c.relname; drop table at_reb_extdep; +-- 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; + +-- Per-column statistics targets should still exist after SET EXPRESSION +create table at_reb_set (val int not null, + gen int generated always as (val + 1) stored); +create index at_reb_set_idx on at_reb_set ((gen + 1)); +alter index at_reb_set_idx alter column 1 set statistics 654; +alter table at_reb_set alter column gen set expression as (val + 2); +select attstattarget + from pg_attribute + where attrelid = 'at_reb_set_idx'::regclass and attnum = 1; +drop table at_reb_set; -- disallow recursive containment of row types create temp table recur1 (f1 int); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index c546b3d6375..30ab006e318 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1349,6 +1349,7 @@ IndexScanDesc IndexScanDescData IndexScanInstrumentation IndexScanState +IndexStatTarget IndexStateFlagsAction IndexStmt IndexTuple -- 2.50.1