From 4389d626ab92a7aa5ecb92f81e6a58383d8b02cb Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sat, 5 Sep 2026 03:19:22 +0000 Subject: [PATCH v19 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. Add regression coverage for ALTER TABLE ... ALTER COLUMN ... SET EXPRESSION, which rebuilds indexes through the same path. 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 | 55 ++++++++++++++++++++++- src/backend/commands/tablecmds.c | 44 ++++++++++++++++++ src/include/nodes/parsenodes.h | 22 ++++++++- src/test/regress/expected/alter_table.out | 36 +++++++++++++++ src/test/regress/sql/alter_table.sql | 25 +++++++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 179 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 932b2febe17..151a9952522 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. @@ -1321,11 +1366,14 @@ 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->idxextensionOids != NIL) { ObjectAddress indexAddress; @@ -1341,6 +1389,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 5d9df8bf134..d1afed33efe 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 auto-extension dependencies */ 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 auto-extension dependencies */ 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 bd72ffb09d2..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 */ 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 6ff3b3c3bf2..b52def050fa 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2346,6 +2346,42 @@ select c.relname, d.deptype, e.extname (2 rows) drop table at_reb_dep; +-- 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; +-- SET EXPRESSION rebuilds indexes through the same path. +create table at_reb_set + (id int not null, 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 c.relname, a.attnum, a.attstattarget + from pg_attribute a join pg_class c on c.oid = a.attrelid + where c.relname = 'at_reb_set_idx' and a.attnum > 0; + relname | attnum | attstattarget +----------------+--------+--------------- + at_reb_set_idx | 1 | 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 6d1ba1b4273..d50fbf1df0a 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1545,6 +1545,31 @@ select c.relname, d.deptype, e.extname order by c.relname; drop table at_reb_dep; +-- 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; +-- SET EXPRESSION rebuilds indexes through the same path. +create table at_reb_set + (id int not null, 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 c.relname, a.attnum, a.attstattarget + from pg_attribute a join pg_class c on c.oid = a.attrelid + where c.relname = 'at_reb_set_idx' and a.attnum > 0; +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/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 58c4749e7e4..5596591509c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1348,6 +1348,7 @@ IndexScanDesc IndexScanDescData IndexScanInstrumentation IndexScanState +IndexStatTarget IndexStateFlagsAction IndexStmt IndexTuple -- 2.50.1