Re: Add expressions to pg_restore_extended_stats()

On Fri, Feb 06, 2026 at 02:55:47AM -0500, Corey Huinker wrote:
>> If
>> my test of the patch is right, the restore function accepts this case,
>> and decides to set the following fields:
>> "avg_width": "0",
>> "null_frac": "0",
>> "n_distinct": "0"
>
> statatt_get_empty_stat_tuple is just assigning the defaults.

Okay. I have looked at the default assignment path, and I don't have
an issue with that after thinking more about it.

> Any explicit null parameters would be handled by the loop that fills out
> the found[] and val[] arrays, and explicit jbvNull is treated the same as
> if the parameter wasn't specified in the first place. So I think the null
> checks you seek are already there, but the existence of defaults in the
> pg_statistic tuple (which has no nullable columns, btw) is throwing you off.

Right, I can see that. As far as my review has brought me, there was
a case that was not checked: passing down a non-null, non-string value
was not tested. Not a big issue, but still.

import_pg_statistic() could be tricked to trigger the assertion based
on JsonContainerIsObject(), for example with test_stat_clone in the
regression tests:
[null, [{"avg_width" : [1]}]]
I have added a test for that, changing the code inside
import_pg_statistic() to emit a WARNING. This seemed simpler than
touching more elem->val.binary.data in import_expressions().

Here is another one: [1, null]. This triggers the default clause for
the loop in import_expressions(). This did not fail, but it was
uncovered.

In import_pg_statistic(), there should be some coverage for the case
where a value is not a string or a NULL, like { "null_frac": 1 }.
Added one with an integer.

By the way, the test you have proposed in [1] to copy the expression
stats from an origin to a target and check the differences was super
nice. Could you add that back, with 'exprs' set as a jsonb blob?
This was posted in 0003, but got cut due to the initial integration of
the feature without expressions.

I have put my hands on the patch, and fixed all these issues, adding
tests to cover the holes I have spotted, and addressed a lot of
stylistic things. I did not take the time to add the diff test,
though when the expressions are cloned. Could you?

The cases of STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM and
STATISTIC_KIND_BOUNDS_HISTOGRAM are a bit sad. How about exposing
them into pg_stats_ext_exprs as a first patch, then extend the
expression restore function to be able to work on them? Modifying the
view would be an entirely independent thing. Please note that I do
not mind skipping these two cases for now, the proposed patch is
already doing a lot. Still if we are able to close entirely the loop
for this release that would be nice. Anyway, let's keep this stuff as
separate patches on top of this v8 and future reviewed versions.

[1]: https://www.postgresql.org/message-id/CADkLM%3DdgvTvuOy%2BZOWQX51gGtLgAmQqqZw7Oi74tWrQ542_bvQ%40mail.gmail.com
--
Michael