Re: pg_class.reltuples can become non-finite and never recovers

On 7/20/26 15:02, Tomas Vondra wrote:
> On 7/20/26 14:40, Jan Nidzwetzki wrote:
>> Hi hackers,
>>
>> We recently tracked down an issue where pg_class.reltuples for a
>> heap relation had become Infinity, which in turn produced
>> cost=Infinity / NaN query plans. While investigating, we found a
>> chain of issues around reltuples that we would like to report, plus
>> two patches that address the parts we consider clear bugs.
>>
>>
>> 1. Uninitialized tuple counts in the parallel GIN build
>> -------------------------------------------------------
>>
>> The root cause is in the parallel GIN build. _gin_parallel_build_main()
>> sets up a fresh GinBuildState in the worker but, unlike ginbuild(),
>> never initializes bs_numtuples or bs_reltuples. bs_numtuples happens to
>> be reset before use, but bs_reltuples is not: the worker accumulates its
>> scanned tuple count into it and publishes it to the leader, which stores
>> it as the heap relation's reltuples via index_update_stats().
>>
>> The value read back is therefore whatever happened to be on the stack,
>> and the store goes through an unchecked narrowing cast:
>>
>> rd_rel->reltuples = (float4) reltuples; /* index.c:2960 */
>>
>> The existing guard in index_update_stats() only rejects reltuples < 0,
>> which a large positive or +Infinity double passes. Any stack residue
>> whose double representation is >= FLT_MAX therefore lands in pg_class as
>> +Infinity; smaller residue lands as a bogus-but-finite count that is
>> unrelated to relpages.
>>
>> This was introduced together with the parallel GIN build:
>>
>> commit 8492feb98f6df3f0f03e84ed56f0d1cbb2ac514c
>> "Allow parallel CREATE INDEX for GIN indexes"
>>
>> which first appeared in v18.
>>
>> Fix in the first attached patch: initialize bs_numtuples and
>> bs_reltuples to 0 in the worker, matching how the leader's state is
>> set up. We recommend back-patching this to 18.
>>
>
> Yeah, this seems like my bug. I guess I got confused regarding what
> fields get initialized, and how the values propagate. I suppose we could
> also just change
>
> state->bs_reltuples += reltuples;
>
> to
>
> state->bs_reltuples = reltuples;
>
> And that'd fix the issue. But I agree it's the explicit initialization
> is better / cleaner.
>
> I'll get this fixed and backpatched.
>

Here's a proposed fix - it's pretty much your patch, except that I
removed the second initialization of the bs_numtuples field (which would
be confusing, IMHO). And a slightly expanded commit message.

There's not much to discuss, it's pretty clear (both the bug / fix).

>>
>> 2. VACUUM can amplify a bad reltuples/relpages ratio to +Infinity
>> -----------------------------------------------------------------
>>
>> Independently of how the bad value first appears, it can get worse
>> during normal operation. Once reltuples/relpages is inconsistent, a
>> partial VACUUM extrapolates the stored density over the unscanned
>> pages in vac_estimate_reltuples():
>>
>> old_density * unscanned_pages + scanned_tuples
>>
>> and the result is again stored through an unchecked (float4) cast in
>> vac_update_relstats(). A merely large-but-finite reltuples can thus
>> flip to +Infinity after a routine VACUUM. We mention this mostly to
>> explain why the value tends to degrade toward Infinity in the field
>> rather than staying at a "just wrong" finite number.
>>
>> This is easy to demonstrate starting from statistics whose individual
>> values are perfectly valid but whose ratio is not: relpages = 1 and
>> reltuples = 3e38 are each finite, yet together they imply an
>> impossible density of 3e38 tuples per page:
>>
>> -+-
>> CREATE TABLE vac_inf (a int);
>> INSERT INTO vac_inf SELECT generate_series(1, 100000);
>> VACUUM ANALYZE vac_inf;
>>
>> SELECT pg_restore_relation_stats(
>> 'schemaname', 'public', 'relname', 'vac_inf',
>> 'version', 190000,
>> 'relpages', 1::integer, -- finite, valid
>> 'reltuples', '3e38'::real); -- finite, valid, near FLT_MAX
>>
>> INSERT INTO vac_inf SELECT generate_series(1, 10000);
>> VACUUM vac_inf;
>>
>> SELECT relpages, reltuples FROM pg_class WHERE relname = 'vac_inf';
>> relpages | reltuples
>> ----------+-----------
>> 487 | Infinity
>> (1 row)
>> -+-
>>
>> The single VACUUM extrapolates the 3e38-per-page density over the
>> newly added pages and overflows float4.
>>
>
> Yeah. Garbage in, garbage out.

Yeah. It's kinda orthogonal to the bug,, but I wonder if there's
something we can do to mitigate this. But it's hard, if we don't know
which of the values is wrong (if any).

I guess we could refuse to update reltuples to non-finite value, but
that doesn't make the existing reltuples value any less bogus. And I'm
not sure it's less harmful than infinity in practice.

>> 3. Infinity defeats autovacuum's self-healing
>> ----------------------------------------------
>>
>> Normally, a wrong reltuples is self-correcting: ANALYZE recomputes it
>> from a fresh sample. But a non-finite reltuples makes the analyze
>> threshold non-finite as well:
>>
>> anlthresh = analyze_threshold + analyze_scale_factor * reltuples
>>
>> so in relation_needs_vacanalyze() the test "anltuples > anlthresh" can
>> never be true, autoanalyze never fires, and the bad value is stuck
>> forever and the relation never recovers on its own.
>>
>> The second attached patch makes autovacuum force an analyze when
>> reltuples is not finite, so a corrupted value heals itself on the next
>> autovacuum cycle. It includes a TAP test that plants an infinite
>> reltuples and checks that autoanalyze restores a finite value.
>>
>
> I'm skeptical about this change. Yeah, it'd probably help in this
> particular Infinity case, but it can be bogus in many other ways. We
> should really be careful to not put bogus values into reltuples.
>

I experimented with this a bit more, but I'm rather skeptical.

I understand the desire to self-heal, but I don't think we do that
elsewhere. We assume the data is correct. I'm sure we had a bunch of
data corruption bugs, and I don't quite see why this particular case
would be any different.

I don't think Infinity/NaN values are somewhat special - sure, they're
defined as special, but those are two singular values. If the stack has
a random value, what's the chance it's Infinity/Nan?

Isn't it more likely it'll be a valid float value? And the consequences
will be almost exactly the same - the threshold may be so high it'll
never be hit, right? But the isfinite() check won't catch that.

So I think we'd need to check the tuple density. The 0002 patch does
that, and it compares it to MaxHeapTuplesPerPage - it we got a higher
density, something has to be wrong and analyze is forced.

But MaxHeapTuplesPerPage is still way higher than normal densities. It's
~290 for 8K pages, and regular densities may be ~50, so an order of
magnitude less. So we still need to mitigate these cases, most likely by
suggesting a manual ANALYZE on some tables in the release notes. Maybe
we should just do that for all cases?

Another possible argument just occurred to me - one of the statistics
import use cases is to allow messing with these fields, to import stats
to simulate a much larger database, etc. Wouldn't this self-healing
potentially interfere with that?

>>
>> 4. Statistics import accepts Infinity and NaN
>> ---------------------------------------------
>>
>> Finally, the reltuples argument of pg_restore_relation_stats() (and the
>> shared relation_statistics_update_internal() path) is only validated
>> with:
>>
>> if (reltuples < -1.0) /* relation_stats.c:126 */
>> {
>> ereport(WARNING, ...);
>> result = false; /* update skipped */
>> }
>>
>> Both Infinity and NaN pass this check and are stored verbatim, while
>> an ordinary out-of-range value like -5 is correctly rejected:
>>
>> -+-
>> CREATE TABLE t();
>>
>> -- returns t, reltuples := Infinity
>> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
>> 'version',190000,'reltuples','Infinity'::real);
>>
>> -- returns t, reltuples := NaN
>> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
>> 'version',190000,'reltuples','NaN'::real);
>>
>> -- WARNING, returns f
>> SELECT pg_restore_relation_stats('schemaname','public','relname','t',
>> 'version',190000,'reltuples','-5'::real);
>> -+-
>>
>> Since pg_dump --statistics / pg_upgrade round-trip reltuples, a
>> corrupted value (from any of the above) also propagates across
>> dump/restore and major-version upgrades.
>>
>> We have not addressed this one in the attached patches, because
>> rejecting non-finite values here is a slightly larger policy question
>> (error vs. clamp-to -1). If there is agreement on the desired
>> behavior, we are glad to propose a patch for this as well.
>>
>
> OTOH this seems like something we might want to do, to validate and
> reject clearly bogus values.
>

No opinion on this. But I was checking how we validate the values when
importing stats, and I noticed relation_statistics_update_internal does
this:

if (reltuples < -1.0)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument \"%s\" must not be less than -1.0",
"reltuples")));
result = false;
}

Isn't that a bit strange it's not (reltuplees < 0.0)?

regards

--
Tomas Vondra