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>
In-Reply-To: 
 <CA+HiwqHdB0r8z6Mut13BxpYNq2W-os+Arju4mcZbCyU9PeaVog@mail.gmail.com>
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 1 Apr 2026 19:56:10 +0800
Message-ID: 
 <CAEG8a3K5ayVNkCDnK9OtNb+4OY0chJtW6ObgEOSFjqyymQda8Q@mail.gmail.com>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
To: Amit Langote <amitlangote09@gmail.com>
Cc: Chao Li <li.evan.chao@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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAEG8a3K5ayVNkCDnK9OtNb%2B4OY0chJtW6ObgEOSFjqyymQda8Q%40mail.gmail.com>
Precedence: bulk

On Wed, Apr 1, 2026 at 5:51=E2=80=AFPM Amit Langote <amitlangote09@gmail.co=
m> wrote:
>
> On Wed, Apr 1, 2026 at 5:51=E2=80=AFPM Amit Langote <amitlangote09@gmail.=
com> wrote:
> > On Wed, Apr 1, 2026 at 12:54=E2=80=AFAM Junwang Zhao <zhjwpku@gmail.com=
> wrote:
> > > +       if (riinfo->fpmeta =3D=3D NULL)
> > > +       {
> > > +               /* Reload to ensure it's valid. */
> > > +               riinfo =3D ri_LoadConstraintInfo(riinfo->constraint_i=
d);
> > >
> > > I was thinking of wrapping the reload in a conditional check like
> > > `!riinfo->valid`, since `riinfo` can be valid even when `fpmeta =3D=
=3D NULL`.
> > > However,  `if (riinfo->fpmeta =3D=3D NULL)` should rarely be true, so=
 the
> > > unconditional reload is harmless, and the code is cleaner.
> > >
> > > +1 to the fix.
> >
> > Thanks for checking.
> >
> > I have just pushed a slightly modified version of that.
> >
> > > > 0002 is the rebased batching patch.
> > >
> > > The change of RI_FastPathEntry from storing riinfo to fk_relid
> > > makes sense to me. I'll do another review on 0002 tomorrow.
> >
> > Here's another version.
> >
> > This time, I have another fixup patch (0001) to make FastPathMeta
> > self-contained by copying the FmgrInfo structs it needs out of
> > RI_CompareHashEntry rather than storing pointers into it.  This avoids
> > any dependency on those cache entries remaining stable.  I'll push
> > that once the just committed patch has seen enough BF animals.
>
> Pushed.
>
> > 0002 is rebased over that.
>
> Rebased again.

+static void
+ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel)
+{
+ /* Reload; may have been invalidated since last batch accumulation. */
+ const RI_ConstraintInfo *riinfo =3D ri_LoadConstraintInfo(fpentry->conoid=
);

...
+ if (riinfo->fpmeta =3D=3D NULL)
+ {
+ /* Reload to ensure it's valid. */
+ riinfo =3D ri_LoadConstraintInfo(riinfo->constraint_id);
+ ri_populate_fastpath_metadata((RI_ConstraintInfo *) riinfo,
+   fk_rel, idx_rel);
+ }

ri_LoadConstraintInfo is currently invoked twice within
ri_FastPathBatchFlush. Should we eliminate the second call?

Alternatively, we could refactor ri_FastPathBatchFlush to accept
an additional parameter, `const RI_ConstraintInfo *riinfo`, so we
can remove the need for the first call. In that case, we need to call
ri_LoadConstraintInfo in ri_FastPathEndBatch.

>
> --
> Thanks, Amit Langote


--=20
Regards
Junwang Zhao