From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <77FA04FE-1F84-4DA1-8855-8BBFD8CC889A@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_1AB14FA3-7520-4FAA-9E08-DF39B58C6C0D"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Tue, 31 Mar 2026 17:09:19 +0800
In-Reply-To: 
 <CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>,
 Haibo Yan <tristan.yim@gmail.com>,
 Pavel Stehule <pavel.stehule@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Tomas Vondra <tomas@vondra.me>
To: Amit Langote <amitlangote09@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>
 <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>
Archived-At: 
 <https://www.postgresql.org/message-id/77FA04FE-1F84-4DA1-8855-8BBFD8CC889A%40gmail.com>
Precedence: bulk


--Apple-Mail=_1AB14FA3-7520-4FAA-9E08-DF39B58C6C0D
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Mar 30, 2026, at 19:15, Amit Langote <amitlangote09@gmail.com> =
wrote:
>=20
> On Mon, Mar 30, 2026 at 1:55=E2=80=AFPM Amit Langote =
<amitlangote09@gmail.com> wrote:
>> Junwang pointed out off-list that FK tuples added to
>> RI_FastPathEntry.batch[] were being copied into TopTransactionContext
>> rather than flush_cxt, so they would accumulate until the batch was
>> exhausted rather than being reclaimed per flush. Fixed in
>> ri_FastPathBatchAdd() in 0002.
>>=20
>> Also added a couple of comments in trigger.c that were missing: an
>> Assert and explanation in RegisterAfterTriggerBatchCallback()
>> clarifying the query_depth >=3D 0 precondition, a comment at the
>> AfterTriggerEndQuery call site explaining why
>> FireAfterTriggerBatchCallbacks() must precede the query_depth
>> decrement and AfterTriggerFreeQuery, and brief intent comments at the
>> AfterTriggerFireDeferred and AfterTriggerSetState call sites.
>>=20
>> Plan is to commit 0001 tomorrow barring objections and let it sit for
>> a bit before committing 0002. Feedback on 0002, particularly on the
>> AfterTriggerBatchCallback mechanism in trigger.c, welcome in the
>> meantime.
>=20
> Kept looking at 0002 and found a couple of things to improve or change
> my thoughts about.  I decided to move the permission check from fast
> path cache entry creation into ri_FastPathBatchFlush(), alongside the
> snapshot, so that permission changes between flushes are respected
> rather than checked once at batch start; the check happens for every
> row in the SPI and non-batched fast path.  Also, improved comments in
> a few places to mention design decisions better.
>=20
> 0001 is mostly unchanged from v11 except I updated its commit message
> to explain why only RI_FKey_check is covered and not the action
> triggers as the topic has come up in previous threads about this
> topic.
>=20
> Still planning to commit 0001 tomorrow.
>=20
> --=20
> Thanks, Amit Langote
> =
<v12-0001-Add-fast-path-for-foreign-key-constraint-checks.patch><v12-0002-=
Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch>

Hi Amit,

While reading the recent commits, I saw that 0001 has been pushed as =
2da86c1ef9b5446e0e22c0b6a5846293e58d98e3. However, I also just noticed a =
use-after-free issue in ri_LoadConstraintInfo(). It dereferences conForm =
after ReleaseSysCache(tup), which is unsafe. I am attaching a tiny patch =
to fix that.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/


--Apple-Mail=_1AB14FA3-7520-4FAA-9E08-DF39B58C6C0D
Content-Disposition: attachment;
	filename=v1-0001-Fix-a-use-after-problem-in-ri_LoadConstraintInfo.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v1-0001-Fix-a-use-after-problem-in-ri_LoadConstraintInfo.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=2085898f7825631f9a46b057ea486a766484c77f9b=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Tue,=2031=20Mar=202026=2017:06:12=20+0800=0ASubject:=20[PATCH=20=
v1]=20Fix=20a=20use-after-problem=20in=20ri_LoadConstraintInfo()=0A=0A=
Author:=20Chao=20Li=20<lic@highgo.com>=0A---=0A=20=
src/backend/utils/adt/ri_triggers.c=20|=204=20++--=0A=201=20file=20=
changed,=202=20insertions(+),=202=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/utils/adt/ri_triggers.c=20=
b/src/backend/utils/adt/ri_triggers.c=0Aindex=20da7640a8005..94bb180325b=20=
100644=0A---=20a/src/backend/utils/adt/ri_triggers.c=0A+++=20=
b/src/backend/utils/adt/ri_triggers.c=0A@@=20-2396,8=20+2396,6=20@@=20=
ri_LoadConstraintInfo(Oid=20constraintOid)=0A=20=09=09=09=09=09=09=20=20=
&riinfo->period_intersect_oper);=0A=20=09}=0A=20=0A-=09=
ReleaseSysCache(tup);=0A-=0A=20=09/*=0A=20=09=20*=20For=20efficient=20=
processing=20of=20invalidation=20messages=20below,=20we=20keep=20a=0A=20=09=
=20*=20doubly-linked=20count=20list=20of=20all=20currently=20valid=20=
entries.=0A@@=20-2412,6=20+2410,8=20@@=20ri_LoadConstraintInfo(Oid=20=
constraintOid)=0A=20=0A=20=09riinfo->fpmeta=20=3D=20NULL;=0A=20=0A+=09=
ReleaseSysCache(tup);=0A+=0A=20=09return=20riinfo;=0A=20}=0A=20=0A--=20=0A=
2.50.1=20(Apple=20Git-155)=0A=0A=

--Apple-Mail=_1AB14FA3-7520-4FAA-9E08-DF39B58C6C0D--