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

On Wed, Apr 1, 2026 at 5:51 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Wed, Apr 1, 2026 at 5:51 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Wed, Apr 1, 2026 at 12:54 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
> > > +       if (riinfo->fpmeta == NULL)
> > > +       {
> > > +               /* Reload to ensure it's valid. */
> > > +               riinfo = ri_LoadConstraintInfo(riinfo->constraint_id);
> > >
> > > I was thinking of wrapping the reload in a conditional check like
> > > `!riinfo->valid`, since `riinfo` can be valid even when `fpmeta == NULL`.
> > > However,  `if (riinfo->fpmeta == 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 = ri_LoadConstraintInfo(fpentry->conoid);

...
+ if (riinfo->fpmeta == NULL)
+ {
+ /* Reload to ensure it's valid. */
+ riinfo = 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


-- 
Regards
Junwang Zhao