RI fast path gets cross-type foreign keys wrong

Started by Peter Geoghegan8 days ago9 messageshackers
Jump to latest

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.

--
Peter Geoghegan

Attachments:

0001-Add-isolation-test-for-the-RI-fast-path-s-cross-type.patchapplication/octet-stream; name=0001-Add-isolation-test-for-the-RI-fast-path-s-cross-type.patchDownload+72-1
#2Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Peter Geoghegan (#1)
Re: RI fast path gets cross-type foreign keys wrong

On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <pg@bowt.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.

- Amit

Show quoted text
#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#2)
Re: RI fast path gets cross-type foreign keys wrong

On Sat, Aug 1, 2026 at 8:49 Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <pg@bowt.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.

- Amit

Show quoted text
#4Mihail Nikalayeu
mihailnikalayeu@gmail.com
In reply to: Amit Langote (#3)
Re: RI fast path gets cross-type foreign keys wrong

Hello, everyone!

I've (or AI?) found another fast-path/foreign-key - related problem which the
stress-test framework I am prototyping detected. I think this's okay
to send it to that thread; if not, please tell me, and I'll create
another one.

It was all built by Claude; sorry, I have no cycles to carefully
validate all details (there are many findings like this), but it looks
valid - it reproduces the error and even data corruption with just two
new injection points.
I think Amit will be able to understand it all clearly and easily as
the author. If not for some reason, please tell me, and I'll handle it
myself later.

That's a strange time to live :)

Best regards,
Mikhail.

Attachments:

v1-0001-Reproducer-RI-fast-path-opens-an-index-a-concurre.patchapplication/x-patch; name=v1-0001-Reproducer-RI-fast-path-opens-an-index-a-concurre.patchDownload+162-1
v1-0003-Re-read-conindid-under-the-referenced-table-s-loc.patchapplication/x-patch; name=v1-0003-Re-read-conindid-under-the-referenced-table-s-loc.patchDownload+19-1
v1-0002-Reproducer-RI-fast-path-accepts-a-row-whose-refer.patchapplication/x-patch; name=v1-0002-Reproducer-RI-fast-path-accepts-a-row-whose-refer.patchDownload+195-2
#5Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Mihail Nikalayeu (#4)
Re: RI fast path gets cross-type foreign keys wrong

On Sun, Aug 2, 2026 at 4:28 Mihail Nikalayeu <mihailnikalayeu@gmail.com>
wrote:

Hello, everyone!

I've (or AI?) found another fast-path/foreign-key - related problem which
the
stress-test framework I am prototyping detected. I think this's okay
to send it to that thread; if not, please tell me, and I'll create
another one.

It was all built by Claude; sorry, I have no cycles to carefully
validate all details (there are many findings like this), but it looks
valid - it reproduces the error and even data corruption with just two
new injection points.
I think Amit will be able to understand it all clearly and easily as
the author. If not for some reason, please tell me, and I'll handle it
myself later.

Thanks Mihail. Adding this to my list to get back to when I’m back next week
- Amit

Show quoted text
#6Mihail Nikalayeu
mihailnikalayeu@gmail.com
In reply to: Amit Langote (#5)
Re: RI fast path gets cross-type foreign keys wrong

Hello!

Thanks Mihail. Adding this to my list to get back to when I’m back next

week

This is one more in the same area (also caught by the stress suite).

The fix and test crafted by Claude were validated and adjusted by me
(but not too carefully).

Enjoy your vacation!
Mikhail.

Attachments:

nocfbot-v1-0001-Re-read-the-FK-constraint-after-locking-the-refer.patchapplication/x-patch; name=nocfbot-v1-0001-Re-read-the-FK-constraint-after-locking-the-refer.patchDownload+380-1
#7Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Mihail Nikalayeu (#6)
Re: RI fast path gets cross-type foreign keys wrong

Hi Mihail,

On Wed, Aug 5, 2026 at 16:58 Mihail Nikalayeu <mihailnikalayeu@gmail.com>
wrote:

Hello!

Thanks Mihail. Adding this to my list to get back to when I’m back next

week

This is one more in the same area (also caught by the stress suite).

The fix and test crafted by Claude were validated and adjusted by me
(but not too carefully).

Enjoy your vacation!

Just got back.

Could you please start a separate thread after all for these issues? It
would be better to keep the discussions separate.

Thanks again for your investigations.

- Amit

Show quoted text
#8Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#3)
Re: RI fast path gets cross-type foreign keys wrong

Hi,

On Sun, Aug 2, 2026 at 12:59 AM Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, Aug 1, 2026 at 8:49 Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <pg@bowt.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

Attachments:

v1-0001-Fix-cross-type-foreign-keys-in-the-batched-fast-p.patchapplication/octet-stream; name=v1-0001-Fix-cross-type-foreign-keys-in-the-batched-fast-p.patchDownload+111-25
#9Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#8)
Re: RI fast path gets cross-type foreign keys wrong

Hi,

On Thu, Aug 6, 2026 at 11:11 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Sun, Aug 2, 2026 at 12:59 AM Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, Aug 1, 2026 at 8:49 Amit Langote <amitlangote09@gmail.com> wrote:

On Sat, Aug 1, 2026 at 8:33 Peter Geoghegan <pg@bowt.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