Re: ri_Fast* crash w/ nullable UNIQUE constraint

Hi,

On Tue, 21 Jul 2026 at 11:27, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:

> Hi Noah,
>
> On Sat, Jul 18, 2026 at 12:25 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> >
> > On Thu, Jul 16, 2026 at 08:32:11PM +0900, Amit Langote wrote:
> > > Btw, do we want to tag Opus as the reporter and the author in the
> > > commit message as you did in your version of 0001?
> >
> > I don't have an opinion on that. It generated the commit message. I'm
> > neither endorsing nor opposing inclusion of those lines.
> >
> > > I'd rather not as
> > > that's not something the project has started doing unless I have
> > > missed. Since you found and reported the bug, I decided to tag you as
> > > the reporter in all of the patches for now.
> >
> > That decision works for me.
>
> Thanks for confirming.
>
> I plan to go on vacation starting Sunday for a couple of weeks, so I
> was hoping to push these patches this week after taking another look.
> If you or anyone else has comments, please let me know.
>

Thanks for the patches!

I was reviewing it yesterday and got stalled on 0004, went through
rest today. 0001 to 0003 looked good to me.

One thing I got stuck on in 0004 (resettable fn_mcxt): each flush does
MemoryContextReset(scratch_cxt), but I couldn't find where the cached
FmgrInfos' fn_extra is cleared. record_eq() (and some cast/eq
functions) cache state via fn_extra allocated in fn_mcxt, so after the
reset fn_extra seems to dangle, and the next flush reusing the same
FmgrInfo reads it back as valid; fmgr_info_copy() zeroes fn_extra for
what looks like this reason. With a composite key over two batches,
and an assert after the reset, fn_extra was non-NULL on the second
batch. Would clearing fn_extra (or re-copying the FmgrInfos) on reset
make sense, maybe with a record-typed two-batch test?

Also, the reset is only in ri_FastPathBatchFlush(); ri_FastPathCheck()
(ALTER TABLE validate / sub-transaction path) uses the same cached
FmgrInfos and scratch_cxt but doesn't reset. I wonder if the growth this
patch targets still applies there.

Doubt: 0005 moves index_beginscan() after the user-id switch in
ri_FastPathCheck(), but ri_FastPathBatchFlush() still begins the scan
before the switch. Is that intentional, or worth the same reordering?

Regards,
Ayush