On Thu, Oct 29, 2020 at 10:59:52AM +0900, Michael Paquier wrote: > REINDEX CONCURRENTLY is by design wanted to provide an experience > transparent to the user similar to what a plain REINDEX would do, at > least that's the idea behind it, so.. This qualifies as a bug to me, > in spirit.
And in spirit, it is possible to address this issue with the patch attached which copies the set of stats from the old to the new index. For a non-concurrent REINDEX, this does not happen because we keep the same base relation, while we replace completely the relation with a concurrent operation. We have a RemoveStatistics() in heap.c, but I did not really see the point to invent a copy flavor for this particular case. Perhaps others have an opinion on that? -- Michael
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 0974f3e23a..3820a03fb6 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -49,6 +49,7 @@ #include "catalog/pg_inherits.h" #include "catalog/pg_opclass.h" #include "catalog/pg_operator.h" +#include "catalog/pg_statistic.h" #include "catalog/pg_tablespace.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" @@ -1711,6 +1712,44 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName) } } + /* + * Copy over any data of pg_statistic from the old index to the new one. + */ + { + HeapTuple tup; + SysScanDesc scan; + ScanKeyData key[1]; + Relation statrel; + + statrel = table_open(StatisticRelationId, RowExclusiveLock); + ScanKeyInit(&key[0], + Anum_pg_statistic_starelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(oldIndexId)); + + 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 = newIndexId; + CatalogTupleInsert(statrel, tup); + + heap_freetuple(tup); + } + + systable_endscan(scan); + + table_close(statrel, RowExclusiveLock); + } + /* Close relations */ table_close(pg_class, RowExclusiveLock); table_close(pg_index, RowExclusiveLock); diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 6ace7662ee..012c1eb067 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2551,6 +2551,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 --------------------------------------------------------------------------------------------------------------- @@ -2608,6 +2619,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 37f7259da9..4e45b18613 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -1079,6 +1079,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); @@ -1091,6 +1097,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.
signature.asc
Description: PGP signature