Re: use of SPI by postgresImportForeignStatistics

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: use of SPI by postgresImportForeignStatistics
Date: 2026-06-18 21:23:40
Message-ID: CADkLM=cL9ZGvO-72QWyXA5Wqv-4P3T1fp+VFJy5e6k3Hf=POMw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 18, 2026 at 6:43 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:

> On Thu, Jun 18, 2026 at 1:00 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
> wrote:
>
> >> IMO I don't think we need to go that far, because 1) the new functions
> >> are provided for FDW authors only, and 2) we don't make changes to the
> >> stats that often.
> >
> >
> > That would be simpler, to be sure. However, in the past I've received
> pushback on functions that had a large number of parameters, and this would
> definitely be a large number of parameters, approximately 17, so I thought
> I should at least offer the variadic way.
> >
> > I'm proceeding with the many-parameters model.
>
> I think that that's acceptable, considering that
> heap_create_with_catalog() has 21 parameters.
>
> Thanks!
>
> Best regards,
> Etsuro Fujita
>

Attached are two patches to remove SPI from the postgres_fdw.c, replaced by
local C functions. The division into two patches is entirely for
readability. Separately, each one represents a half-measure that would
benefit no one. The first does the minimal changes needed to get the
relation-stats to stop using SPI, and the second does the same for
attribute stats, removes all vestiges of SPI from postgres_fdw.c, and adds
some tests to make sure that a foreign table has the exact stats of the
source loopback table.

The function import_attribute_stats() is a bit parameter-happy, but that's
mostly necessary. I left the non-key parameters as all type char* because
that avoids even more "bool foo_isnull" parameters, and that allows the
caller to not have to rework the various stat types into their
corresponding C types (float, int4, float[], and the anyarrays that
basically aren't cast-able by any client program anyway). I'm open to
requiring the caller to use the right datatypes for some of the parameters,
but as I said there is no way to "win" with the anyarrays, and even the
pg_restore_attribute_stats() function falls back to type text for those.

To be clear, this solves the SPI problem, but questions about the design of
attribute_statistics_update() and relation_statistics_update() remain, but
those concerns are now isolated within their respective files
attribute_stats.c and relation_stats.c. The inefficiencies therein aren't
really in a critical path, so if we wanted to leave them be until v20 they
could, but if time allows I'd at least like try unwinding some of that. But
first, let's get SPI out of postgres_fdw.c.

Attachment Content-Type Size
v1-0001-Remove-SPI-in-favor-of-import_relation_statistics.patch text/x-patch 9.8 KB
v1-0002-Remove-SPI-in-favor-of-import_attribute_statistic.patch text/x-patch 22.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2026-06-18 21:23:50 Re: DataChecksumsStateStruct cost_delay fields and locking
Previous Message Tom Lane 2026-06-18 21:22:25 Re: PG20 Minimum Dependency Thread