From bb62df46bcaa109d5eb1907392034024dde0886e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Sun, 1 Nov 2020 21:24:10 +0900 Subject: [PATCH] Preserve index data in pg_statistic across REINDEX CONCURRENTLY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Statistics associated to an index got lost after running REINDEX CONCURRENTLY, while the non-concurrent case preserves these correctly. The concurrent and non-concurrent operations need to be consistent for the end-user, and missing statistics would force to wait for a new analyze to happen, which could take some time depending on the activity of the existing autovacuum workers. This issue is fixed by copying any existing entries in pg_statistic associated to the old index to the new one. Note that this copy is already done with the data of the index in the stats collector. Reported-by: Fabrízio de Royes Mello Author: Michael Paquier, Fabrízio de Royes Mello Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/CAFcNs+qpFPmiHd1oTXvcPdvAHicJDA9qBUSujgAhUMJyUMb+SA@mail.gmail.com Backpatch-through: 12 --- src/backend/catalog/heap.c | 41 ++++++++++++++++++++++ src/backend/catalog/index.c | 3 ++ src/include/catalog/heap.h | 1 + src/test/regress/expected/create_index.out | 22 ++++++++++++ src/test/regress/sql/create_index.sql | 12 +++++++ 5 files changed, 79 insertions(+) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index e393c93a45..50f72e8bfd 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -3112,6 +3112,47 @@ cookConstraint(ParseState *pstate, return expr; } +/* + * CopyStatistics --- copy entries in pg_statistic from one rel to another + */ +void +CopyStatistics(Oid fromrelid, Oid torelid) +{ + HeapTuple tup; + SysScanDesc scan; + ScanKeyData key[1]; + Relation statrel; + + statrel = table_open(StatisticRelationId, RowExclusiveLock); + + /* Now search for stat records */ + ScanKeyInit(&key[0], + Anum_pg_statistic_starelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fromrelid)); + + scan = systable_beginscan(statrel, StatisticRelidAttnumInhIndexId, + true, NULL, 1, key); + + while (HeapTupleIsValid((tup = systable_getnext(scan)))) + { + Form_pg_statistic statform; + + /* make a modifiable copy */ + tup = heap_copytuple(tup); + statform = (Form_pg_statistic) GETSTRUCT(tup); + + /* update the copy of the tuple and insert it */ + statform->starelid = torelid; + CatalogTupleInsert(statrel, tup); + + heap_freetuple(tup); + } + + systable_endscan(scan); + + table_close(statrel, RowExclusiveLock); +} /* * RemoveStatistics --- remove entries in pg_statistic for a rel or column diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index cdc01c49c9..453b6e8793 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1725,6 +1725,9 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName) } } + /* Copy data of pg_statistic from the old index to the new one */ + CopyStatistics(oldIndexId, newIndexId); + /* Close relations */ table_close(pg_class, RowExclusiveLock); table_close(pg_index, RowExclusiveLock); diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index cbfdfe2abe..bec795abac 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -133,6 +133,7 @@ extern void RemoveAttributeById(Oid relid, AttrNumber attnum); extern void RemoveAttrDefault(Oid relid, AttrNumber attnum, DropBehavior behavior, bool complain, bool internal); extern void RemoveAttrDefaultById(Oid attrdefId); +extern void CopyStatistics(Oid fromrelid, Oid torelid); extern void RemoveStatistics(Oid relid, AttrNumber attnum); extern const FormData_pg_attribute *SystemAttributeDefinition(AttrNumber attno); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..49aa5780a8 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2419,6 +2419,17 @@ CREATE UNIQUE INDEX concur_exprs_index_pred ON concur_exprs_tab (c1) CREATE UNIQUE INDEX concur_exprs_index_pred_2 ON concur_exprs_tab ((1 / c1)) WHERE ('-H') >= (c2::TEXT) COLLATE "C"; +ANALYZE concur_exprs_tab; +SELECT starelid::regclass, count(*) FROM pg_statistic WHERE starelid IN ( + 'concur_exprs_index_expr'::regclass, + 'concur_exprs_index_pred'::regclass, + 'concur_exprs_index_pred_2'::regclass) + GROUP BY starelid ORDER BY starelid::regclass::text; + starelid | count +-------------------------+------- + concur_exprs_index_expr | 1 +(1 row) + SELECT pg_get_indexdef('concur_exprs_index_expr'::regclass); pg_get_indexdef --------------------------------------------------------------------------------------------------------------- @@ -2476,6 +2487,17 @@ SELECT pg_get_indexdef('concur_exprs_index_pred_2'::regclass); CREATE UNIQUE INDEX concur_exprs_index_pred_2 ON public.concur_exprs_tab USING btree (((1 / c1))) WHERE ('-H'::text >= (c2 COLLATE "C")) (1 row) +-- Statistics should remain intact. +SELECT starelid::regclass, count(*) FROM pg_statistic WHERE starelid IN ( + 'concur_exprs_index_expr'::regclass, + 'concur_exprs_index_pred'::regclass, + 'concur_exprs_index_pred_2'::regclass) + GROUP BY starelid ORDER BY starelid::regclass::text; + starelid | count +-------------------------+------- + concur_exprs_index_expr | 1 +(1 row) + DROP TABLE concur_exprs_tab; -- Temporary tables and on-commit actions, where CONCURRENTLY is ignored. -- ON COMMIT PRESERVE ROWS, the default. diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..344c648485 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1003,6 +1003,12 @@ CREATE UNIQUE INDEX concur_exprs_index_pred ON concur_exprs_tab (c1) CREATE UNIQUE INDEX concur_exprs_index_pred_2 ON concur_exprs_tab ((1 / c1)) WHERE ('-H') >= (c2::TEXT) COLLATE "C"; +ANALYZE concur_exprs_tab; +SELECT starelid::regclass, count(*) FROM pg_statistic WHERE starelid IN ( + 'concur_exprs_index_expr'::regclass, + 'concur_exprs_index_pred'::regclass, + 'concur_exprs_index_pred_2'::regclass) + GROUP BY starelid ORDER BY starelid::regclass::text; SELECT pg_get_indexdef('concur_exprs_index_expr'::regclass); SELECT pg_get_indexdef('concur_exprs_index_pred'::regclass); SELECT pg_get_indexdef('concur_exprs_index_pred_2'::regclass); @@ -1015,6 +1021,12 @@ ALTER TABLE concur_exprs_tab ALTER c2 TYPE TEXT; SELECT pg_get_indexdef('concur_exprs_index_expr'::regclass); SELECT pg_get_indexdef('concur_exprs_index_pred'::regclass); SELECT pg_get_indexdef('concur_exprs_index_pred_2'::regclass); +-- Statistics should remain intact. +SELECT starelid::regclass, count(*) FROM pg_statistic WHERE starelid IN ( + 'concur_exprs_index_expr'::regclass, + 'concur_exprs_index_pred'::regclass, + 'concur_exprs_index_pred_2'::regclass) + GROUP BY starelid ORDER BY starelid::regclass::text; DROP TABLE concur_exprs_tab; -- Temporary tables and on-commit actions, where CONCURRENTLY is ignored.