From 4964884b5aeb74026defdc92bed7d22abdbae619 Mon Sep 17 00:00:00 2001 From: Ewan Young Date: Wed, 26 Aug 2026 00:07:55 +0800 Subject: [PATCH v1] Reject non-finite values when restoring attribute statistics pg_restore_attribute_stats() accepted NaN and Infinity for its float arguments and stored them verbatim in pg_statistic. Statistics produced by ANALYZE are always finite, and the planner does not defend against non-finite ones: CLAMP_PROBABILITY() is a pair of comparisons that are both false for NaN, so a NaN null_frac or n_distinct propagates into selectivity estimates (row counts blow up to the clamp ceiling), and a NaN correlation produces a literal "cost=...NaN" in an index scan, which corrupts path cost comparisons. pg_restore_relation_stats() already rejects a non-finite reltuples (7cb9060dcde); this extends the same treatment to the attribute-stats entry points, which the earlier work did not cover. The scalar arguments null_frac, n_distinct, correlation and range_empty_frac, and the float4[] arguments most_common_freqs, most_common_elem_freqs and elem_count_histogram are checked; a bad value is dropped with a WARNING, as for the other non-fatal checks, and the rest of the import proceeds. Negative n_distinct values, which encode a distinct-value ratio rather than a count, are still accepted, matching the -1.0 special case kept for reltuples. These functions perform only superficial validation by design, so this does not attempt to reject out-of-range finite values (which the planner does clamp); it only rejects the non-finite values the planner cannot. --- src/backend/statistics/attribute_stats.c | 32 +++++++ src/backend/statistics/stat_utils.c | 56 +++++++++++- src/include/statistics/stat_utils.h | 2 + src/test/regress/expected/stats_import.out | 101 +++++++++++++++++++++ src/test/regress/sql/stats_import.sql | 62 +++++++++++++ 5 files changed, 252 insertions(+), 1 deletion(-) diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index 16b0dd6a003..feabe839ca3 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -293,6 +293,38 @@ attribute_statistics_update_internal(Oid reloid, result = false; } + /* + * Reject non-finite scalar values. This runs after the pair check above + * so that suppressing a bad range_empty_frac does not make the pair check + * complain that its companion argument is missing. null_frac and + * n_distinct are consumed through PG_ARGISNULL() at storage time, so + * clearing the argument suffices; correlation and range_empty_frac are + * gated by the "do" flags computed above, which must be cleared instead. + */ + if (!stats_check_arg_finite(fcinfo, attarginfo, NULL_FRAC_ARG)) + { + fcinfo->args[NULL_FRAC_ARG].isnull = true; + result = false; + } + + if (!stats_check_arg_finite(fcinfo, attarginfo, N_DISTINCT_ARG)) + { + fcinfo->args[N_DISTINCT_ARG].isnull = true; + result = false; + } + + if (!stats_check_arg_finite(fcinfo, attarginfo, CORRELATION_ARG)) + { + do_correlation = false; + result = false; + } + + if (!stats_check_arg_finite(fcinfo, attarginfo, RANGE_EMPTY_FRAC_ARG)) + { + do_range_length_histogram = false; + result = false; + } + /* derive information from attribute */ statatt_get_type(reloid, attnum, &atttypid, &atttypmod, diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c index 5ff37ef4cf8..85f129ff4ab 100644 --- a/src/backend/statistics/stat_utils.c +++ b/src/backend/statistics/stat_utils.c @@ -16,6 +16,8 @@ #include "postgres.h" +#include + #include "access/htup_details.h" #include "access/relation.h" #include "catalog/index.h" @@ -62,9 +64,43 @@ stats_check_required_arg(FunctionCallInfo fcinfo, arginfo[argnum].argname))); } +/* + * Check that a float argument is either NULL or a finite value. + * + * Statistics produced by ANALYZE are always finite, and non-finite values + * are not neutralized by the planner's CLAMP_PROBABILITY() (which is a pair + * of comparisons, both false for NaN), so they would propagate into + * selectivity and cost estimates. Reject them, in the same non-fatal way as + * the other checks. This mirrors the reltuples handling in + * pg_restore_relation_stats(). + * + * If a problem is found, emit a WARNING, and return false. Otherwise return + * true. + */ +bool +stats_check_arg_finite(FunctionCallInfo fcinfo, + struct StatsArgInfo *arginfo, + int argnum) +{ + if (PG_ARGISNULL(argnum)) + return true; + + if (!isfinite(DatumGetFloat4(PG_GETARG_DATUM(argnum)))) + { + ereport(WARNING, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument \"%s\" must be a finite value", + arginfo[argnum].argname))); + return false; + } + + return true; +} + /* * Check that argument is either NULL or a one dimensional array with no - * NULLs. + * NULLs. A float4[] must additionally contain only finite values; see + * stats_check_arg_finite() for why. * * If a problem is found, emit a WARNING, and return false. Otherwise return * true. @@ -99,6 +135,24 @@ stats_check_arg_array(FunctionCallInfo fcinfo, return false; } + if (ARR_ELEMTYPE(arr) == FLOAT4OID) + { + float4 *elems = (float4 *) ARR_DATA_PTR(arr); + int nelems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr)); + + for (int i = 0; i < nelems; i++) + { + if (!isfinite(elems[i])) + { + ereport(WARNING, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument \"%s\" array must not contain non-finite values", + arginfo[argnum].argname))); + return false; + } + } + } + return true; } diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h index 15e962dbb7c..743bfa4853c 100644 --- a/src/include/statistics/stat_utils.h +++ b/src/include/statistics/stat_utils.h @@ -30,6 +30,8 @@ extern void stats_check_required_arg(FunctionCallInfo fcinfo, int argnum); extern bool stats_check_arg_array(FunctionCallInfo fcinfo, struct StatsArgInfo *arginfo, int argnum); +extern bool stats_check_arg_finite(FunctionCallInfo fcinfo, + struct StatsArgInfo *arginfo, int argnum); extern bool stats_check_arg_pair(FunctionCallInfo fcinfo, struct StatsArgInfo *arginfo, int argnum1, int argnum2); diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out index f3f915e7ed1..15dfb5a7270 100644 --- a/src/test/regress/expected/stats_import.out +++ b/src/test/regress/expected/stats_import.out @@ -718,6 +718,107 @@ SELECT pg_catalog.pg_restore_attribute_stats( 'inherited', NULL::boolean, 'null_frac', 0.1::real); ERROR: argument "inherited" must not be null +-- error: non-finite scalar values are rejected with a WARNING and skipped +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'null_frac', 'NaN'::real); +WARNING: argument "null_frac" must be a finite value + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'n_distinct', 'Infinity'::real); +WARNING: argument "n_distinct" must be a finite value + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'correlation', '-Infinity'::real); +WARNING: argument "correlation" must be a finite value + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'range_empty_frac', 'NaN'::real, + 'range_length_histogram', '{399,499,Infinity}'::text); +WARNING: argument "range_empty_frac" must be a finite value + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +-- error: a non-finite element in any of the float4[] arguments is rejected +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'most_common_vals', '{1,2,3}'::text, + 'most_common_freqs', '{0.1,Infinity,0.2}'::real[]); +WARNING: argument "most_common_freqs" array must not contain non-finite values + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'most_common_elems', '{1,2,3}'::text, + 'most_common_elem_freqs', '{0.1,0.2,NaN}'::real[]); +WARNING: argument "most_common_elem_freqs" array must not contain non-finite values + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'tags', + 'inherited', false::boolean, + 'elem_count_histogram', '{1,1,-Infinity,1}'::real[]); +WARNING: argument "elem_count_histogram" array must not contain non-finite values + pg_restore_attribute_stats +---------------------------- + f +(1 row) + +-- ok: a negative n_distinct is a ratio, not a count, and is still accepted +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'n_distinct', '-0.5'::real); + pg_restore_attribute_stats +---------------------------- + t +(1 row) + -- ok: just the fixed values, with version, no stakinds SELECT pg_catalog.pg_restore_attribute_stats( 'schemaname', 'stats_import', diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql index 334d7d8dc00..80268784170 100644 --- a/src/test/regress/sql/stats_import.sql +++ b/src/test/regress/sql/stats_import.sql @@ -565,6 +565,68 @@ SELECT pg_catalog.pg_restore_attribute_stats( 'inherited', NULL::boolean, 'null_frac', 0.1::real); +-- error: non-finite scalar values are rejected with a WARNING and skipped +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'null_frac', 'NaN'::real); + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'n_distinct', 'Infinity'::real); + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'correlation', '-Infinity'::real); + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'range_empty_frac', 'NaN'::real, + 'range_length_histogram', '{399,499,Infinity}'::text); + +-- error: a non-finite element in any of the float4[] arguments is rejected +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'most_common_vals', '{1,2,3}'::text, + 'most_common_freqs', '{0.1,Infinity,0.2}'::real[]); + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'most_common_elems', '{1,2,3}'::text, + 'most_common_elem_freqs', '{0.1,0.2,NaN}'::real[]); + +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'tags', + 'inherited', false::boolean, + 'elem_count_histogram', '{1,1,-Infinity,1}'::real[]); + +-- ok: a negative n_distinct is a ratio, not a count, and is still accepted +SELECT pg_catalog.pg_restore_attribute_stats( + 'schemaname', 'stats_import', + 'relname', 'test', + 'attname', 'id', + 'inherited', false::boolean, + 'n_distinct', '-0.5'::real); + -- ok: just the fixed values, with version, no stakinds SELECT pg_catalog.pg_restore_attribute_stats( 'schemaname', 'stats_import', -- 2.47.3