Re: Remove fcinfo from statistics update internal functions

>
> I still think that using a set of two structures to pass all the
> information is the cleanest way forward, for the following reasons:
> - Previous patch 0001 tried to reuse the same structures as the
> restore APIs, with some of the parameters getting unused. This
> creates a weird design layer, because the import code begins to rely
> on portions it does not care about.
> - The dependencies of the StatsInfo structures used by the restore
> functions become more integrated with the import code, but I doubt we
> want that: the import code cares only about the values, knowing
> already about OIDs of the objects to manipulate in the catalogs.
> - We finish with APIs that are weaker to argument reordering. Values
> in structures are self-documented.
> - A suspicion: long-term maintainability cost.
>

The argument reordering argument is a good one.

>
> We are never going to add a new stats value in a import APIs on a
> stable branch, as it touches the shape of the catalogs, but this kind
>

That wasn't my concern. My concern was that somebody implements these
functions for their own FDW extension, and then when Postgres N+1 comes out
we've added a new stats type, but they don't notice because their code
written for version N still compiles. The initializer macro below does
alleviate my concern a bit.

> And then use this initializer for the defined structures, for both
> the restore *and* the import code.
>

Sounds good so far.

>
> At this point, it would be impossible to miss that the initialization
> macros need to be updated when adding a new field, and extensions can
> use that to deal with their own choices. That also takes care of
> enforcing a cleaner default for the restore code, something we do not
> do now, even if we don't add stats fields very often (the last
> instance was around v11, isn't it?).

There's adding fields to pg_statistic, which we haven't done in a while,
but the stats import is leveraged through pg_stats, and that changed as
recently as bc3c8db8ae2f (2023) when we exposed range_length_histogram and
range_bounds_histogram. The fact that it doesn't happen all that often
makes the change all the more likely to be overlooked.

> > The internal bit can wait for v20, as was always the plan. If I had
> thought
> > it wasn't the plan, I would have kept up the work on this thru July. The
> > FDW API, however, I think we need to decide now. I'd be ok with going to
> > the struct pointers in the FDW API, but we're creating a hassle for
> > ourselves if we decide to do that down the road.
>
> 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.
>

Does that mean you're in favor of pushing the structs and postgres_fdw API
change to v19 then?