Horton
Menu

Re: Eliminating SPI / SQL from some RI triggers - take 3

Haibo Yan <tristan.yim@gmail.com>

View thread
From: Haibo Yan <tristan.yim@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>, Evan Montgomery-Recht <montge@mianetworks.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2026-04-10T23:34:21Z
Lists: pgsql-hackers 
On Fri, Apr 10, 2026 at 12:39 AM Amit Langote <amitlangote09@gmail.com>
wrote:

> On Thu, Apr 9, 2026 at 7:26 PM Chao Li <li.evan.chao@gmail.com> 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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some const qualifier use in ri_triggers.c

  2. doc: Improve consistency of parallel vacuum description.

  3. Move afterTriggerFiringDepth into AfterTriggersData

  4. Fix typo left by 34a30786293

  5. Fix RI fast-path crash under nested C-level SPI

  6. Add nkeys parameter to recheck_matched_pk_tuple()

  7. Fix deferred FK check batching introduced by commit b7b27eb41a5

  8. Optimize fast-path FK checks with batched index probes

  9. Make FastPathMeta self-contained by copying FmgrInfo structs

  10. Add fast path for foreign key constraint checks