Re: Extended Statistics set/restore/clear functions.

On Tue, Nov 25, 2025 at 11:14 PM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
wrote:

>
>> I don’t see any of my comments are addressed in v18.
>>
>>
> My apologies. My v17 focused entirely on the input functions, as those
> were receiving the vast majority of the attention. Now that those are out
> of the way (Thanks Michael!) I can address those issues.
>

Paraphrasing the "quotes" here for brevity...

> Several functions are made external visible, they are all renamed with
adding a prefix “statatt_”, why text_to_stavalues is an exception?

Michael had specifically said that one didn't need to be renamed. I suppose
statatt_import_stavalues() might be a good name for it. It *is* specific to
attribute stats, though that definition also applies to the attribute stats
nested in the stxdexprs of extended stats. I have no strong opinion on the
matter.

> This MVDependency * can be const.

+1

> static void
> upsert_pg_statistic_ext_data(Datum *values, bool *nulls, bool *replaces)
> {
> ```

> This function pass values, nulls and replaces to heap_modify_tuple() and
heap_form_tuple(),
> the both functions take all const pointers as parameters.
> So, here values, nulls and replaces can all be const.
I find your argument here persuasive enough to override Michael's
previously stated non-excitement.

> ...the two NULL_FRAC questions

Yes, fixed.

> import_mcvlist is declared twice, looks like a copy-paste mistake.

That or a rebase/apply gone wrong. Fixed.

> PREPQUERY_DUMPEXTSTATSSTATS weird, suggest PREPQUERY_DUMPEXTSTATSDATA

Awkward names are almost inevitable when talking about the statistics
associated with an object of type "statistics".

There was a debate about whether statistics were data or not, and I'd
rather not restart that, so I went with PREPQUERY_DUMPEXTSTATSOBJSTATS for
now.

Incorporated these fixes, and some other lessons learned.