Re: Extended Statistics set/restore/clear functions.

On Tue, Jan 20, 2026 at 03:48:01PM -0500, Corey Huinker wrote:
> Indeed, I've identified 29 WARNINGs that need direct coverage, and that's
> not counting the ndistinct and dependencies, though for those I think we
> can limit ourselves to testing ways in which those values can fail the
> validation against the extended stats object rather than covering all the
> ways that a value can be invalid in a vacuum.

Yeah, that should be OK for these two. We should sufficient have
coverage for trash input based on the definition of the extstat
object, the input functions dealing with most of the details.

> In doing so, I noticed that we have two ways of handling some of this
> warning-waterfall.
>
> The first is to have a function call that emits some warnings and flips a
> boolean:
>
> datum = some_function(input, input, &ok);
> if (!ok)
> ...
>
> And the second is to directly test the datum, which is fine because the
> expected datum output is never null:
>
> datum = some_function(input, input);
> if (datum == (Datum) 0)
> ...
>
> There seems to be more precedent for the second pattern, but I thought I'd
> bring it up for discussion before switching to that pattern wherever
> possible.

Hmm. Not sure yet which one is better here, TBH. The "ok" pattern
feels a but better to me because a Datum == 0 could also be fine in
some cases, be they existing of future cases, so that's more flexible
by design.

>> I am wondering if we should not cut the pie into two parts here: the
>> dependencies and ndistinct data is actually so much more
>> straight-forward as they have predictive input patterns that we can
>> discard easiy if they are incorrect. The MCV list part with its
>> import logic and cross-validation of the inputs is something else
>> based on the attribute types. I don't think that we are far from a
>> good solution, still that would be one strategy to get something done
>> if we cannot get the MCV part completely right.
>
> I'm not sure what cutting the pie would mean in terms of code
> reorganization, so I can't comment on whether I think it's a good idea. For
> the time being I'm going to add the tests in stats_import.sql, and if it
> gets unweildly then we can consider splitting that.

When I review a patch and that I want to do a clean split, I tend to
apply the whole, then remove the parts that can be split out, reapply,
and then generate a diff of the previously-removed code. It's just
easier to delete code. For this patch set, it would mean removing the
MVC variables and related structures. It is actually not that bad
based on how you have structured the restore function; there is no
strong dependency between each code path treating each stxkind. This
is just thinking of possible strategy if this whole thing cannot make
it into the tree by the freeze deadline. Some of it may be better
than nothing of it, and it is possible to add more parameters later
with the right basics in place. For me, review and integration tends
to be easier as a incremental set of small-ish steps, but I think that
you are aware of my way of doing things :D
--
Michael