Re: BUG #19857: Stats restore rejects empty arrays, losing range and array stats

On Tue, Nov 10, 2026 at 08:50:39PM -0500, Corey Huinker wrote:
> Same patch, commit message updated to have both discussion threads.

arr = DatumGetArrayTypeP(PG_GETARG_DATUM(argnum));

- if (ARR_NDIM(arr) != 1)
+ if (ARR_NDIM(arr) > 1)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),

This change would trigger a CVE, I suspect.. Under valgrind, and for
example these sequences:
CREATE TABLE t (a int);
-- most_common_freqs
SELECT pg_restore_attribute_stats(
'schemaname', 'public', 'relname', 't', 'attname', 'a',
'inherited', false,
'most_common_vals', '{}'::text,
'most_common_freqs', '{}'::real[]);
EXPLAIN SELECT * FROM t WHERE a = 1;

CREATE TABLE t2 (b int[]);
-- most_common_elem_freqs
SELECT pg_restore_attribute_stats(
'schemaname', 'public', 'relname', 't2', 'attname', 'b',
'inherited', false,
'most_common_elems', '{}'::text,
'most_common_elem_freqs', '{}'::real[]);
EXPLAIN SELECT * FROM t2 WHERE b @> '{1}';
-- elem_count_histogram
SELECT pg_clear_attribute_stats('public', 't2', 'b', false);
SELECT pg_restore_attribute_stats(
'schemaname', 'public', 'relname', 't2', 'attname', 'b',
'inherited', false,
'elem_count_histogram', '{}'::real[]);
EXPLAIN SELECT * FROM t2, unnest(b);

Then get_attstatsslot() does an out-of-bound read for
most_common_freqs, most_common_elem_freqs and elem_count_histogram
because an array with 0 dimension has no follow-up data as far as I
can see, giving this pattern:
==804366== Invalid read of size 4
==804366== at 0x1A58924: get_attstatsslot (lsyscache.c:3636)
==804366== by 0x1950B6C: var_eq_const (selfuncs.c:422)
==804366== by 0x195063F: eqsel_internal (selfuncs.c:349)
==804366== by 0x1950297: eqsel (selfuncs.c:302)
==804366== by 0x1ACE474: FunctionCall4Coll (fmgr.c:1215)
==804366== by 0x1AD0C37: OidFunctionCall4Coll (fmgr.c:1451)
==804366== by 0x122CC7F: restriction_selectivity (plancat.c:2242)
==804366== by 0x1063476: clause_selectivity_ext (clausesel.c:854)
==804366== by 0x1060B84: clauselist_selectivity_ext (clausesel.c:136)
==804366== by 0x1060AB1: clauselist_selectivity (clausesel.c:106)
==804366== by 0x10823BD: set_baserel_size_estimates (costsize.c:5615)
==804366== by 0x104B64A: set_plain_rel_size (allpaths.c:641)

Instead of allowing all these arrays to be empty, we should snipe at
range_length_histogram *and* most_common_elems, and only all these
two without generalizing the rule. The other three are rejected
anyway at load, so blocking them at restore would be a consistency
move.

Perhaps we should update get_attstatsslot() as well to avoid the
over-read issue to begin with; that feels like a sloppy thing to do if
there is an empty array to process.

We'll likely need to do something similar for the extstats. One step
at a time. We could of course do a more relaxing approach like you
are proposing, but I cannot really get excited to let that go through
at restore time as the loads would fail even if get_attstatsslot() is
adjusted (actually this path should be adjusted anyway), and the
option-basis thing is not that invasive in the attached, at least it
seems so to me.

Attached is a cleaned-up patch. So, thoughts or comments?
--
Michael