Re: pg_restore_attribute_stats() accepts non-finite values

On Tue, Aug 25, 2026 at 7:36 AM Ewan Young <kdbase(dot)hack(at)gmail(dot)com> wrote:

> Hi,
>
> 7cb9060dcde taught pg_restore_relation_stats() to reject a non-finite
> reltuples, on the grounds that Infinity and NaN pass the existing range
> check and then get stored and used verbatim. pg_restore_attribute_stats()
> has the same gap for its float arguments, which that thread did not cover.
>
> CREATE TABLE t (a int);
> INSERT INTO t SELECT g FROM generate_series(1, 1000) g;
> ANALYZE t;
>
> SELECT pg_restore_attribute_stats('schemaname', 'public', 'relname',
> 't',
> 'attname', 'a', 'inherited', false,
> 'null_frac', 'NaN'::real, 'n_distinct', 'Infinity'::real);
> t
>
> SELECT stanullfrac, stadistinct FROM pg_statistic
> WHERE starelid = 't'::regclass;
> NaN | Infinity
>
> The values are stored, and the planner does not defend against them.
> CLAMP_PROBABILITY() is two comparisons, both false for NaN, so it does not
> neutralise a non-finite value the way it clamps an out-of-range finite one.
> The effect is visible immediately:
>
> -- with the NaN null_frac above:
> EXPLAIN SELECT * FROM t WHERE a = 5;
> Seq Scan on t (cost=0.00..17.50 rows=10000000000...000 width=4)
>
> A non-finite n_distinct or correlation is worse than a bad row estimate: a
> NaN correlation puts a literal "cost=0.29..NaN" on an index scan, which
> then takes part in path cost comparisons.
>
> This only comes in through the restore path -- ANALYZE never produces a
> non-finite value, even for a column that itself contains Infinity/NaN,
> since the stats are frequencies and ratios rather than the data. So the
> realistic trigger is a corrupt or cross-version dump fed through
> pg_restore_attribute_stats(), and once stored the value survives until the
> next ANALYZE.
>
> Patch attached. It rejects non-finite values for the scalar arguments
> null_frac, n_distinct, correlation and range_empty_frac, and for the
> float4[] arguments most_common_freqs, most_common_elem_freqs and
> elem_count_histogram, dropping the bad value with a WARNING as the other
> non-fatal checks do and letting the rest of the import proceed. The two
> new checks live in stat_utils.c alongside the existing ones.
>
> Two things I decided deliberately, happy to be overruled:
>
> - A negative n_distinct encodes a distinct-value ratio rather than a
> count, so it is still accepted, matching the -1.0 special case kept for
> reltuples.
>
> - These functions do only superficial validation by design (per
> ce207d2a790), so I did not add range checks for finite-but-bogus values;
> the planner does clamp those. This only closes the non-finite hole,
> which the planner cannot.
>
> --
> Regards,
> Ewan Young
>

Back when this was being developed, there were extremely tight proposed
checks [1] on all parameters.

At the time, the need for that was questioned [2], though the relevant
comment was focusing on the array parameters.

> I'm dubious that we can fully vet the contents of these arrays,
> and even a little dubious that we need to try. As an example,
> what's the worst that's going to happen if a histogram array isn't
> sorted precisely? You might get bogus selectivity estimates
> from the planner, but that's no worse than you would've got with
> no stats at all.
...
> We do need to verify data types, lack of nulls, and maybe
> 1-dimensional-ness, which could break the accessing code at a fairly
> low level; but I'm not sure that we need more than that.

Later, it was suggested that leaving such checks out was a form of fuzzing
tool [3].

> It could be argued that feeding bogus data to the planner for testing
> purposes is a valid use-case for this feature. (Of course, as
> superuser we could inject bogus data into pg_statistic manually,
> so it's not necessary to have this feature for that purpose.)
> I guess I'm a great deal more sanguine than other people about the
> planner's ability to tolerate inconsistent data; but in any case
> I don't have a lot of faith in relying on checks in
> pg_set_attribute_stats to substitute for that ability. That idea
> mainly leads to having a whole lot of code that has to be kept in
> sync with other code that's far away from it and probably isn't
> coded in a parallel fashion either.

The net result was I removed most of the proposed data validation checks.
So every time we add one such check in (or back in, depending on your
perspective), we need to balance the value of the check vs the burden of
the code sync that goes with it. We're clearly on a trajectory for
re-adding checks like this, so I'm in favor of a patch like this one.

As for the patch itself, there's currently a thread [4] proposing a change
from FunctionCallInfo to NullableDatum[] for the stats args, so this patch
would have to be coordinated with that.

I like the stats_check_arg_finite() and how it is used.

I'm less happy with the change to stats_check_arg_array(), specifically
adding the is_finite check based on whether it happens to be float4 or not,
rather than whether we know we need it. Currently those two things are in
sync, but they may not be in the future. I'm especially concerned about
future stat types covering values of user-defined datatypes, which store as
an ANYARRAY which would then conditionally execute based on the datatype
the user had chosen. I grant you that's a weird hypothetical, but it would
result in very POLA-violating behavior. Maybe the better thing is to have a
separate check.

Another concern about the array value testing is that if we're walking back
the suggestion made in [2], then do we also bring back things like making
sure that the frequency arrays are monotonically non-increasing? We will
need some sort of consensus on where to draw the new line.

The test cases seem sufficient for the time being.

--

[1]
https://www.postgresql.org/message-id/CADkLM=e=_6dtacmrvd2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com

[2] https://www.postgresql.org/message-id/790773.1711910899@sss.pgh.pa.us

[3] https://www.postgresql.org/message-id/864345.1711932452@sss.pgh.pa.us
note: the quote cites a function named pg_set_attribute_stats which was
eventually renamed to pg_restore_attribute_stats

[4]
https://www.postgresql.org/message-id/flat/CADkLM%3Deo7MtuCE%3DYjovW%2B%3DASw1%3Dq39qQ3qarrsw%2BEKfU901ztA%40mail.gmail.com