Re: use of SPI by postgresImportForeignStatistics - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: use of SPI by postgresImportForeignStatistics
Date
Msg-id aof7CwOXSNkiMrX-@paquier.xyz
Whole thread
In response to Re: use of SPI by postgresImportForeignStatistics  (Corey Huinker <corey.huinker@gmail.com>)
List pgsql-hackers
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.fujita@gmail.com>
> wrote:
>> On Thu, Aug 20, 2026 at 2:15 AM Corey Huinker <corey.huinker@gmail.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.

I am unconvinced with the value patches 0012 and the follow-up ones
bring, as well.  For example for the recovery check, we just hide more
stuff in thin wrappers, and we don't have that many entrance points
with these SQL stats import functions.

> 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.

Spoiler: v3, which is the latest patch set I am aware of in terms of
replacement of the fcinfo, does not apply cleanly on HEAD.  Looking at
the patch, you also seem to update multiple times the same area of the
code, that makes the result more confusing to me, because the result
you are looking for is hidden in a forest of cascading changes.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Koshino Taiki
Date:
Subject: doc: Reformat SELECT queries using GRAPH_TABLE
Next
From: Tomas Vondra
Date:
Subject: Re: toast table corrupted by vacuum - missing chunk number 0 for toast value