[BUG / PATCH] PG19: VACUUM misestimates reltuples (often 0) after on-access VM setting

Hi hackers,

In PostgreSQL 19 (reproduced on REL_19_STABLE at 48b43081e55 and master),
VACUUM can misestimate pg_class.reltuples -- often setting reltuples = 0
and resetting pg_stat_all_tables.n_live_tup to 0 on a newly populated table
--
if a read-only query runs between loading the table and the first VACUUM.
It
can also corrupt an existing reltuples estimate that was just computed by
ANALYZE.

1. Root cause
-------------
vac_estimate_reltuples() assumes that a relation with no prior tuple-density
estimate (old_rel_tuples = -1, or old_rel_pages = 0) cannot have most of its
pages marked all-visible in the visibility map, and therefore scales the
tuple
density of scanned_pages across total_pages whenever scanned_pages > 1.

Since commits b46e1e54d ("Allow on-access pruning to set pages
all-visible"),
378a216187a ("Set pd_prune_xid on insert"), and 4a2f5533f3c ("Correct
on-access VM setting heuristic"), a read-only query after an ordinary COPY,
INSERT, or CREATE TABLE AS marks all full pages all-visible in the VM before
VACUUM ever runs. When the first VACUUM runs, it skips nearly the entire
relation via the VM and scans only the tail end:

* at most the last, partially filled data page, plus
* any uninitialized (PageIsNew / all-zeroes) pages left behind at the end
of
the relation by bulk extension (up to 64 blocks).
lazy_scan_new_or_empty()
intentionally does not mark PageIsNew pages all-visible in the VM, so
every VACUUM scans them and counts them in vacrel->scanned_pages with 0
tuples.

This produces four failure modes (plus a pgstat side effect):

Mode 1 (Never-vacuumed / unanalyzed table -> reltuples = 0 or tiny
fraction):
When a newly loaded table is read before its first VACUUM, VACUUM skips all
the all-visible data pages and scans only the tail page(s) and any trailing
PageIsNew blocks from bulk extension. vac_estimate_reltuples() then
extrapolates that zero-heavy tail sample across the entire relation, setting
reltuples to 0 (or a tiny fraction of the true row count). Because
old_rel_pages then matches total_pages, subsequent VACUUMs hit the < 2% /
<= 1 page guard and preserve the wrong reltuples value indefinitely.

Mode 2 (Table vacuumed while empty [relpages=0, reltuples=0] -> reltuples =
0
and n_live_tup reset to 0):
If a table was previously vacuumed while empty and is later populated and
read, VACUUM either extrapolates 0 from the PageIsNew tail or hits the
`scanned_pages <= 1` rule and returns `old_rel_tuples` (0). Furthermore,
whenever vac_estimate_reltuples() returns -1 (for instance, when
scanned_pages == 1), heap_vacuum_rel() clamps `Max(vacrel->new_live_tuples,
0)`
when calling pgstat_report_vacuum(), wiping pg_stat_all_tables.n_live_tup
back to 0.

Mode 3 (Table with valid ANALYZE stats -> reltuples drops sharply on
VACUUM):
Even when ANALYZE is run right after a bulk load, ANALYZE includes any
trailing PageIsNew blocks in old_rel_pages when recording old_rel_tuples.
When a subsequent read marks the populated pages all-visible, VACUUM skips
the data pages and scans only the trailing PageIsNew blocks (which contain 0
tuples and can easily exceed 2% of total_pages, defeating the 2% stability
guard). vac_estimate_reltuples() then applies the diluted old_density to
only the unscanned data pages and adds 0 scanned tuples -- effectively
double-counting the empty PageIsNew pages and sharply reducing the reltuples
estimate that ANALYZE just computed.

Mode 4 (Table with valid ANALYZE stats grows -> reltuples stays stale):
When a table with prior statistics grows and a read-only query marks the
newly added pages all-visible before VACUUM runs, VACUUM may scan only a
single page. Because `if (scanned_pages <= 1) return old_rel_tuples;` does
not check whether `old_rel_pages == total_pages`, VACUUM leaves reltuples
unchanged at the pre-growth value instead of scaling by old_density.

2. Minimal reproduction
-----------------------
-- 1. Never-vacuumed table (4,000 rows): reltuples becomes 0 (PG18: 4000)
CREATE TABLE repro_new WITH (autovacuum_enabled = off) AS
SELECT g AS a, repeat('x', 100) AS b FROM generate_series(1, 4000) g;
SELECT count(*) FROM repro_new;
VACUUM repro_new;
SELECT relpages, reltuples FROM pg_class WHERE oid = 'repro_new'::regclass;
-- PG19: relpages = 69, reltuples = 0 (EXPLAIN estimates rows=1)
-- PG18: relpages = 69, reltuples = 4000

-- 2. Empty-vacuumed table: reltuples and n_live_tup both become 0 (PG18:
4000)
CREATE TABLE repro_empty (a int, b text) WITH (autovacuum_enabled = off);
VACUUM repro_empty;
INSERT INTO repro_empty
SELECT g, repeat('x', 100) FROM generate_series(1, 4000) g;
SELECT count(*) FROM repro_empty;
SELECT pg_stat_force_next_flush();
VACUUM repro_empty;
SELECT pg_stat_force_next_flush();
SELECT c.relpages, c.reltuples, s.n_live_tup
FROM pg_class c JOIN pg_stat_all_tables s ON s.relid = c.oid
WHERE c.oid = 'repro_empty'::regclass;
-- PG19: relpages = 69, reltuples = 0, n_live_tup = 0
-- PG18: relpages = 69, reltuples = 4000, n_live_tup = 4000

-- 3. Post-ANALYZE drift: reltuples drops from 4000 to 2156 (PG18: 4000)
CREATE TABLE repro_analyzed WITH (autovacuum_enabled = off) AS
SELECT g AS a, repeat('x', 100) AS b FROM generate_series(1, 4000) g;
ANALYZE repro_analyzed;
SELECT count(*) FROM repro_analyzed;
VACUUM repro_analyzed;
SELECT relpages, reltuples FROM pg_class WHERE oid =
'repro_analyzed'::regclass;
-- PG19: relpages = 69, reltuples = 2156
-- PG18: relpages = 69, reltuples = 4000

3. Proposed fix and design trade-offs
-------------------------------------
The attached patch fixes these issues as follows:

1. Count PageIsNew blocks separately (`vacrel->scanned_new_pages`) in
lazy_scan_new_or_empty() (and contrib/pgstattuple's statapprox_heap())
and
pass `scanned_new_pages` to vac_estimate_reltuples(). Exclude those
uninitialized pages from both the sampled pages (`sampled_pages =
scanned_pages - scanned_new_pages`) and sampleable pages
(`sampleable_pages
= total_pages - scanned_new_pages`), since they are never marked
all-visible and are not representative of the relation's initialized
pages.

2. When prior tuple density is unknown (`old_rel_tuples < 0 ||
old_rel_pages == 0`)
and VACUUM sampled `<= 1` initialized page or `< 2%` of sampleable pages,
return `-1` (unknown) rather than extrapolating from the tail or
returning
`0` for a previously empty relation.

3. Restrict the `sampled_pages <= 1` early return to relations whose size
has
not changed (`old_rel_pages == total_pages`), matching the `< 2%` guard,
so
that relations that grew since the last VACUUM/ANALYZE still scale using
`old_density`.

4. Pass `vacrel->new_live_tuples` directly to pgstat_report_vacuum() and
keep
the existing `tabentry->live_tuples` when `livetuples < 0`, so a VACUUM
that could not estimate live tuples does not wipe `n_live_tup` to 0.

Note on returning -1 vs. PG18 behavior:
When a table has no prior statistics (`reltuples = -1` or `relpages = 0`)
and
on-access pruning marks nearly all pages all-visible before the first
VACUUM,
returning `-1` means `pg_class.reltuples` remains `-1` after that VACUUM
(which vac_estimate_reltuples() already did in PG19 when `scanned_pages <=
1`
and `old_rel_tuples == -1`), whereas in PG18 the first VACUUM always scanned
all pages and set `reltuples` to the exact count. With `relpages > 0` and
`reltuples = -1`, table_block_relation_estimate_size() falls back to its
relpages / tuple-width estimate and autovacuum schedules an ANALYZE (since
`n_live_tup` and `n_mod_since_analyze` are preserved).

Two alternatives could avoid leaving `reltuples = -1` after VACUUM:
(a) Disable VM page-skipping in VACUUM (`vacrel->skipwithvm = false`) when
`rel->rd_rel->reltuples < 0 || rel->rd_rel->relpages == 0`, forcing
the
first VACUUM on an unanalyzed table to scan all pages as in PG18, at
the
cost of giving up the I/O savings of b46e1e54d/378a216187a on the
first
VACUUM when ANALYZE has not run first.
(b) Fall back to pgstat's `live_tuples` count when
vac_estimate_reltuples()
returns `-1`, though that would introduce a dependency from
`pg_class.reltuples` on cumulative stats.

Thoughts on whether returning `-1` (as in the attached patch) is preferred,
or
whether we should also do (a) or (b)? If either (a) or (b) is needed, I can
share separate patches for them.

Thanks,
Vaibhav Jain