public inbox for [email protected]
help / color / mirror / Atom feedFrom: Haibo Yan <[email protected]>
To: Amit Langote <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Evan Montgomery-Recht <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Fri, 10 Apr 2026 16:34:21 -0700
Message-ID: <CABXr29FPjrywzww=JHHLmehon-s9KccDb-q+gTJOUh5c0TH4yw@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqFh_CMi95g2-W259i216_MxDMuOUcR+vrA8VqSTf8KLXA@mail.gmail.com>
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>
<[email protected]>
<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>
<[email protected]>
<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>
<[email protected]>
<CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
<[email protected]>
<CA+HiwqFx=aciJYkkaviyTutUm303QXz6GtXSqzG7nfd4MAzddQ@mail.gmail.com>
<CA+HiwqH39QU7vGVx65JH1e0nzVvQc5eVmuY7=qyj0T_+b-HO3A@mail.gmail.com>
<[email protected]>
<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>
<[email protected]>
<CA+HiwqFWLR01NjK5Y+MKiyaqg64ThVS7UYKK3ZBVNHvtm=3-ug@mail.gmail.com>
<[email protected]>
<CA+HiwqFh_CMi95g2-W259i216_MxDMuOUcR+vrA8VqSTf8KLXA@mail.gmail.com>
On Fri, Apr 10, 2026 at 12:39 AM Amit Langote <[email protected]>
wrote:
> On Thu, Apr 9, 2026 at 7:26 PM Chao Li <[email protected]> wrote:
> > 0001 and 0002 look good to me. I didn’t review 0003 and don’t intend to
> review it.
>
> I've now pushed 0001 (34a3078629) and 0002 (d6e96bacd3c).
>
> 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.
>
> --
> Thanks, Amit Langote
>
I reviewed the patch, and overall it looks close. I have a few comments:
1.
Should spi_exec_sql() be made exception-safe?
The current implementation does not restore CurrentResourceOwner or
release/delete childowner on all error paths, and it also does not check
for SPI_connect() failure. Since this module is specifically meant to
exercise ResourceOwner lifetime interactions, I think the helper itself
should be robust in both success and error paths.
2.
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
behind after an error.
3.
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
message, the SQL file header, and the expected output.
Regards
Haibo
view thread (63+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
In-Reply-To: <CABXr29FPjrywzww=JHHLmehon-s9KccDb-q+gTJOUh5c0TH4yw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox