On Fri, 2025-02-28 at 14:51 +0530, Ashutosh Bapat wrote:
> 2. We aren't restoring the statistics faithfully - as mentioned in
> Greg's reply. If users dump and restore with autovacuum turned off,
> they will be surprised to see the statistics to be different on the
> original and restored database - which may have other effects like
> change in plans.
Then let's just address that concern directly: disable updating stats
implicitly if autovacuum is off. If autovacuum is on, the user
shouldn't have an expectation of stable stats anyway. Patch attached.
Regards,
Jeff Davis
From a8945b9ce4e358f4a79d3065c07f3b42a94fd387 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Thu, 27 Feb 2025 17:06:00 -0800
Subject: [PATCH v2] During CREATE INDEX, don't update stats if autovacuum is
off.
We previously fixed this for binary upgrade in 71b66171d0, but a
similar problem existed when using pg_dump --no-data without
pg_upgrade involved.
Fix by not implicitly updating stats during create index when
autovacuum is disabled.
Reported-by: Ashutosh Bapat <ashutosh.bapat....@gmail.com>
Discussion: https://postgr.es/m/CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=c+fnxdvfprwv...@mail.gmail.com
---
src/backend/catalog/index.c | 31 +++++++++++++++--
src/test/regress/expected/stats_import.out | 39 ++++++++++++++++++----
src/test/regress/sql/stats_import.sql | 22 ++++++++++--
3 files changed, 82 insertions(+), 10 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f37b990c81d..318a44e1e1d 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -63,6 +63,7 @@
#include "optimizer/optimizer.h"
#include "parser/parser.h"
#include "pgstat.h"
+#include "postmaster/autovacuum.h"
#include "rewrite/rewriteManip.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
@@ -121,6 +122,7 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
bool isready);
static void index_update_stats(Relation rel,
bool hasindex,
+ bool update_stats,
double reltuples);
static void IndexCheckExclusion(Relation heapRelation,
Relation indexRelation,
@@ -1261,6 +1263,17 @@ index_create(Relation heapRelation,
}
else if ((flags & INDEX_CREATE_SKIP_BUILD) != 0)
{
+ bool update_stats = true;
+
+ /*
+ * If autovacuum is disabled, don't implicitly update stats as a part
+ * of index creation.
+ */
+ if (!AutoVacuumingActive() ||
+ (heapRelation->rd_options != NULL &&
+ !((StdRdOptions *) heapRelation->rd_options)->autovacuum.enabled))
+ update_stats = false;
+
/*
* Caller is responsible for filling the index later on. However,
* we'd better make sure that the heap relation is correctly marked as
@@ -1268,6 +1281,7 @@ index_create(Relation heapRelation,
*/
index_update_stats(heapRelation,
true,
+ update_stats,
-1.0);
/* Make the above update visible */
CommandCounterIncrement();
@@ -2807,9 +2821,9 @@ FormIndexDatum(IndexInfo *indexInfo,
static void
index_update_stats(Relation rel,
bool hasindex,
+ bool update_stats,
double reltuples)
{
- bool update_stats;
BlockNumber relpages = 0; /* keep compiler quiet */
BlockNumber relallvisible = 0;
Oid relid = RelationGetRelid(rel);
@@ -2837,7 +2851,8 @@ index_update_stats(Relation rel,
* Don't update statistics during binary upgrade, because the indexes are
* created before the data is moved into place.
*/
- update_stats = reltuples >= 0 && !IsBinaryUpgrade;
+ if (reltuples < 0 || IsBinaryUpgrade)
+ update_stats = false;
/*
* Finish I/O and visibility map buffer locks before
@@ -2981,6 +2996,7 @@ index_build(Relation heapRelation,
Oid save_userid;
int save_sec_context;
int save_nestlevel;
+ bool update_stats = true;
/*
* sanity checks
@@ -3121,15 +3137,26 @@ index_build(Relation heapRelation,
table_close(pg_index, RowExclusiveLock);
}
+ /*
+ * If autovacuum is disabled, don't implicitly update stats as a part
+ * of index creation.
+ */
+ if (!AutoVacuumingActive() ||
+ (heapRelation->rd_options != NULL &&
+ !((StdRdOptions *) heapRelation->rd_options)->autovacuum.enabled))
+ update_stats = false;
+
/*
* Update heap and index pg_class rows
*/
index_update_stats(heapRelation,
true,
+ update_stats,
stats->heap_tuples);
index_update_stats(indexRelation,
false,
+ update_stats,
stats->index_tuples);
/* Make the updated catalog row versions visible */
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index 1f150f7b08d..abd391181e4 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -12,15 +12,42 @@ CREATE TABLE stats_import.test(
arange int4range,
tags text[]
) WITH (autovacuum_enabled = false);
+SELECT
+ pg_catalog.pg_restore_relation_stats(
+ 'relation', 'stats_import.test'::regclass,
+ 'relpages', 18::integer,
+ 'reltuples', 21::real,
+ 'relallvisible', 24::integer);
+ pg_restore_relation_stats
+---------------------------
+ t
+(1 row)
+
+-- CREATE INDEX on a table with autovac disabled should not overwrite
+-- stats
CREATE INDEX test_i ON stats_import.test(id);
+SELECT
+ pg_catalog.pg_restore_relation_stats(
+ 'relation', 'stats_import.test_i'::regclass,
+ 'relpages', 28::integer,
+ 'reltuples', 35::real,
+ 'relallvisible', 42::integer);
+ pg_restore_relation_stats
+---------------------------
+ t
+(1 row)
+
-- starting stats
-SELECT relpages, reltuples, relallvisible
+SELECT relname, relpages, reltuples, relallvisible
FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
- relpages | reltuples | relallvisible
-----------+-----------+---------------
- 0 | -1 | 0
-(1 row)
+WHERE oid = 'stats_import.test'::regclass
+ OR oid = 'stats_import.test_i'::regclass
+ORDER BY relname;
+ relname | relpages | reltuples | relallvisible
+---------+----------+-----------+---------------
+ test | 18 | 21 | 24
+ test_i | 28 | 35 | 42
+(2 rows)
BEGIN;
-- regular indexes have special case locking rules
diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index 8c183bceb8a..f8907504de1 100644
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -15,12 +15,30 @@ CREATE TABLE stats_import.test(
tags text[]
) WITH (autovacuum_enabled = false);
+SELECT
+ pg_catalog.pg_restore_relation_stats(
+ 'relation', 'stats_import.test'::regclass,
+ 'relpages', 18::integer,
+ 'reltuples', 21::real,
+ 'relallvisible', 24::integer);
+
+-- CREATE INDEX on a table with autovac disabled should not overwrite
+-- stats
CREATE INDEX test_i ON stats_import.test(id);
+SELECT
+ pg_catalog.pg_restore_relation_stats(
+ 'relation', 'stats_import.test_i'::regclass,
+ 'relpages', 28::integer,
+ 'reltuples', 35::real,
+ 'relallvisible', 42::integer);
+
-- starting stats
-SELECT relpages, reltuples, relallvisible
+SELECT relname, relpages, reltuples, relallvisible
FROM pg_class
-WHERE oid = 'stats_import.test'::regclass;
+WHERE oid = 'stats_import.test'::regclass
+ OR oid = 'stats_import.test_i'::regclass
+ORDER BY relname;
BEGIN;
-- regular indexes have special case locking rules
--
2.34.1
