pg_restore_attribute_stats() accepts non-finite values

On Tue, Aug 25, 2026 at 7:36 AM Ewan Young <kdbase.hack@gmail.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]/messages/by-id/CADkLM=e=_6dtacmrvd2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com on all parameters.

At the time, the need for that was questioned [2]/messages/by-id/790773.1711910899@sss.pgh.pa.us, 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]/messages/by-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.

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]/messages/by-id/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com 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]/messages/by-id/790773.1711910899@sss.pgh.pa.us, 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]: /messages/by-id/CADkLM=e=_6dtacmrvd2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com
/messages/by-id/CADkLM=e=_6dtacmrvd2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com

[2]: /messages/by-id/790773.1711910899@sss.pgh.pa.us

[3]: /messages/by-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
note: the quote cites a function named pg_set_attribute_stats which was
eventually renamed to pg_restore_attribute_stats

[4]: /messages/by-id/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com
/messages/by-id/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com

Thanks for the thorough review, and for the history -- that context on why
the checks were removed is helpful.

On Thu, Aug 27, 2026 at 5:21 AM Corey Huinker <corey.huinker@gmail.com> wrote:

On Tue, Aug 25, 2026 at 7:36 AM Ewan Young <kdbase.hack@gmail.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.

Happy to rebase onto that whenever it lands -- the finite checks are a
mechanical conversion (PG_ARGISNULL(n) -> args[n].isnull, PG_GETARG_DATUM(n)
-> args[n].value). Since you're driving both, I'll follow whatever order
you prefer.

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.

Agreed, and done in v2. stats_check_arg_array() is back to being a purely
structural check (1-D, no NULLs). The finiteness test now lives in a
separate stats_check_arg_array_finite(), which is called explicitly for the
three arguments that must be finite -- most_common_freqs,
most_common_elem_freqs and elem_count_histogram -- rather than being keyed
on the element type. So the check is driven by "we know this argument is a
frequency/count array that has to be finite", not by "this array happens to
be float4". A future stat type that passes user data values through an
ANYARRAY won't accidentally pick it up, even if the user's type is float4.

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.

I'd keep this patch to finiteness only, and leave monotonicity for the
consensus discussion. They're different in kind: a NaN/infinity isn't
neutralized by CLAMP_PROBABILITY() and propagates into selectivity and cost
estimates, and ANALYZE never emits one, so rejecting it is unambiguous. A
non-monotonic histogram is exactly the "bogus selectivity, no worse than no
stats" case from [2] -- softer, and more of a policy call about where the
new line goes. Happy to help draw that line separately.

The test cases seem sufficient for the time being.

--

[1] /messages/by-id/CADkLM=e=_6dtacmrvd2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com

[2] /messages/by-id/790773.1711910899@sss.pgh.pa.us

[3] /messages/by-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] /messages/by-id/CADkLM=eo7MtuCE=YjovW+=ASw1=q39qQ3qarrsw+EKfU901ztA@mail.gmail.com

--
Regards,
Ewan Young

On Thu, Aug 27, 2026 at 04:09:21PM +0800, Ewan Young wrote:

Thanks for the thorough review, and for the history -- that context on why
the checks were removed is helpful.

Question: do we get elog(ERROR) problems, assertion failures or
backend breakages when we insert these values or is the backend OK
with them?
--
Michael

On Mon, Aug 31, 2026 at 3:57 PM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Aug 27, 2026 at 04:09:21PM +0800, Ewan Young wrote:

Thanks for the thorough review, and for the history -- that context on why
the checks were removed is helpful.

Question: do we get elog(ERROR) problems, assertion failures or
backend breakages when we insert these values or is the backend OK
with them?

No hard breakage. On an assertion-enabled build of master I injected
NaN and +/-Infinity through every unchecked argument (null_frac,
n_distinct, correlation, most_common_freqs, most_common_elem_freqs,
elem_count_histogram) and ran queries exercising each stat slot
(IS NULL, =, IN, GROUP BY, equijoins, array @>/<@, ORDER BY + LIMIT
over an index): no assertion failures, no elog(ERROR), no crashes.
And since only estimates are affected, query results stay correct.

What the values do poison is the cost model, in two distinct ways:

1. NaN probabilities sail through CLAMP_PROBABILITY (both of its
comparisons are false for NaN); the NaN selectivity then hits
clamp_row_est(), whose isnan() guard turns it into
MAXIMUM_ROWCOUNT. With null_frac = NaN:

Seq Scan on tf (cost=0.00..20.00 rows=1e100 width=47)
Filter: (a IS NULL)

(rows is printed as the full 101-digit integer), and every join or
aggregate above such a scan now plans against 1e100 rows. The
same happens for = / IN / join selectivity when most_common_freqs
contains NaN. +/-Infinity is tamer here, since Inf > 1.0 is true
and CLAMP_PROBABILITY catches it.

2. NaN correlation flows into the index-scan cost arithmetic
unclamped, producing paths whose cost is literally NaN:

Index Scan using tf_a_idx on tf (cost=0.28..NaN rows=889 ...)

Every comparison involving a NaN cost is false, so path cost
comparisons degenerate and the chosen plan is essentially
arbitrary. A NaN also propagates up through the whole plan tree
(Limit/GroupAggregate above it print cost=..NaN too).

There is no self-healing: the values sit in pg_statistic until some
later ANALYZE happens to overwrite them.

So the damage class is the same as the reltuples case fixed by
7cb9060dcde: nothing crashes, but it's stored garbage the planner has
no defense against, and rejecting it at import time seems much
cheaper than teaching every consumer of pg_statistic to cope with
non-finite inputs.

--
Michael

--
Regards,
Ewan Young