Re: Remove fcinfo from statistics update internal functions

On Thu, Sep 03, 2026 at 02:35:53PM +0900, Etsuro Fujita wrote:
> How about setting those parameters as well, as we did before, because
> we might use them in *_statistics_update_internal() in the future, as
> I said upthread? IOW: I still don't think it's a good idea to
> complicate the core code just for those parameters in the FDW case. I
> think that that would just make the version diff large and
> back-patching hard.

Hmm. I'm not sure that this is correct with a v19 in mind.

> I think that that is just an appearance problem; we could address it
> by defining NullableDatums for each stats in import_fetched_function()
> like:
>
> NullableDatum version;
> NullableDatum null_frac;
> NullableDatum avg_width;
> NullableDatum n_distinct;
> ...

Not sure that I that much. It does not address my concerns regarding
the import APIs.

> Also, I think it's important for import_*_statistics() to have
> arguments like this that are consistent with the error messages in
> stats-checking functions I mentioned upthread. Using the structures
> doesn't match the messages, so that would degrade developer
> experience.

Ditto.

>> Honestly, that's not a pattern I have seen a lot in the code base; we
>> tend to use structure-based approaches to pass down data rather than a
>> lot of args, because C makes this kind of code written easier to
>> parse and less opaque by assigning values to each member of the
>> structure. One example, the [auto]vacuum code for its state data and
>> its options passed across various stacks.
>
> That's true. However, having such a tendency is no reason to prohibit this.

Disagreed. Consistency is usually good in code style. In terms of
extension experience, the structures feel much easier to work with.
That's less index positioning.

>> What matters the most to me is the long-term maintenance of both code
>> structures, and the more I think about it, the more the structures
>> lead to a so-much-cleaner result. That would be even better with two
>> initializer macros for each structure that extensions can just reuse
>> to force isnull=true for each element, though. This enforces a safer
>> catalog insert policy for the import *and* the restore code.
>
> I don't feel the need for such macros at least for the use of
> import_*_statistics(), because those functions are only used by FDW
> authors, and they reference postgres_fdw when developing their FDWs,
> so I think it's enough to provide a safe use of those functions in
> postgres_fdw (and the documentation for it) even without those macros.

Even with my disagreements, it is pretty clear that you do not like
these improvements, so I'll revert the refactoring piece. I am not
planning to argue much for the FDW part (honestly I don't like the way
this has been coded, and I regret not having spent a couple of cycles
looking at what was happening, but I did not commit that). For the
stats part, though, I am going to be on standby and more careful for
any changes suggested.

Another part that I do not find acceptable is the "unused" arguments
based on the StatsInfo. That's overlapping the import and restore
bits, IMO. That part alone makes the 0001 proposed upthread a no-go
here.
--
Michael