public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jeff Davis <[email protected]>
To: Ashutosh Bapat <[email protected]>
To: 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: [email protected]
Cc: jian he <[email protected]>
Subject: Re: Statistics Import and Export
Date: Thu, 27 Feb 2025 18:32:20 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=C+FnxDvfprWvkng@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>
On Tue, 2025-02-25 at 11:11 +0530, Ashutosh Bapat wrote:
> So the dumped statistics are not restored exactly. The reason for
> this
> is the table statistics is dumped before dumping ALTER TABLE ... ADD
> CONSTRAINT command which changes the statistics. I think all the
> pg_restore_relation_stats() calls should be dumped after all the
> schema and data modifications have been done. OR what's the point in
> dumping statistics only to get rewritten even before restore
> finishes.
In your example, it's not so bad because the stats are actually better:
the index is built after the data is present, and therefore relpages
and reltuples are correct.
The problem is more clear if you use --no-data. If you load data,
ANALYZE, pg_dump --no-data, then reload the sql file, then the stats
are lost.
That workflow is very close to what pg_upgrade does. We solved the
problem for pg_upgrade in commit 71b66171d0 by simply not updating the
statistics when building an index and IsBinaryUpgrade.
To solve the issue with dump --no-data, I propose that we change the
test in 71b66171d0 to only update the stats if the physical relpages is
non-zero.
Patch attached:
* If the dump is --no-data, or during pg_upgrade, the table will be
empty, so the physical relpages will be zero and the restored stats
won't be overwritten.
* If (like in your example) the dump includes data, the new stats are
based on real data, so they are better anyway. This is sort of like the
case where autoanalyze kicks in.
* If the dump is --statistics-only, then there won't be any indexes
created in the SQL file, so when you restore the stats, they will
remain until you do something else to change them.
* If your example really is a problem, you'd need to dump first with -
-no-statistics, and then with --statistics-only, and restore the two
SQL files in order.
Alternatively, we could put stats into SECTION_POST_DATA, which was
already discussed[*], and we decided against it (though there was not a
clear consensus).
Regards,
Jeff Davis
*:
https://www.postgresql.org/message-id/1798867.1712376328%40sss.pgh.pa.us
Attachments:
[text/x-patch] v1-0001-Do-not-update-stats-on-empty-table-when-building-.patch (4.1K, ../[email protected]/2-v1-0001-Do-not-update-stats-on-empty-table-when-building-.patch)
download | inline diff:
From 86c03cb525b49d24019c5c0ea8ec36bb82b3c58a Mon Sep 17 00:00:00 2001
From: Jeff Davis <[email protected]>
Date: Thu, 27 Feb 2025 17:06:00 -0800
Subject: [PATCH v1] Do not update stats on empty table when building index.
We previously fixed this for binary upgrade in 71b66171d0, but a
similar problem exists when using pg_dump --no-data without pg_upgrade
involved. Fix both problems by not updating the stats when the table
has no pages.
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 | 17 +++++++++++------
src/test/regress/expected/stats_import.out | 22 +++++++++++++++++++++-
src/test/regress/sql/stats_import.sql | 12 ++++++++++++
3 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f37b990c81d..1a3fdeab350 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2833,11 +2833,7 @@ index_update_stats(Relation rel,
if (reltuples == 0 && rel->rd_rel->reltuples < 0)
reltuples = -1;
- /*
- * Don't update statistics during binary upgrade, because the indexes are
- * created before the data is moved into place.
- */
- update_stats = reltuples >= 0 && !IsBinaryUpgrade;
+ update_stats = reltuples >= 0;
/*
* Finish I/O and visibility map buffer locks before
@@ -2850,7 +2846,16 @@ index_update_stats(Relation rel,
{
relpages = RelationGetNumberOfBlocks(rel);
- if (rel->rd_rel->relkind != RELKIND_INDEX)
+ /*
+ * Don't update statistics when the relation is completely empty. This
+ * is important during binary upgrade, because at the time the schema
+ * is loaded, the data has not yet been moved into place. It's also
+ * useful when restoring a dump containing only schema and statistics.
+ */
+ if (relpages == 0)
+ update_stats = false;
+
+ if (update_stats && rel->rd_rel->relkind != RELKIND_INDEX)
visibilitymap_count(rel, &relallvisible, NULL);
}
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index 1f150f7b08d..4c81fb60c91 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -12,14 +12,34 @@ 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)
+
+-- creating an index on an empty table shouldn't overwrite stats
CREATE INDEX test_i ON stats_import.test(id);
+SELECT relpages, reltuples, relallvisible
+FROM pg_class
+WHERE oid = 'stats_import.test'::regclass;
+ relpages | reltuples | relallvisible
+----------+-----------+---------------
+ 18 | 21 | 24
+(1 row)
+
-- starting stats
SELECT relpages, reltuples, relallvisible
FROM pg_class
WHERE oid = 'stats_import.test'::regclass;
relpages | reltuples | relallvisible
----------+-----------+---------------
- 0 | -1 | 0
+ 18 | 21 | 24
(1 row)
BEGIN;
diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index 8c183bceb8a..c8abb715130 100644
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -15,8 +15,20 @@ 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);
+
+-- creating an index on an empty table shouldn't overwrite stats
CREATE INDEX test_i ON stats_import.test(id);
+SELECT relpages, reltuples, relallvisible
+FROM pg_class
+WHERE oid = 'stats_import.test'::regclass;
+
-- starting stats
SELECT relpages, reltuples, relallvisible
FROM pg_class
--
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
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