Re: use of SPI by postgresImportForeignStatistics

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, "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-22 10:36:48
Message-ID: CAPmGK17yErcjiDc4Xa7GfqR-ejXoJrxg5fFZ7T=1+Xmf8T26HQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael-san,

On Fri, Aug 21, 2026 at 4:15 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Aug 20, 2026 at 12:28:30PM -0400, Corey Huinker wrote:
> > 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:
> >>> 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.
>
> Just looking at v3-0011, expanding functions like
> attribute_statistics_update_internal() so as these use a gazzilion of
> input arguments is not something I am on board with. That's bug prone
> in the long term. If you wish to eliminate the FCINFO part, which is,
> well, not completely wrong either and does work, I'd suggest to pass
> one pointer to a structure that has access to all these fields. The
> NullableDatum seems to be that, but the patch does not use that so I
> cannot be entirely sure about the benefit of suggestion 2.

We have discussed this upthread, and the consensus is to use a single
NullableDatum array storing all stats values, not such arguments, for
internal functions like attribute_statistics_update_internal(), so I
think he is updating the patchset as such.

> > Oops, forgot to post the commitfest entry:
> >
> > https://commitfest.postgresql.org/patch/7170/
>
> A replacement of newfcinfo by a different facility in
> src/backend/statistics/ has nothing to do with SPI and postgres_fdw.
> I'd suggest to:
> - Rebase (and refactor) the patch.
> - Create a new thread dealing with the specific topic you wish to deal
> with.

+1 for creating the new thread as well.

Best regards,
Etsuro Fujita

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2026-08-22 11:42:47 Re: [POC] Implement async DNS with getaddrinfo_a
Previous Message Andrey Borodin 2026-08-22 10:34:11 Re: GIN page deletion and page recycling bugs