public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jeff Davis <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: Corey Huinker <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: jian he <[email protected]>
Subject: Re: Statistics Import and Export: difference in statistics dumped
Date: Fri, 28 Feb 2025 12:10:37 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAExHW5ttputnXg231arxPWWScP7MkZjqRNjt4jPeht5GfypErw@mail.gmail.com>
References: <CADkLM=f0a43aTd88xW4xCFayEF25g-7hTrHX_WhV40HyocsUGg@mail.gmail.com>
<[email protected]>
<CADkLM=ckXu5wzSQZ3Y_fvAaL8rZ+upZdSecBLf5FCTMj7M6T-A@mail.gmail.com>
<[email protected]>
<CADkLM=dRMC6t8gp9GVf6y6E_r5EChQjMAAh_vPyih_zMiq0zvA@mail.gmail.com>
<y2d36zf6pl3n6ni6e4ctblu3a5iazi2dycknq53h2mjvp7lto4@hhww3h2qgdrw>
<[email protected]>
<CADkLM=eWSv2z_DVB=psfwHitq7DbFJQaP9A46R27eARC-4OYtQ@mail.gmail.com>
<cpdanvzykcb5o64rmapkx6n5gjypoce3y52hff7ocxupgpbxu4@53jmlyvukijo>
<CADkLM=dX67xer59tVYMCQZ+pHAT2sCyyrUDzeDCdW_TdxqwOgA@mail.gmail.com>
<curtrasm6ylt7es2ukfuals2mh5an72yzxgvyel65mw3ere7qs@nw6m4ffpo4m3>
<[email protected]>
<CACJufxHG9MBQozbJQ4JRBcRbUO+t+sx4qLZX092rS_9b4SR_EA@mail.gmail.com>
<CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=C+FnxDvfprWvkng@mail.gmail.com>
<[email protected]>
<CAExHW5ttputnXg231arxPWWScP7MkZjqRNjt4jPeht5GfypErw@mail.gmail.com>
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
Attachments:
[text/x-patch] v2-0001-During-CREATE-INDEX-don-t-update-stats-if-autovac.patch (6.8K, ../[email protected]/2-v2-0001-During-CREATE-INDEX-don-t-update-stats-if-autovac.patch)
download | inline diff:
From a8945b9ce4e358f4a79d3065c07f3b42a94fd387 Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
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 <[email protected]>
Discussion: https://postgr.es/m/CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=C+FnxDvfprWvkng@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
view thread (170+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Statistics Import and Export: difference in statistics dumped
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox