Reject ill-formed range bounds histograms in pg_restore_attribute_stats()
Hi,
pg_restore_attribute_stats() (and pg_set_attribute_stats()) stores a
STATISTIC_KIND_BOUNDS_HISTOGRAM verbatim from user input, checking only
that the argument is a one-dimensional, NULL-free array of the right
type. It does not verify the invariant that ANALYZE's
compute_range_stats() always establishes: the bounds histogram must
contain no empty ranges, and its lower and upper bounds must each be
sorted in ascending order.
The range and multirange selectivity estimators rely on that invariant.
They split the histogram into separate lower- and upper-bound histograms
and assume each is ordered. So an imported histogram that violates it
makes the planner misbehave when it later reads the statistics:
- a non-monotonic histogram makes calc_length_hist_frac() fail its
Assert(length2 >= length1) (rangetypes_selfuncs.c, and the identical
multirangetypes_selfuncs.c), i.e. a backend crash on an assert-enabled
build, or a nonsensical, possibly negative, selectivity otherwise;
- an empty range trips the "shouldn't happen" elog(ERROR, "bounds
histogram contains an empty range").
Reproducer on HEAD (assert build):
CREATE TABLE t (r int4range);
INSERT INTO t SELECT int4range(g, g+10) FROM generate_series(1,500) g;
ANALYZE t;
SELECT pg_restore_attribute_stats(
'schemaname', 'public', 'relname', 't',
'attname', 'r', 'inherited', false,
'range_bounds_histogram',
'{"[50,60)","[1,2)","[90,100)","[5,6)"}'::text,
'range_length_histogram', '{1,5,10,20}'::text,
'range_empty_frac', 0.0::real); -- accepted, returns true
EXPLAIN SELECT * FROM t WHERE r <@ int4range(1,100);
-- TRAP: failed Assert("length2 >= length1"), rangetypes_selfuncs.c
The same happens for a multirange column (whose bounds histogram is also
an array of ranges). Setting statistics requires table ownership /
MAINTAIN, so this is not a security issue, but it is another way for a
stats-import call to feed the planner data it cannot cope with, and the
functions already validate other properties and reject bad input with a
WARNING rather than crashing.
This is the same class of problem as commit f6e4ec0a705, which rejected
oversized MCV lists in pg_restore_extended_stats().
The attached patch validates the bounds histogram at import time and
rejects an ill-formed one with a WARNING, matching the treatment of the
other inconsistent inputs. Because the histogram element type is a range
for both range- and multirange-typed columns, the single check covers
both.
--
Regards,
Ewan Young
Attachments:
v1-0001-Reject-ill-formed-range-bounds-histograms-in-pg_rest.patchapplication/octet-stream; name=v1-0001-Reject-ill-formed-range-bounds-histograms-in-pg_rest.patchDownload+134-2
On Tue, Jul 07, 2026 at 04:13:30PM +0800, Ewan Young wrote:
pg_restore_attribute_stats() (and pg_set_attribute_stats()) stores a
STATISTIC_KIND_BOUNDS_HISTOGRAM verbatim from user input, checking only
that the argument is a one-dimensional, NULL-free array of the right
type. It does not verify the invariant that ANALYZE's
compute_range_stats() always establishes: the bounds histogram must
contain no empty ranges, and its lower and upper bounds must each be
sorted in ascending order.
I'll check that tomorrow. Thanks for the report.
--
Michael
On Tue, Jul 07, 2026 at 04:13:30PM +0800, Ewan Young wrote:
The attached patch validates the bounds histogram at import time and
rejects an ill-formed one with a WARNING, matching the treatment of the
other inconsistent inputs. Because the histogram element type is a range
for both range- and multirange-typed columns, the single check covers
both.
I have been looking at that, and while the consequences of buggy
inputs are minor when loaded back, I don't really mind putting more
defenses to inform about that at the front of the restore functions.
Now, your patch is entirely blind about extended statistics; these can
also load histogram bounds. The function you are introducing to
filter the inputs provided could be reused in this secondary case,
just by moving to stat_utils.c.
I am not really convinced that any of this stuff would be worth a
backpatch, even if it's non-invasive. We don't have guards for
buggy infinite values, even if using empty or unordered bounds feels
kind of a stupid thing to do if one is a table owner.
--
Michael
Thanks for looking!
On Wed, Jul 8, 2026 at 10:17 AM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jul 07, 2026 at 04:13:30PM +0800, Ewan Young wrote:
The attached patch validates the bounds histogram at import time and
rejects an ill-formed one with a WARNING, matching the treatment of the
other inconsistent inputs. Because the histogram element type is a range
for both range- and multirange-typed columns, the single check covers
both.I have been looking at that, and while the consequences of buggy
inputs are minor when loaded back, I don't really mind putting more
defenses to inform about that at the front of the restore functions.Now, your patch is entirely blind about extended statistics; these can
also load histogram bounds. The function you are introducing to
filter the inputs provided could be reused in this secondary case,
just by moving to stat_utils.c.
Done in v2. I moved the check to stat_utils.c as stats_check_bounds_histogram()
and now call it from both pg_restore_attribute_stats() and
pg_restore_extended_stats(). In the extended path I reject an ill-formed
histogram the same way the sibling stakinds there already do — WARNING plus
goto pg_statistic_error.
I am not really convinced that any of this stuff would be worth a
backpatch, even if it's non-invasive. We don't have guards for
buggy infinite values, even if using empty or unordered bounds feels
kind of a stupid thing to do if one is a table owner.
Agreed — v2 targets master only, no back-branch patches.
--
Michael
--
Regards,
Ewan Young
Attachments:
v2-0001-Reject-ill-formed-range-bounds-histograms-in-stat.patchapplication/x-patch; name=v2-0001-Reject-ill-formed-range-bounds-histograms-in-stat.patchDownload+183-3
On Wed, Jul 08, 2026 at 12:16:27PM +0800, Ewan Young wrote:
Done in v2. I moved the check to stat_utils.c as stats_check_bounds_histogram()
and now call it from both pg_restore_attribute_stats() and
pg_restore_extended_stats(). In the extended path I reject an ill-formed
histogram the same way the sibling stakinds there already do — WARNING plus
goto pg_statistic_error.
Thanks. I have simplified some of the very-talkative comments,
adjusted one place where array_in_safe() was not mentioned (for
extstats expressions we rely on it for NULLs), tweaked a few more
minor things and applied on HEAD.
Agreed — v2 targets master only, no back-branch patches.
If somebody has an argument to make here, please feel free.
--
Michael