Date: Wed, 14 May 2025 11:53:08 -0400
From: Nathan Bossart <nathandbossart@gmail.com>
To: Hari Krishna Sunder <hari.db.pg@gmail.com>
Cc: Corey Huinker <corey.huinker@gmail.com>, Jeff Davis <pgsql@j-davis.com>,
	Robert Treat <rob@xzilla.net>, Robert Haas <robertmhaas@gmail.com>,
	Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
	Michael Paquier <michael@paquier.xyz>,
	jian he <jian.universality@gmail.com>,
	Bruce Momjian <bruce@momjian.us>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Magnus Hagander <magnus@hagander.net>,
	Stephen Frost <sfrost@snowman.net>,
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	alvherre@alvh.no-ip.org
Subject: Re: Statistics Import and Export
Message-ID: <aCS8ZHR_6Py8sMzc@nathan>
References: <Z-3x2AnPCP331JA3@nathan>
 <8a6905c658ed52418c6f413393390dc43d63a995.camel@j-davis.com>
 <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAAeiqZ0o2p4SX5_xPcuAbbsmXjg6MJLNuPYSLUjC=Wh-VeW64A@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aCS8ZHR_6Py8sMzc%40nathan>
Precedence: bulk

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=3d351d916b20534f973eda760cde17d96545d4c4>
> 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