Re: PG19: two RI fast-path issues found while testing the batching revert

On Mon Oct 5, 2026 at 10:18 AM -03, Amit Langote wrote:
> So I propose fixing the fast-path selection criteria to avoid it when
> such a cast is necessary, as done in the attached 0001. 0002 is a
> patch to drop the code added in the fast-path commit to facilitate
> this cast handling as it is now dead code. 0001+0002 apply on top of
> the other fix I posted at [1], which I plan to commit tomorrow.
>
> [ ... ]
>
> I would like to commit 0001 and 0002 before RC1 next week, since
> without them a foreign key using such a cast is checked differently
> depending on whether the fast path or SPI handles it. I can wait for
> others to opine on whether 0003 is warranted (I haven't checked the
> back-patching pain).
>

Hi, thanks for the patches! I've reviewed and tested 0001 and 0002 and I
dind't find any issues.

I also noticed that a user-defined WITH INOUT AS IMPLICIT cast now works
through SPI. Before 0001 it failed with "no conversion function" from
ri_HashCompareOp(), so perhaps it's worth adding a test for it.

The pg_cast invalidation also seems to work fine. I tested it with a
session that had cached the fast path for a FK using a binary cast,
while another session replaced the cast with a function returning NULL,
and the next insert on the first session went through SPI and reported
the FK violation. The same works when the cast is replaced within the
same transaction.

I have just a single comment: I'm wondering if the tests that 0001
rewords (fk_defer_main, the fkint invalidation test, fp_reentry and the
ri_snapshot STABLE test) now all seems to go through SPI, so they no
longer test the fast path issues they were written for, what do you
think?

Some minor comments:

- "getBaseType(fk_type) != righttype &&" on the new check seems
redundant since IsBinaryCoercible() already handles the same type and
domain cases.

- The "typedef struct RI_CompareHashEntry RI_CompareHashEntry;" added
by 2da86c1ef9 before FastPathMeta is not needed anymore.

- The ri_check_fastpath_index() header comment only mentions index
properties, but the new check depends on the FK table. The comment
before the fast path call on RI_FKey_check() also only mentions
non-partitioned and non-temporal FKs.

- "pk_fast" on the ri_snapshot test now also goes through SPI, so the
name is a may be misleading.

I'll review 0003 next.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com