public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hari Krishna Sunder <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Corey Huinker <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: Robert Treat <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: jian he <[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: Ashutosh Bapat <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: [email protected]
Subject: Re: Statistics Import and Export
Date: Wed, 14 May 2025 13:30:48 -0700
Message-ID: <CAAeiqZ3BPCXziob2-Ldf15h0eS-0C6qbNoT3n5jiXEvMrjEW-w@mail.gmail.com> (raw)
In-Reply-To: <aCS8ZHR_6Py8sMzc@nathan>
References: <Z-3x2AnPCP331JA3@nathan>
<[email protected]>
<Z-9Bx3ml2i7OfHiN@nathan>
<Z_A5hrD4d8xbsxQB@nathan>
<Z_A71fHM6Z3ZgfUA@nathan>
<Z_BIDYMTVksxZpLr@nathan>
<Z_BcWVMvlUIJ_iuZ@nathan>
<CADkLM=dMXt_=eeZbe=Kzx1bTNihmOar-1pyBfoe6iOoEWXsU2Q@mail.gmail.com>
<Z_CQLseJCVRwTXCi@nathan>
<CAAeiqZ0o2p4SX5_xPcuAbbsmXjg6MJLNuPYSLUjC=Wh-VeW64A@mail.gmail.com>
<aCS8ZHR_6Py8sMzc@nathan>
Thanks Nathan.
Here is the patch with a comment.
On Wed, May 14, 2025 at 8:53 AM Nathan Bossart <[email protected]>
wrote:
> On Tue, May 13, 2025 at 05:01:02PM -0700, Hari Krishna Sunder wrote:
> > We found a minor issue when testing statistics import with upgrading from
> > versions older than v14. (We have VACUUM and ANALYZE disabled)
> > 3d351d916b20534f973eda760cde17d96545d4c4
> > <
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3d351d916b20534f973eda760cde17d96545d...
> >
> > changed
> > the default value for reltuples from 0 to -1. So when such tables are
> > imported they get the pg13 default of 0 which in pg18 is treated
> > as "vacuumed and seen to be empty" instead of "never yet vacuumed". The
> > planner then proceeds to pick seq scans even if there are indexes for
> these
> > tables.
> > This is a very narrow edge case and the next VACUUM or ANALYZE will fix
> it
> > but the perf of these tables immediately after the upgrade is
> considerably
> > affected.
>
> There was a similar report for vacuumdb's new --missing-stats-only option.
> We fixed that in commit 9879105 by removing the check for reltuples != 0,
> which means that --missing-stats-only will process empty tables.
>
> > Can we instead use -1 if the version is older than 14, and reltuples is
> 0?
> > This will have the unintended consequence of treating a truly empty table
> > as "never yet vacuumed", but that should be fine as empty tables are
> going
> > to be fast regardless of the plan picked.
>
> I'm inclined to agree that we should do this. Even if it's much more
> likely that 0 means empty versus not-yet-processed, the one-time cost of
> processing some empty tables doesn't sound too bad. In any case, since
> this only applies to upgrades from <v14, that trade-off should dissipate
> over time.
>
> > PS: This is my first patch, so apologies for any issues with the patch.
>
> It needs a comment, but otherwise it looks generally reasonable to me after
> a quick glance.
>
> --
> nathan
>
Attachments:
[application/octet-stream] 0001-Stats-import-Fix-default-reltuples-on-versions-older.patch (1.4K, ../CAAeiqZ3BPCXziob2-Ldf15h0eS-0C6qbNoT3n5jiXEvMrjEW-w@mail.gmail.com/3-0001-Stats-import-Fix-default-reltuples-on-versions-older.patch)
download | inline diff:
From f3024a4b50af63f61fb91a82e7b91c98f9bf882d Mon Sep 17 00:00:00 2001
From: Hari Krishna Sunder <[email protected]>
Date: Tue, 13 May 2025 23:26:32 +0000
Subject: [PATCH] Stats import: Fix default reltuples on versions older than 14
---
src/bin/pg_dump/pg_dump.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e2e7975b34e..45548004240 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -10924,7 +10924,17 @@ dumpRelationStats_dumper(Archive *fout, const void *userArg, const TocEntry *te)
appendStringLiteralAH(out, rsinfo->dobj.name, fout);
appendPQExpBufferStr(out, ",\n");
appendPQExpBuffer(out, "\t'relpages', '%d'::integer,\n", rsinfo->relpages);
- appendPQExpBuffer(out, "\t'reltuples', '%s'::real,\n", rsinfo->reltuples);
+
+ /*
+ * Before version 14, the default value for reltuples of tables that had not yet been ANALYZED
+ * was 0. In version 14, the default value was changed to -1. Even if the table is empty, let's
+ * just assume it has not yet been ANALYZED and set to -1.
+ */
+ if (fout->remoteVersion < 140000 && strcmp("0", rsinfo->reltuples) == 0)
+ appendPQExpBufferStr(out, "\t'reltuples', '-1'::real,\n");
+ else
+ appendPQExpBuffer(out, "\t'reltuples', '%s'::real,\n", rsinfo->reltuples);
+
appendPQExpBuffer(out, "\t'relallvisible', '%d'::integer",
rsinfo->relallvisible);
--
2.26.0
view thread (88+ 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], [email protected], [email protected], [email protected]
Subject: Re: Statistics Import and Export
In-Reply-To: <CAAeiqZ3BPCXziob2-Ldf15h0eS-0C6qbNoT3n5jiXEvMrjEW-w@mail.gmail.com>
* 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