On Fri, Oct 30, 2020 at 3:22 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> 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.
Did some tests and everything went ok... some comments below!
> 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.
Exactly!
> 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?
>
Even if we won't use it now, IMHO it is more legible to separate this
responsibility into its own CopyStatistics function as attached.
Regards,
--
Fabrízio de Royes Mello
PostgreSQL Developer at OnGres Inc. - https://ongres.com
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 67144aa3c9..7da1f7eb56 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3142,6 +3142,46 @@ cookConstraint(ParseState *pstate,
return expr;
}
+/*
+ * CopyStatistics --- copy entries in pg_statistic from old to new rel
+ *
+ */
+void
+CopyStatistics(Oid oldRelId, Oid newRelId)
+{
+ 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(oldRelId));
+
+ 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 = newRelId;
+ 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 0974f3e23a..a6975c2dbe 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,11 @@ 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.
+ */
+ 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 d31141c1a2..3d42edbbf6 100644
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -134,6 +134,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 oldRelId, Oid newRelId);
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 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.
