Re: Remove fcinfo from statistics update internal functions

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Remove fcinfo from statistics update internal functions
Date: 2026-09-02 17:21:25
Message-ID: CADkLM=d=pc2Nj3C=WojUy4W9A+RD4wAzWNeF4zUDbqF97GBjNA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
> Also, we have exported import_attribute_statistics() in v19, so we
> should avoid changing its signature. So I think the second patch
> should be reverted at least. Sorry, but this isn't that trivial a
> change, so I think you should have taken more time (at least a few
> days) for others to look at it.
>

I remembered this hours after sending my message, and I agree with the
concern.

> > With that, we should be able to bypass the positional issues, as well
> > as the fact that some of the parameters are not used, while cleaning
> > up the FDW-side import functions and all their arguments. The point
> > is where to make the cut due to the pairing of the arguments from the
> > fcinfos in the restore functions, but that's doable.
>
> My way of thinking is the exact opposite of yours (and Corey's): I
> think it's better to handle both restore and FDW cases in a unified
> way, rather than complicating the code for the latter case, which also
> makes the version diff large and thus makes back-patching hard.
>

I'm actually going to carve out a third way of thinking here. I think we
should settle on the FDW API now, and we can fix the internals in v20.

The existing v19 FDW has an unattractive number of parameters, but that
becomes a strength in situations where new stat types are added: a missing
parameter is always a compile error, whereas uninitialized struct values
are not, and undersized arrays passed into a structure are similarly
dangerous. This strength is more important here because the programmer
likely using that API is an extension writer who will likely compiling that
extension across multiple pg-versions, and wouldn't likely notice that an
array got one element longer from v22 to v23, or this struct had a member
added from v21 to v22. So I would rank them as mass-of-ugly-parameters >
pointer to big struct > array. `They will notice a compile error, though,
and that avoids a lot of POLA-violations for our consumers.

Internally, the calculus is different, and the pointers to a struct feel
cleaner and more self documenting, and also serves to alleviate my
discomfort over using regular strings over constants or defines, as the
string being used is always the struct member name in quotes.

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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tristan Partin 2026-09-02 17:35:09 Re: glist: _Generic wrapper for selective dlist/dclist usage
Previous Message Corey Huinker 2026-09-02 16:52:28 Re: Remove fcinfo from statistics update internal functions