MIME-Version: 1.0
References: 
 <CA+TgmoYqMtWb4zLUkT98oFnEkJ=Wz0Pw-ggDJrp9wnSXPzUaeQ@mail.gmail.com>
 <CAPmGK161TJ4G01eptv3sBqgxa4SNHDE9VRqoC6jy5Y6G3D8yDw@mail.gmail.com>
 <CA+TgmoZpwfTGqqp=0efV99e_BMYWdLFq2XKRWVSDcukifkZMeA@mail.gmail.com>
 <CADkLM=dxe8NbQu4RNi2OFXpaRKdpS0-CHGm6Mqtf_rW155BM4g@mail.gmail.com>
 <CADkLM=cV=uezAKyZdyyxnZOBRhGLUSbeYfDj5_jbDCLP9LP24Q@mail.gmail.com>
 <CA+TgmoaVcgZJpwxyi7HEfNMJhmm7BXkTQ468jvxkW2QzQZRkvg@mail.gmail.com>
 <CADkLM=dy18dDJ+j3-_Ofn11vt5J7zF33oTWnE6N=ePc83uS91A@mail.gmail.com>
 <CAPmGK15MfVT2H+PUvA_nAJvySGr_pAVkVx3KcnVXWGO36acWLA@mail.gmail.com>
 <CADkLM=dr5bfB8YN-bV1U0NDvyVO7a6uuz0BsAi89a1Md4A==8g@mail.gmail.com>
 <CAPmGK14P2GRe78hp6fZ3vExU_9RRDjMsYVOx7AALvUQ-hKTUiQ@mail.gmail.com>
 <CADkLM=cL9ZGvO-72QWyXA5Wqv-4P3T1fp+VFJy5e6k3Hf=POMw@mail.gmail.com>
 <CA+TgmoZdTVeXRQo__cEr_1y+mUO9nubOX=6PEfw7Cr0UfZQn6w@mail.gmail.com>
In-Reply-To: 
 <CA+TgmoZdTVeXRQo__cEr_1y+mUO9nubOX=6PEfw7Cr0UfZQn6w@mail.gmail.com>
From: Corey Huinker <corey.huinker@gmail.com>
Date: Sat, 20 Jun 2026 00:11:17 -0400
Message-ID: 
 <CADkLM=fe=ycPdwT2wE_iYwK8e8viMbWvwWhXR9vDe+55KKF-KQ@mail.gmail.com>
Subject: Re: use of SPI by postgresImportForeignStatistics
To: Robert Haas <robertmhaas@gmail.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000806a840654a79a6a"
Archived-At: 
 <https://www.postgresql.org/message-id/CADkLM%3Dfe%3DycPdwT2wE_iYwK8e8viMbWvwWhXR9vDe%2B55KKF-KQ%40mail.gmail.com>
Precedence: bulk

--000000000000806a840654a79a6a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 19, 2026 at 9:45=E2=80=AFAM Robert Haas <robertmhaas@gmail.com>=
 wrote:

> On Thu, Jun 18, 2026 at 5:23=E2=80=AFPM Corey Huinker <corey.huinker@gmai=
l.com>
> wrote:
> > To be clear, this solves the SPI problem, but questions about the desig=
n
> 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 the=
y
> could, but if time allows I'd at least like try unwinding some of that. B=
ut
> first, let's get SPI out of postgres_fdw.c.
>
> I think that's the right order of priority, but I don't think that
> having import_attribute_statistics construct an fcinfo is great.
>

Me either, I'm looking at "phase 2" already where
relation/attribute_statistics_update becomes a conventional function, and
pg_clear_attribute_stats and pg_restore_attribute_stats (and their relstats
equivalents) marshal their parameters to call that instead.


> Ideally, attribute_statistics_update would call
> import_attribute_statistics rather than the other way around.
>

I think we're mostly on the same page. If we require the caller to
understand the stats data structures to a greater level of detail, and
require it to do the transformations to the proper input types
(BlockNumbers, floats, float arrays, cstrings for anyarray, etc), then the
import_attribute_statistics functon and the new attribute_statistics_update
would be one-in-the-same. The v1 patch leaned heavily (perhaps too far)
towards letting the caller pass along string values fetched via
PQgetvalue() from a pg_stats without modification.

--000000000000806a840654a79a6a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_quote gmail_quote_container"><div dir=
=3D"ltr" class=3D"gmail_attr">On Fri, Jun 19, 2026 at 9:45=E2=80=AFAM Rober=
t Haas &lt;<a href=3D"mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</=
a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0p=
x 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On=
 Thu, Jun 18, 2026 at 5:23=E2=80=AFPM Corey Huinker &lt;<a href=3D"mailto:c=
orey.huinker@gmail.com" target=3D"_blank">corey.huinker@gmail.com</a>&gt; w=
rote:<br>
&gt; To be clear, this solves the SPI problem, but questions about the desi=
gn of attribute_statistics_update() and relation_statistics_update() remain=
, but those concerns are now isolated within their respective files attribu=
te_stats.c and relation_stats.c. The inefficiencies therein aren&#39;t real=
ly in a critical path, so if we wanted to leave them be until v20 they coul=
d, but if time allows I&#39;d at least like try unwinding some of that. But=
 first, let&#39;s get SPI out of postgres_fdw.c.<br>
<br>
I think that&#39;s the right order of priority, but I don&#39;t think that<=
br>
having import_attribute_statistics construct an fcinfo is great.<br></block=
quote><div><br></div><div>Me either, I&#39;m looking at &quot;phase 2&quot;=
 already where relation/attribute_statistics_update becomes a conventional =
function, and pg_clear_attribute_stats and pg_restore_attribute_stats (and =
their relstats equivalents) marshal their parameters to call that instead.<=
/div><div></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D=
"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-le=
ft:1ex">
Ideally, attribute_statistics_update would call<br>
import_attribute_statistics rather than the other way around.<br></blockquo=
te><div><br></div><div>I think we&#39;re mostly on the same page. If we req=
uire the caller to understand the stats data structures to a greater level =
of detail, and require it to do the transformations to the proper input typ=
es (BlockNumbers, floats, float arrays, cstrings for anyarray, etc), then t=
he import_attribute_statistics functon and the new attribute_statistics_upd=
ate would be one-in-the-same. The v1 patch leaned heavily (perhaps too far)=
 towards letting the caller
 pass along string values fetched via PQgetvalue() from a pg_stats without =
modification.</div></div></div>

--000000000000806a840654a79a6a--