MIME-Version: 1.0
References: 
 <CA+HiwqF4C0ws3cO+z5cLkPuvwnAwkSp7sfvgGj3yQ=Li6KNMqA@mail.gmail.com>
 <CA+HiwqGM6nvAV5O+=Nr+BXMPWOma0oeCRVzVP0XiLE8zX5TVAg@mail.gmail.com>
 <CA+HiwqGMaovCUgDbGxVGnK0Mrivr+ph3YE2Ws+47-ugyPb4f7g@mail.gmail.com>
 <CAFj8pRDaiBe_GOLk_yyYHTtPiDAAaLOM8u1-=Q3ZgXBTH+1igg@mail.gmail.com>
 <CA+HiwqGA5Ay_MR0eJEEbt4j6WrVh4F+AasTp8yCbs5aJLOJn6Q@mail.gmail.com>
 <CAEG8a3JM=NoqiTK0V6S9FNxZPvy1+C5F7rfafTtPKBVWnunL-g@mail.gmail.com>
 <CA+HiwqEyiLCY6MTLbOJXDdLNNQLaURYHvdW797MQgbjEK9od4Q@mail.gmail.com>
 <CAEG8a3+VBpwPf1Rm-ECD90whM9b3YnGhux5CVXdsL6khiBfzRQ@mail.gmail.com>
 <CA+HiwqF2UHzF0sKCp-F2a-U29rqh_9ZPy=f1h+Fh_=M8efj3pg@mail.gmail.com>
 <CAEG8a3L9Ew-WL8sxLROVOcypeaENPmd8qCmMvz4geoGL1TDGCA@mail.gmail.com>
 <CAEG8a3+nUFQo4sdPQF9xy0J73J8RFJ5U9A5+_kMosGDaZ+1sXA@mail.gmail.com>
 <2BE661BA-D909-4093-BF78-DB9B0C099337@gmail.com>
 <CAEG8a3JyKdizWvYsF+z_mA1BKy=dpW11iKVMOG=bk6Tbz6M1Bw@mail.gmail.com>
 <CAEG8a3+Hf4tvvbts29_k_AFhWQmRYfEo_SW4C5FY_140iKghBw@mail.gmail.com>
 <CA+HiwqFV-PY-3BxM6j5TaAiC3AwedDxo-6vwRSbvygg3zF+xAQ@mail.gmail.com>
 <CA+HiwqHpaisS-e+0gAgzh92qZAFxncAJMmmTRZZN=efoeTPgOg@mail.gmail.com>
 <CA+HiwqFwZ6WLRbY8R7VC7JVp5Jot6ktZOkr9wDxTjoK=W1SgdQ@mail.gmail.com>
 <CA+HiwqF_Kz=R8juHJBiOATvabWSOugK4VaGOxoJ_n=E2c7UM9w@mail.gmail.com>
 <CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
 <77FA04FE-1F84-4DA1-8855-8BBFD8CC889A@gmail.com>
 <CA+HiwqHpVtP485wEKuXdOkdoZWhvVvfFH40-og07Jp3MPx21eg@mail.gmail.com>
 <CAEG8a3JWHkJSXe9nNcVK7wnYKUEqWuMGFDhy5BynB_9tEjmEUg@mail.gmail.com>
 <CA+HiwqFjfumKrWy03q5M309xJJVYt0WgGfH6AZ8BjFhSwppwsQ@mail.gmail.com>
 <CAEG8a3JjP1LaKSv-r3AMJLRyLMzENJrKshWsDvDouMPM_sizmA@mail.gmail.com>
 <CA+HiwqFQ+ZA7hSOygv4uv_t75B3r0_gosjadetCsAEoaZwTu6g@mail.gmail.com>
 <CA+HiwqHdB0r8z6Mut13BxpYNq2W-os+Arju4mcZbCyU9PeaVog@mail.gmail.com>
 <CAEG8a3K5ayVNkCDnK9OtNb+4OY0chJtW6ObgEOSFjqyymQda8Q@mail.gmail.com>
 <CA+HiwqGJYCgEs_F-LBtrRdx-Y969LMr-OVogjFXU6U-0q5bOwQ@mail.gmail.com>
 <CA+HiwqF2Ma6R_QyjfPBvFreYbezFpGcwASJE0a2bM+Y0jvof+g@mail.gmail.com>
 <C3ACBAE1-CC5F-4688-9828-4668CDE37F83@gmail.com>
 <CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
 <C2133B47-79CD-40FF-B088-02D20D654806@gmail.com>
 <CA+HiwqFx=aciJYkkaviyTutUm303QXz6GtXSqzG7nfd4MAzddQ@mail.gmail.com>
 <CA+HiwqH39QU7vGVx65JH1e0nzVvQc5eVmuY7=qyj0T_+b-HO3A@mail.gmail.com>
 <72AA2663-B642-4FB1-BDC2-5FAFF2D2DF15@gmail.com>
 <CA+HiwqF+jAyHUiNzpR+vRBbpeCiVAdFU52-ffTGko9Zit317oA@mail.gmail.com>
 <CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com>
 <CA+HiwqFK8rXTNknxV0MMe++7W08g3kY6eeLK21A1xCrK2Wuk8Q@mail.gmail.com>
 <CA+HiwqFdkJGBU9QmYkm6056SuhunGk7yFCuVP=2vBejJa+qhGg@mail.gmail.com>
 <CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@mail.gmail.com>
 <8561287B-19F1-421E-959D-99F4593CFF54@gmail.com>
 <CA+HiwqFWLR01NjK5Y+MKiyaqg64ThVS7UYKK3ZBVNHvtm=3-ug@mail.gmail.com>
 <E121826D-E962-4CA8-890D-88A9F858BD68@gmail.com>
 <CA+HiwqFh_CMi95g2-W259i216_MxDMuOUcR+vrA8VqSTf8KLXA@mail.gmail.com>
 <CABXr29FPjrywzww=JHHLmehon-s9KccDb-q+gTJOUh5c0TH4yw@mail.gmail.com>
In-Reply-To: 
 <CABXr29FPjrywzww=JHHLmehon-s9KccDb-q+gTJOUh5c0TH4yw@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Wed, 15 Apr 2026 15:03:50 +0900
Message-ID: 
 <CA+HiwqG+rjGXmGEdfgM1atkZX-DrkxjTCP10LUJNQtbM8BNujw@mail.gmail.com>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
To: Haibo Yan <tristan.yim@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>,
 Evan Montgomery-Recht <montge@mianetworks.net>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqG%2BrjGXmGEdfgM1atkZX-DrkxjTCP10LUJNQtbM8BNujw%40mail.gmail.com>
Precedence: bulk

On Sat, Apr 11, 2026 at 8:34=E2=80=AFAM Haibo Yan <tristan.yim@gmail.com> w=
rote:
> On Fri, Apr 10, 2026 at 12:39=E2=80=AFAM Amit Langote <amitlangote09@gmai=
l.com> wrote:
>> Here's the remaning patch to add src/test/modules/test_spi_resowner
>> rebased against master. I'm holding off on committing the test module
>> until I confirm the policy on new test modules during feature freeze.
>> It's also worth discussing whether this is the right place for testing
>> C extensions that use SPI with a dedicated resource owner, or whether
>> that coverage belongs elsewhere.
>
> I reviewed the patch, and overall it looks close. I have a few comments:
>
> Should spi_exec_sql() be made exception-safe?
>
> The current implementation does not restore CurrentResourceOwner or relea=
se/delete childowner on all error paths, and it also does not check for SPI=
_connect() failure. Since this module is specifically meant to exercise Res=
ourceOwner lifetime interactions, I think the helper itself should be robus=
t in both success and error paths.
>
> Consider adding a follow-up test that does failure first, then success.
>
> That would help show that the helper does not leave any lingering state b=
ehind after an error.
>
> Consider trimming the long explanatory comments in the regression test a =
bit.
>
> The rationale is useful, but some of it is repeated across the commit mes=
sage, the SQL file header, and the expected output.

Thanks Haibo for the review. Your points are well taken and would need
to be addressed if this module were to be committed, but I've been
reconsidering whether to commit it at all. It was written to reproduce
a specific crash caused by an extension's idiosyncratic use of SPI
with a dedicated resource owner, a pattern that's specific to PostGIS
and similar extensions rather than something core PostgreSQL
exercises. Now that the crash is fixed, the module's main value is as
a regression test for that one scenario. I'm not convinced it pulls
its weight as a permanent addition to the test suite, especially given
the maintenance burden and the time it adds to test runs.

I'll hold off on committing it unless someone feels strongly that it
should be included.

--=20
Thanks, Amit Langote