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

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: RI fast path gets cross-type foreign keys wrong
Date: 2026-08-06 14:11:54
Message-ID: CA+HiwqGRTjfkov4-oV=CpMptUeFrD1EgoA4r+bYzOGWXB=MFuA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

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.

Also, added an open item for this.

--
Thanks, Amit Langote

Attachment Content-Type Size
v1-0001-Fix-cross-type-foreign-keys-in-the-batched-fast-p.patch application/octet-stream 10.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Nazir Bilal Yavuz 2026-08-06 14:04:27 Re: [PATCH] Extending FK check skipping on replicas to ADD FK and TRUNCATE