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>
In-Reply-To: 
 <CADkLM=dxe8NbQu4RNi2OFXpaRKdpS0-CHGm6Mqtf_rW155BM4g@mail.gmail.com>
From: Corey Huinker <corey.huinker@gmail.com>
Date: Tue, 16 Jun 2026 15:11:45 -0400
Message-ID: 
 <CADkLM=cV=uezAKyZdyyxnZOBRhGLUSbeYfDj5_jbDCLP9LP24Q@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="0000000000007f71e6065463b7f3"
Archived-At: 
 <https://www.postgresql.org/message-id/CADkLM%3DcV%3DuezAKyZdyyxnZOBRhGLUSbeYfDj5_jbDCLP9LP24Q%40mail.gmail.com>
Precedence: bulk

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

On Tue, Jun 16, 2026 at 2:08=E2=80=AFPM Corey Huinker <corey.huinker@gmail.=
com>
wrote:

>
> On Tue, Jun 16, 2026 at 10:34=E2=80=AFAM Robert Haas <robertmhaas@gmail.c=
om>
> wrote:
>
>> On Tue, Jun 16, 2026 at 8:04=E2=80=AFAM Etsuro Fujita <etsuro.fujita@gma=
il.com>
>> wrote:
>> > I thought it would be a good idea to use
>> > pg_restore_attribute_stats/pg_restore_relation_stats, because future
>> > changes in attribute/relation stats would be absorbed by these
>> > functions, which would lower the maintenance cost of this feature.
>>
>> I agree that we want to reuse code, but this isn't the right way to do
>> it. For example, when we want to look up the OID of a relation from
>> SQL, we can say 'whatever'::regclass::oid. But when we want to do the
>> same thing from C, we don't construct a SELECT statement and execute
>> it via SPI. Instead, we have functions like RangeVarGetRelidExtended()
>> which provide access to the same underlying functionality more
>> directly.
>>
>> Another example is converting strings to integers. The user calls
>> int4in(), which then hands off the call to pg_strtoint32_safe(), which
>> can also be called via pg_strtoint32(). Hence, C code should prefer to
>> use pg_strtoint32(), while SQL will go through int4in(). Both
>> ultimately reach the underlying pg_strtoint32_safe() function, but the
>> interfaces are different, so that we can have it be suitable both for
>> SQL access and for C access.
>>
>> This needs to work more like that. The underlying code that ingests
>> and updates the stats should be shared, but the stuff that is specific
>> to a FunctionCallInfo interface needs to be separated out so that we
>> don't need to go through that when calling from C.
>>
>>
> Back when pg_restore_attribute_stats and pg_restore_relation_stats were
> being converted from positional to variadic functions, there was a patchs=
et
> or two (possibly un-posted, because I couldn't find it just now) where th=
e
> variadic functions re-marshalled the arguments into a call to a
> fixed-parameter function. If that model were revived, we could have a mor=
e
> conventional DirectFunctonCall() interface.
>
> Is it possible that we never built a direct function call interface to a
> variadic because we never needed one? Perhaps that time is now.
>

Obviously, even that wouldn't get us to the no-FunctionCallInfo-at-all
goal, but it would get us out of the SPI situation. If we did have a
non-fcinfo function API, that API would either have to pass char *params
almost exclusively, or else each caller would have to do the translation or
float-array strings to double[] and such, which would be a lot of work for
the caller.

Would you be ok with a function like attribute_statistics_update, but
purely with cstring args? Obviously the callers would have to modify their
calls each time a new stat param is added, but that is seemingly preferable
for you than the current situation.

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

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Tue, Jun 16,=
 2026 at 2:08=E2=80=AFPM Corey Huinker &lt;<a href=3D"mailto:corey.huinker@=
gmail.com">corey.huinker@gmail.com</a>&gt; wrote:<br></div><blockquote clas=
s=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid r=
gb(204,204,204);padding-left:1ex"><div dir=3D"ltr"><br><div class=3D"gmail_=
quote"><div dir=3D"ltr" class=3D"gmail_attr">On Tue, Jun 16, 2026 at 10:34=
=E2=80=AFAM Robert Haas &lt;<a href=3D"mailto:robertmhaas@gmail.com" target=
=3D"_blank">robertmhaas@gmail.com</a>&gt; wrote:<br></div><blockquote class=
=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg=
b(204,204,204);padding-left:1ex">On Tue, Jun 16, 2026 at 8:04=E2=80=AFAM Et=
suro Fujita &lt;<a href=3D"mailto:etsuro.fujita@gmail.com" target=3D"_blank=
">etsuro.fujita@gmail.com</a>&gt; wrote:<br>
&gt; I thought it would be a good idea to use<br>
&gt; pg_restore_attribute_stats/pg_restore_relation_stats, because future<b=
r>
&gt; changes in attribute/relation stats would be absorbed by these<br>
&gt; functions, which would lower the maintenance cost of this feature.<br>
<br>
I agree that we want to reuse code, but this isn&#39;t the right way to do<=
br>
it. For example, when we want to look up the OID of a relation from<br>
SQL, we can say &#39;whatever&#39;::regclass::oid. But when we want to do t=
he<br>
same thing from C, we don&#39;t construct a SELECT statement and execute<br=
>
it via SPI. Instead, we have functions like RangeVarGetRelidExtended()<br>
which provide access to the same underlying functionality more<br>
directly.<br>
<br>
Another example is converting strings to integers. The user calls<br>
int4in(), which then hands off the call to pg_strtoint32_safe(), which<br>
can also be called via pg_strtoint32(). Hence, C code should prefer to<br>
use pg_strtoint32(), while SQL will go through int4in(). Both<br>
ultimately reach the underlying pg_strtoint32_safe() function, but the<br>
interfaces are different, so that we can have it be suitable both for<br>
SQL access and for C access.<br>
<br>
This needs to work more like that. The underlying code that ingests<br>
and updates the stats should be shared, but the stuff that is specific<br>
to a FunctionCallInfo interface needs to be separated out so that we<br>
don&#39;t need to go through that when calling from C.<br>
<br></blockquote><div><br></div><div><div>Back when pg_restore_attribute_st=
ats and pg_restore_relation_stats=20
were being converted from positional to variadic functions, there was a=20
patchset or two (possibly un-posted, because I couldn&#39;t find it just=20
now) where the variadic functions re-marshalled the arguments=20
into a call to a fixed-parameter function. If that model were revived, we c=
ould have a more conventional DirectFunctonCall() interface.</div><div><br>=
</div><div>Is it possible that we never built a direct function call interf=
ace to a variadic because we never needed one? Perhaps that time is now.</d=
iv></div></div></div></blockquote><div><br></div><div>Obviously, even that =
wouldn&#39;t get us to the no-FunctionCallInfo-at-all goal, but it would ge=
t us out of the SPI situation. If we did have a non-fcinfo function API, th=
at API would either have to pass char *params almost exclusively, or else e=
ach caller would have to do the translation or float-array strings to doubl=
e[] and such, which would be a lot of work for the caller.<br><br></div><di=
v>Would you be ok with a function like attribute_statistics_update, but pur=
ely with cstring args? Obviously the callers would have to modify their cal=
ls each time a new stat param is added, but that is seemingly preferable fo=
r you than the current situation.</div></div></div>

--0000000000007f71e6065463b7f3--