On Fri, Aug 7, 2026 at 10:16 PM Amit Langote <[email protected]> wrote: > On Thu, Aug 6, 2026 at 11:11 PM Amit Langote <[email protected]> wrote: > > On Sun, Aug 2, 2026 at 12:59 AM Amit Langote <[email protected]> > > wrote: > > > On Sat, Aug 1, 2026 at 8:49 Amit Langote <[email protected]> wrote: > > >> On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <[email protected]> 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.
Pushed (cc449a504bf) and marked open item for this issue as fixed. -- Thanks, Amit Langote
