| From: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
|---|---|
| To: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
| Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
| Subject: | Re: use of SPI by postgresImportForeignStatistics |
| Date: | 2026-08-20 16:28:30 |
| Message-ID: | CADkLM=dvRPdFG83260bOX8UxmBUqjQmEvCfuVDT6nFMyAxm2ig@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Aug 20, 2026 at 7:43 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:
> On Thu, Aug 20, 2026 at 2:15 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
> wrote:
> >> IIUC I think what you are proposing here is:
> >>
> >> * Removing the construction of FCINFO from pg_restore_*_stats/import_*
> functions
> >> * Removing the special handling of the version parameter
> >> * Renaming the argnum enum values
> >>
> >> Am I right? If so, I think these are in the order of necessity, so I
> >> recommend re-splitting the patchset into the three parts in the order.
> >> I lowered the priority of the renaming, because 1) I don't make it a
> >> blocker for others, and 2) I want to see how it improves things on its
> >> own.
> >
> > That's what I'm proposing, but if you're saying that renaming the enums
> is not desirable, then my revised proposal would be to keep the existing
> enum, keep a strict ordering in the enum where all key/version parameters
> come before any actual stats values, thus allowing us to pass the
> stats-only portion of the nullabledatum array to the caller. We'd use an
> offset technique like we do filling out tuple values/isnull/replaces arrays
> where we always index by Anum_something - 1.
> >
> > 1. Remove special handling of version parameter, add it into existing
> StatsArgInfo structures. Not the highest priority, but doing it first
> remove the possibility of having to re-adjust subsequent changes related to
> the offset I'm proposing.
> > 2. Remove construction of of FCINFO, going with passing the latter
> portion of the positional NullableDatum array.
>
> +1. My point is: this is actually a minor issue (that would probably
> be fine as it is), so we should fix it with minimal changes, not to
> make back-patching hard. In other words: if we can't do so, we should
> do nothing about it.
>
> >> Also, as this is an improvement for v20, I recommend creating a new
> >> thread (and a new CF entry) for it.
> >
> > Doing so now. Normally I'd start a new thread, but this one isn't that
> long and carries a lot of context.
>
> Thank you! I think that that will attract more attention from developers.
Oops, forgot to post the commitfest entry:
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-08-20 16:39:50 | Re: Make formatting.c use StringInfos for output buffers |
| Previous Message | Zhijie Hou (Fujitsu) | 2026-08-20 15:56:43 | RE: Logical replication row filter loses unchanged toasted columns |