Re: RI fast path gets cross-type foreign keys wrong

Hi,

On Thu, Aug 6, 2026 at 11:11 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Sun, Aug 2, 2026 at 12:59 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Sat, Aug 1, 2026 at 8:49 Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >> On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> >>>
> >>> The attached isolation test patch (written by Claude code) shows that
> >>> the new RI fast path can get some things wrong when a foreign key uses
> >>> a cross-type equality operator.
> >>>
> >>> The test has two permutations differing only in the primary key's type
> >>> -- date against a timestamp FK in the first, timestamp against
> >>> timestamp in the second. The interleaving is identical and the
> >>> referenced key is present throughout, so both should behave the same
> >>> way. The second succeeds (which is correct), while the first fails
> >>> with a foreign key violation (which is incorrect).
> >>>
> >>> The fast path was introduced by b7b27eb41. This is an issue on 19 and
> >>> master only.
> >>
> >>
> >> Thanks Peter for the report. I’m on vacation atm, will take a look when I get back next week.
> >
> > Still away, but couldn’t help reading the code on my phone.
> >
> > The recheck block in ri_FastPathFlushArray() appears to put the key it has just read out of the locked tuple (found_val) into sk_argument, and then pass that same slot to recheck_matched_pk_tuple(). If I’m reading that right, both operands come from the locked tuple, so it compares the key with itself and since sk_argument is the right-hand operand, which on a cross-type operator is the FK-typed side, the PK value there gets read as an FK value. That would make the same-type case pass trivially and the cross-type case fail always, which seems to fit what Peter is seeing.
> >
> > If that’s really what’s going on, maybe the recheck can just go away, because the loop a few lines further down in the same scan iteration already compares found_val against the buffered FK values. I’ll look at it properly next week.
>
> The reading on the phone was right, so removing the recheck works and
> is fine. The marking loop a few lines below already compares
> found_val, read after the chain has been followed, against every
> buffered FK value, with the arguments in the order the operator
> expects, so a key that has moved away matches nothing and we still
> report the violation. Reorienting the scan key isn't an option, since
> sk_argument would have to be the particular buffered value that this
> tuple matched, and the AM doesn't expose which array element that was.
>
> ri_FastPathProbeOne() passes its original scan key, with the FK value
> still in sk_argument, so it was never affected. Single-row statements
> go through it, and Peter's test passes on unpatched HEAD if the INSERT
> adds one row instead of two.
>
> Attached is a patch. I kept Peter's test and added a permutation that
> leaves the key where it was moved, so the check must report a
> violation.

Would like to commit tomorrow, barring objections.

--
Thanks, Amit Langote