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>
In-Reply-To: 
 <CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Wed, 8 Apr 2026 10:23:59 +0900
Message-ID: 
 <CA+HiwqFK8rXTNknxV0MMe++7W08g3kY6eeLK21A1xCrK2Wuk8Q@mail.gmail.com>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
To: Evan Montgomery-Recht <montge@mianetworks.net>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000001d0c20064ee8c274"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqFK8rXTNknxV0MMe%2B%2B7W08g3kY6eeLK21A1xCrK2Wuk8Q%40mail.gmail.com>
Precedence: bulk

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

Hi Evan,

On Tue, Apr 7, 2026 at 10:00=E2=80=AFPM Evan Montgomery-Recht
<montge@mianetworks.net> wrote:
>
> Hi Amit,
>
> First time contributing to this project, let me know if I missed
> something or need to adjust what I put together.
>
> I found a crash in the RI fast-path FK check code introduced by
> 2da86c1ef9b and extended by b7b27eb41a5. C-language extensions that
> use SPI to INSERT into tables with multiple FK constraints hit an
> assertion failure (or, without assertions, a server crash) when the
> batch callback fires. I discovered this via PostGIS's topology CI --
> toTopoGeom() uses SPI to insert into edge_data, which has 4 immediate
> FK constraints referencing node and face. PG 18 passes the same test;
> the master crashes.
>
> This appears to be a separate issue from Chao Li's deferred-trigger
> batching bug; the patches touch different files and don't conflict.  I
> did do a regression test on the merge referenced both PostgreSQL and
> PostGIS (to validate that this works.)
>
> The problem: ri_FastPathGetEntry() opens pk_rel/idx_rel and creates
> TupleTableSlots, registering them with the current resource owner --
> the SPI portal's. The batch callback ri_FastPathTeardown() only fires
> when query_depth =3D=3D 0 (via FireAfterTriggerBatchCallbacks), but by
> that time the inner portal has finished and its resource owner has
> released the relation references and TupleDesc pins. The teardown then
> tries to close relations whose refcounts are already zero:
>
> TRAP: failed Assert("rel->rd_refcnt > 0"), File: "relcache.c"
>
> RelationDecrementReferenceCount
> -> RelationClose -> index_close -> ri_FastPathTeardown
> -> ri_FastPathEndBatch -> FireAfterTriggerBatchCallbacks
> -> AfterTriggerEndQuery -> standard_ExecutorFinish
>
> and TupleDesc pins that are no longer tracked by any resource owner
> cause "tupdesc reference not owned by resource owner" errors.
>
> Note that simple PL/pgSQL functions don't trigger this because
> PL/pgSQL's SPI connection spans the entire function call, so the
> portal's resource owner outlives the batch callback. The crash
> requires nested SPI from a C extension, which creates a shorter-lived
> portal.
>
> The attached patch (against master, for application) fixes this by
> transferring both relation references and TupleDesc pins from the
> current resource owner to CurTransactionResourceOwner immediately
> after creating them in ri_FastPathGetEntry(). The transfer uses
> RelationIncrementReferenceCount / PinTupleDesc under the target owner
> followed by RelationDecrementReferenceCount / ReleaseTupleDesc under
> the original. I chose this over switching CurrentResourceOwner around
> the table_open/index_open calls because the latter also affects
> transient buffer pins acquired during catalog lookups inside those
> functions.
>
> ri_FastPathTeardown is updated to clear any buffered tuples (whose
> buffer pins belong to the current resource owner) before switching to
> CurTransactionResourceOwner for the close/drop operations.
>
> The patch also adds a test module (test_spi_func) with a C function
> that executes SQL via SPI_connect/SPI_execute/SPI_finish, since this
> crash cannot be triggered from PL/pgSQL. The test exercises the
> C-level SPI INSERT with multiple FK constraints, FK violations, and
> nested PL/pgSQL-calls-C-SPI (matching the PostGIS call pattern).
>
> This is purely a correctness fix with no performance or backward
> compatibility impact. No documentation changes are needed since this
> is an internal bug fix.
>
> The patch compiles cleanly and passes pgindent, clang-tidy, and
> cppcheck. All 247 regression subtests pass, along with the full meson
> test suite (370 ok, 0 fail, 21 skipped) (skipped due to hardware
> availability on my side this week). I also verified the PostGIS
> topology test (toTopoGeom) passes clean with no warnings, and tested
> abort paths (FK violation, transaction rollback, subtransaction abort
> via EXCEPTION blocks) (not in scope of PostgreSQL but more for my own
> verification that things work). Code coverage on the new lines is
> 100%. Tested on macOS (aarch64) and Linux (aarch64, via Docker).

Thanks for the report and the patch.

I'll need to study this one a bit more closely.  Added an open item
for the time being:
https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items

> Unrelated to my patch, SonarCloud flagged a potential issue in
> recheck_matched_pk_tuple() (line 3370): the function loops over
> ii_NumIndexKeyAttrs elements of the skeys array, but the caller in
> ri_FastPathFlushArray passes recheck_skey[1] -- an array of exactly
> one element. This is safe because ri_FastPathFlushArray is the
>
> single-column FK path, so ii_NumIndexKeyAttrs is always 1 there.
> However, the function signature doesn't communicate this constraint,
> which flags as CWE-125 (out-of-bounds read) / CERT C ARR30-C. Adding
> an nkeys parameter (like ri_FastPathProbeOne already has) would make
> the contract explicit.

Makes sense.  Will push the attached patch for this.

--=20
Thanks, Amit Langote

--0000000000001d0c20064ee8c274
Content-Type: application/octet-stream;
	name="v1-0001-Add-nkeys-parameter-to-recheck_matched_pk_tuple.patch"
Content-Disposition: attachment;
	filename="v1-0001-Add-nkeys-parameter-to-recheck_matched_pk_tuple.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mnpd6rfv0>
X-Attachment-Id: f_mnpd6rfv0

RnJvbSAzNDRiYTY5NDQ1MWZiYWE1N2JiYjJlMTI5MzBmZTIwZmQ0MjVlODA2IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBbWl0IExhbmdvdGUgPGFtaXRsYW5AcG9zdGdyZXNxbC5vcmc+
CkRhdGU6IFdlZCwgOCBBcHIgMjAyNiAxMDoyMDoxNSArMDkwMApTdWJqZWN0OiBbUEFUQ0ggdjFd
IEFkZCBua2V5cyBwYXJhbWV0ZXIgdG8gcmVjaGVja19tYXRjaGVkX3BrX3R1cGxlKCkKClRoZSBm
dW5jdGlvbiBsb29wZWQgb3ZlciBpaV9OdW1JbmRleEtleUF0dHJzIGVsZW1lbnRzIG9mIHRoZSBz
a2V5cwphcnJheSwgYnV0IG9uZSBjYWxsZXIgKHJpX0Zhc3RQYXRoRmx1c2hBcnJheSkgcGFzc2Vz
IGEgb25lLWVsZW1lbnQKYXJyYXkgc2luY2UgaXQgb25seSBoYW5kbGVzIHNpbmdsZS1jb2x1bW4g
RktzLiAgVGhlIGZ1bmN0aW9uCnNpZ25hdHVyZSBkaWQgbm90IGNvbW11bmljYXRlIHRoaXMgY29u
c3RyYWludCwgd2hpY2ggc3RhdGljIGFuYWx5c2lzCmZsYWdzIGFzIGEgcG90ZW50aWFsIG91dC1v
Zi1ib3VuZHMgcmVhZC4KCkFkZCBhbiBua2V5cyBwYXJhbWV0ZXIgYW5kIGFzc2VydCB0aGF0IGl0
IG1hdGNoZXMKaWlfTnVtSW5kZXhLZXlBdHRycywgdGhlbiB1c2UgaXQgaW4gdGhlIGxvb3AuICBU
aGUgY2FsbCBzaXRlcwphbHJlYWR5IGtub3cgdGhlIGtleSBjb3VudC4KClJlcG9ydGVkLWJ5OiBF
dmFuIE1vbnRnb21lcnktUmVjaHQgPG1vbnRnZUBtaWFuZXR3b3Jrcy5uZXQ+CkRpc2N1c3Npb246
IGh0dHBzOi8vcG9zdGdyLmVzL20vQ0FFZzdwd2NLZjAxRm1EcUZBZi1IenVfcFluTVlTY1lfT3Rp
ZC1wZTl1dzNCSjZncTlnQG1haWwuZ21haWwuY29tCi0tLQogc3JjL2JhY2tlbmQvdXRpbHMvYWR0
L3JpX3RyaWdnZXJzLmMgfCAxMSArKysrKystLS0tLQogMSBmaWxlIGNoYW5nZWQsIDYgaW5zZXJ0
aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC91dGlscy9h
ZHQvcmlfdHJpZ2dlcnMuYyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9yaV90cmlnZ2Vycy5jCmlu
ZGV4IDg0ZjlmZWNkYjRjLi4xOGVjODU4MzU3ZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRp
bHMvYWR0L3JpX3RyaWdnZXJzLmMKKysrIGIvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3JpX3RyaWdn
ZXJzLmMKQEAgLTMyOSw3ICszMjksNyBAQCBzdGF0aWMgYm9vbCByaV9Mb2NrUEtUdXBsZShSZWxh
dGlvbiBwa19yZWwsIFR1cGxlVGFibGVTbG90ICpzbG90LCBTbmFwc2hvdCBzbmFwLAogc3RhdGlj
IGJvb2wgcmlfZmFzdHBhdGhfaXNfYXBwbGljYWJsZShjb25zdCBSSV9Db25zdHJhaW50SW5mbyAq
cmlpbmZvKTsKIHN0YXRpYyB2b2lkIHJpX0NoZWNrUGVybWlzc2lvbnMoUmVsYXRpb24gcXVlcnlf
cmVsKTsKIHN0YXRpYyBib29sIHJlY2hlY2tfbWF0Y2hlZF9wa190dXBsZShSZWxhdGlvbiBpZHhy
ZWwsIFNjYW5LZXlEYXRhICpza2V5cywKLQkJCQkJCQkJCSBUdXBsZVRhYmxlU2xvdCAqbmV3X3Ns
b3QpOworCQkJCQkJCQkJIGludCBua2V5cywgVHVwbGVUYWJsZVNsb3QgKm5ld19zbG90KTsKIHN0
YXRpYyB2b2lkIGJ1aWxkX2luZGV4X3NjYW5rZXlzKGNvbnN0IFJJX0NvbnN0cmFpbnRJbmZvICpy
aWluZm8sCiAJCQkJCQkJCSBSZWxhdGlvbiBpZHhfcmVsLCBEYXR1bSAqcGtfdmFscywKIAkJCQkJ
CQkJIGNoYXIgKnBrX251bGxzLCBTY2FuS2V5IHNrZXlzKTsKQEAgLTMxMzgsNyArMzEzOCw3IEBA
IHJpX0Zhc3RQYXRoRmx1c2hBcnJheShSSV9GYXN0UGF0aEVudHJ5ICpmcGVudHJ5LCBUdXBsZVRh
YmxlU2xvdCAqZmtfc2xvdCwKIAkJCQkJCQkJICAgaWR4X3JlbC0+cmRfaW5kY29sbGF0aW9uWzBd
LAogCQkJCQkJCQkgICBmcG1ldGEtPnJlZ29wc1swXSwKIAkJCQkJCQkJICAgZm91bmRfdmFsKTsK
LQkJCWlmICghcmVjaGVja19tYXRjaGVkX3BrX3R1cGxlKGlkeF9yZWwsIHJlY2hlY2tfc2tleSwg
cGtfc2xvdCkpCisJCQlpZiAoIXJlY2hlY2tfbWF0Y2hlZF9wa190dXBsZShpZHhfcmVsLCByZWNo
ZWNrX3NrZXksIDEsIHBrX3Nsb3QpKQogCQkJCWNvbnRpbnVlOwogCQl9CiAKQEAgLTMxOTMsNyAr
MzE5Myw3IEBAIHJpX0Zhc3RQYXRoUHJvYmVPbmUoUmVsYXRpb24gcGtfcmVsLCBSZWxhdGlvbiBp
ZHhfcmVsLAogCQkJCQkJICAgJmNvbmN1cnJlbnRseV91cGRhdGVkKSkKIAkJewogCQkJaWYgKGNv
bmN1cnJlbnRseV91cGRhdGVkKQotCQkJCWZvdW5kID0gcmVjaGVja19tYXRjaGVkX3BrX3R1cGxl
KGlkeF9yZWwsIHNrZXksIHNsb3QpOworCQkJCWZvdW5kID0gcmVjaGVja19tYXRjaGVkX3BrX3R1
cGxlKGlkeF9yZWwsIHNrZXksIG5rZXlzLCBzbG90KTsKIAkJCWVsc2UKIAkJCQlmb3VuZCA9IHRy
dWU7CiAJCX0KQEAgLTMzNDAsNyArMzM0MCw3IEBAIHJpX0NoZWNrUGVybWlzc2lvbnMoUmVsYXRp
b24gcXVlcnlfcmVsKQogICogbm90IGZvdW5kLgogICovCiBzdGF0aWMgYm9vbAotcmVjaGVja19t
YXRjaGVkX3BrX3R1cGxlKFJlbGF0aW9uIGlkeHJlbCwgU2NhbktleURhdGEgKnNrZXlzLAorcmVj
aGVja19tYXRjaGVkX3BrX3R1cGxlKFJlbGF0aW9uIGlkeHJlbCwgU2NhbktleURhdGEgKnNrZXlz
LCBpbnQgbmtleXMsCiAJCQkJCQkgVHVwbGVUYWJsZVNsb3QgKm5ld19zbG90KQogewogCS8qCkBA
IC0zMzU5LDggKzMzNTksOSBAQCByZWNoZWNrX21hdGNoZWRfcGtfdHVwbGUoUmVsYXRpb24gaWR4
cmVsLCBTY2FuS2V5RGF0YSAqc2tleXMsCiAJCSAgIGluZGV4SW5mby0+aWlfRXhjbHVzaW9uT3Bz
ID09IE5VTEwpOwogCiAJLyogRm9ybSB0aGUgaW5kZXggdmFsdWVzIGFuZCBpc251bGwgZmxhZ3Mg
Z2l2ZW4gdGhlIHRhYmxlIHR1cGxlLiAqLworCUFzc2VydChua2V5cyA9PSBpbmRleEluZm8tPmlp
X051bUluZGV4S2V5QXR0cnMpOwogCUZvcm1JbmRleERhdHVtKGluZGV4SW5mbywgbmV3X3Nsb3Qs
IE5VTEwsIHZhbHVlcywgaXNudWxsKTsKLQlmb3IgKGludCBpID0gMDsgaSA8IGluZGV4SW5mby0+
aWlfTnVtSW5kZXhLZXlBdHRyczsgaSsrKQorCWZvciAoaW50IGkgPSAwOyBpIDwgbmtleXM7IGkr
KykKIAl7CiAJCVNjYW5LZXlEYXRhICpza2V5ID0gJnNrZXlzW2ldOwogCi0tIAoyLjQ3LjMKCg==
--0000000000001d0c20064ee8c274--