Обсуждение: Preserve index stats during ALTER TABLE ... TYPE ...
Hi hackers,
while working on relfilenode statistics [1], I observed that index stats
are not preserved during ALTER TABLE ... TYPE ....
Indeed, for example:
postgres=# CREATE TABLE test_tab(a int primary key, b int, c int);
CREATE INDEX test_b_idx ON test_tab(b);
-- Force an index scan on test_b_idx
SELECT * FROM test_tab WHERE b = 2;
CREATE TABLE
CREATE INDEX
a | b | c
---+---+---
(0 rows)
postgres=# select indexrelname, idx_scan from pg_stat_all_indexes where indexrelname in ('test_b_idx',
'test_tab_pkey');
indexrelname | idx_scan
---------------+----------
test_tab_pkey | 0
test_b_idx | 1
(2 rows)
postgres=# select idx_scan from pg_stat_all_tables where relname = 'test_tab';
idx_scan
----------
1
(1 row)
postgres=# ALTER TABLE test_tab ALTER COLUMN b TYPE int;
ALTER TABLE
postgres=# select indexrelname, idx_scan from pg_stat_all_indexes where indexrelname in ('test_b_idx',
'test_tab_pkey');
indexrelname | idx_scan
---------------+----------
test_tab_pkey | 0
test_b_idx | 0
(2 rows)
postgres=# select idx_scan from pg_stat_all_tables where relname = 'test_tab';
idx_scan
----------
0
(1 row)
During ALTER TABLE ... TYPE ... on an indexed column, a new index is created and
the old one is dropped.
As you can see, the index stats (linked to the column that has been altered) are
not preserved. I think that they should be preserved (like a REINDEX does).
Note that the issue is the same if a rewrite is involved (ALTER TABLE test_tab
ALTER COLUMN b TYPE bigint).
PFA, a patch to $SUBJECT.
A few remarks:
- We can not use pgstat_copy_relation_stats() because the old index is dropped
before the new one is created, so the patch adds a new PgStat_StatTabEntry
pointer in AlteredTableInfo.
- The stats are saved in ATPostAlterTypeParse() (before the old index is dropped)
and restored in ATExecAddIndex() once the new index is created.
- Note that pending statistics (if any) are not preserved, only the
accumulated stats from previous transactions. I think this is
acceptable since the accumulated stats represent the historical usage patterns we
want to maintain.
- The patch adds a few tests to cover multiple scenarios (with and without
rewrites, and indexes with and without associated constraints).
- I'm not familiar with this area of the code, the patch is an attempt to fix
the issue, maybe there is a more elegant way to solve it.
- The issue exists back to v13, but I'm not sure that's serious enough for
back-patching.
Looking forward to your feedback,
Regards,
[1]: https://postgr.es/m/ZlGYokUIlERemvpB%40ip-10-97-1-34.eu-west-3.compute.internal
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Вложения
Hi,
Thanks for raising this issue and for the patch!
> As you can see, the index stats (linked to the column that has been altered) are
> not preserved. I think that they should be preserved (like a REINDEX does).
I agree.
> - We can not use pgstat_copy_relation_stats() because the old index is dropped
> before the new one is created, so the patch adds a new PgStat_StatTabEntry
> pointer in AlteredTableInfo.
I wonder if it will be good to have a pgstat_save_relation_stats() routine that
gets called in all code paths that will need to restore the stats. This way
pgstat_copy_relation_stats can also be used. This will be cleaner than code
paths that need this having to deal with pgstat_fetch_stat_tabentry?
Have not thought this thoroughly, but it seems like it might be a more general
approach.
> - The patch adds a few tests to cover multiple scenarios (with and without
> rewrites, and indexes with and without associated constraints).
The current patch does not work for partitioned tables because
the "oldId" is that of the parent index which has no stats. So we
are just copying zeros to the new entry.
```
DROP TABLE test_tab;
CREATE TABLE test_tab(a int primary key, b int, c int) partition by range (a);
CREATE TABLE test_tab_p1 PARTITION OF test_tab
FOR VALUES FROM (0) TO (100);
CREATE TABLE test_tab_p2 PARTITION OF test_tab
FOR VALUES FROM (100) TO (200);
CREATE INDEX test_b_idx ON test_tab(b);
-- Force an index scan on test_b_idx
SELECT * FROM test_tab WHERE b = 2;
test=# select indexrelname, idx_scan from pg_stat_all_indexes where
indexrelname like '%test%';
indexrelname | idx_scan
-------------------+----------
test_tab_p1_pkey | 0
test_tab_p2_pkey | 0
test_tab_p1_b_idx | 1
test_tab_p2_b_idx | 1
(4 rows)
test=# ALTER TABLE test_tab ALTER COLUMN b TYPE int;
ALTER TABLE
test=# select indexrelname, idx_scan from pg_stat_all_indexes where
indexrelname like '%test%';
indexrelname | idx_scan
-------------------+----------
test_tab_p1_pkey | 0
test_tab_p2_pkey | 0
test_tab_p1_b_idx | 0
test_tab_p2_b_idx | 0
(4 rows)
```
Regards,
--
Sami Imseih
Amazon Web Services (AWS)
Hi, On Fri, Oct 10, 2025 at 07:37:59AM -0500, Sami Imseih wrote: > Hi, > > Thanks for raising this issue and for the patch! Thanks for looking at it! > > As you can see, the index stats (linked to the column that has been altered) are > > not preserved. I think that they should be preserved (like a REINDEX does). > > I agree. > > > - We can not use pgstat_copy_relation_stats() because the old index is dropped > > before the new one is created, so the patch adds a new PgStat_StatTabEntry > > pointer in AlteredTableInfo. > > I wonder if it will be good to have a pgstat_save_relation_stats() routine that > gets called in all code paths that will need to restore the stats. This way > pgstat_copy_relation_stats can also be used. This will be cleaner than code > paths that need this having to deal with pgstat_fetch_stat_tabentry? pgstat_copy_relation_stats() needs 2 Relation, I'm not sure how a new pgstat_save_relation_stats() could help using pgstat_copy_relation_stats() here. > The current patch does not work for partitioned tables because > the "oldId" is that of the parent index which has no stats. So we > are just copying zeros to the new entry. Doh, of course. I've spend some time on it and now have something working. The idea is to: - store a List of savedIndexStats. The savedIndexStats struct would get the PgStat_StatTabEntry + all the information needed to be able to use CompareIndexInfo() when restoring the stats (so that we can restore each PgStat_StatTabEntry in the right index). - Iterate on all the indexes and populate this new list in AlteredTableInfo in ATPostAlterTypeParse(). - Iterate on all the indexes and use the list above and CompareIndexInfo() to restore the stats in ATExecAddIndex(). Will polish and share next week. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Hi,
On Fri, Oct 10, 2025 at 03:52:58PM +0000, Bertrand Drouvot wrote:
> The idea is to:
>
> - store a List of savedIndexStats. The savedIndexStats struct would get the
> PgStat_StatTabEntry + all the information needed to be able to use
> CompareIndexInfo() when restoring the stats (so that we can restore each PgStat_StatTabEntry
> in the right index).
>
> - Iterate on all the indexes and populate this new list in AlteredTableInfo in
> ATPostAlterTypeParse().
>
> - Iterate on all the indexes and use the list above and CompareIndexInfo() to
> restore the stats in ATExecAddIndex().
>
> Will polish and share next week.
PFA v2 that handles partitioned tables/indexes.
A few words about its design:
I started by just creating a list of of PgStat_StatTabEntry + all the information
needed to be able to use CompareIndexInfo() when restoring the stats.
But that lead to O(P^2) when restoring the stats (for each new partition index
(P), it was scanning through all saved ones (P)), and could be non negligible.
For example, with 20K partitions and no rewrite:
- 89.64% 0.00% postgres postgres [.] ATController
- ATController
- 79.23% ATRewriteCatalogs
- 64.43% ATExecCmd
- 56.53% ATExecAddIndex
+ 46.34% DefineIndex
+ 10.19% ATExecAddIndex_RestoreStats
+ 5.29% ATExecAlterColumnType
+ 2.60% CommandCounterIncrement
+ 11.91% ATPostAlterTypeCleanup
+ 2.77% relation_open
+ 8.79% ATPrepCmd
+ 1.62% ATRewriteTables
We can see ATExecAddIndex_RestoreStats was not negligible at that time. That was
less of an issue when rewrite was involved:
- 89.35% 0.00% postgres postgres [.] ATController
- ATController
+ 51.24% ATRewriteTables
- 33.89% ATRewriteCatalogs
- 26.98% ATExecCmd
- 22.16% ATExecAddIndex
+ 17.44% DefineIndex
4.71% ATExecAddIndex_RestoreStats
+ 3.58% ATExecAlterColumnType
+ 1.24% CommandCounterIncrement
+ 5.53% ATPostAlterTypeCleanup
+ 1.32% relation_open
+ 4.22% ATPrepCmd
So I added a hash table keyed by partition table OID, with each entry containing
a list of saved index stats for that partition. This way, restoration is now O(P)
instead of O(P²).
With the attached, the perf profile (again 20K partitions and no rewrite) is:
- 89.06% 0.00% postgres postgres [.] ATController
- ATController
- 77.65% ATRewriteCatalogs
- 61.57% ATExecCmd
- 52.63% ATExecAddIndex
+ 51.16% DefineIndex
+ 1.47% ATExecAddIndex_RestoreStats
+ 5.96% ATExecAlterColumnType
+ 2.98% CommandCounterIncrement
+ 13.26% ATPostAlterTypeCleanup
+ 2.73% relation_open
+ 9.59% ATPrepCmd
+ 1.82% ATRewriteTables
As we can see, the ATExecAddIndex_RestoreStats impact is now around 1.5%, which
I think is acceptable given the benefit of preserving historical statistics.
Additional remarks:
- I initially tried using only CompareIndexInfo() for matching, but this fails
when multiple indexes exist on the same column(s). So I added the index name as
the primary matching check with CompareIndexInfo() kept as a sanity check (I think
that it could be removed).
- The new resources are allocated in the PortalContext, it's a short lived one so
the patch does not free them explicitly.
- Much more tests have been added as compared to v1.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com