Re: BUG #19849: queries fail after ANALYZE with expression stats on arrays

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> These two could also be passed down as arguments of your new
> fill_attstats_values(), filling them within the routine instead of
> letting the two callers do it.
> + * values[] and nulls[] must have room for Natts_pg_statistic entries. All
> One thing that always stresses me with this kind of assumption is that
> an error means an out-of-bound write, and a CVE. Here, we could also
> use a palloc0'ed area worth Natts_pg_statistic, instead of expecting
> the caller to do the right thing. The ANALYZE and expression stats
> code paths are not performance-critical. Or ubsan would just catch
> any problems?

Yeah, I didn't love this particular refactorization either. I think
it'd be nicer to have the values[] and nulls[] arrays totally inside
the shared function and do the tuple-building within it, as attached.

One thing that bugs me slightly about this is that it seems weird to
be passing starelid and stainherit explicitly while digging staattnum
out of the VacAttrStats struct. It works, because examine_expression
is careful to set stats->tupattnum = InvalidAttrNumber, but maybe
it would be cleaner to pass all three parts of the primary key as
explicit arguments. Does anyone else feel that way?

regards, tom lane