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

On Mon, Oct 5, 2026 at 10:18 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> Hi,
>
> While reviewing the RI fast path after the batching removal, with help
> from Claude Opus 5.5, I found more issues suggesting that directly
> calling the cast function using FunctionCall3() might not be such a
> good idea. Nikolay's report upthread, where a cast is redefined while
> ri_triggers.c's internal cache still uses the old definition causing
> "cache lookup failed for function", is only one of the issues caused
> by it. When comparing how the corresponding SPI query containing the
> same cast handles necessary cases, Opus found more issues resulting
> from directly calling the cast function. For example, a cast
> returning NULL also raises an internal "function ... returned NULL"
> instead of a FK violation as shown here (Opus' example):
>
> CREATE TABLE pk (id int4 PRIMARY KEY);
> INSERT INTO pk VALUES (1);
> CREATE TYPE word AS ENUM ('one', 'none');
> CREATE FUNCTION word_to_int4(word) RETURNS int4
> LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
> THEN 1 END $$;
> CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
> CREATE TABLE fk_word (a word REFERENCES pk, n int);
> INSERT INTO fk_word VALUES ('one', 0);
> INSERT 0 1
> INSERT INTO fk_word VALUES ('none', 0);
> ERROR: function 16397 returned NULL
>
> If the PK table is partitioned, the fast path is not taken, and you
> get proper handling of the cast function defined this way:
>
> CREATE TABLE pk (id int4 PRIMARY KEY) PARTITION BY LIST (id);
> CREATE TABLE pk_part PARTITION OF pk DEFAULT;
> INSERT INTO pk VALUES (1);
> CREATE TYPE word AS ENUM ('one', 'none');
> CREATE FUNCTION word_to_int4(word) RETURNS int4
> LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
> THEN 1 END $$;
> CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
> CREATE TABLE fk_word (a word REFERENCES pk, n int);
> INSERT INTO fk_word VALUES ('one', 0);
> INSERT INTO fk_word VALUES ('none', 0);
> ERROR: insert or update on table "fk_word" violates foreign key
> constraint "fk_word_a_fkey"
> DETAIL: Key (a)=(none) is not present in table "pk".
>
> 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.
>
> Note that these errors are also reachable in the
> RI_FKey_fk_upd_check_required() -> ri_KeysEqual() path, which predates
> the addition of the fast-path. It compares the old and new key of an
> updated referencing row by calling the cast function the same way, so
> with the setup of the first example above:
>
> UPDATE fk_word SET a = 'none';
> ERROR: function 16467 returned NULL
>
> I wrote a patch to fix that too (0003). The lack of field complaints
> might stem from people rarely setting things up this way in the real
> world, but I think it's worth committing. The fix is simple: when a
> cast function would be needed, compare the bytes of the old and new
> values instead. At worst, this runs a check that could have been
> skipped.
>
> 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).

Added an open item, though I'll try to commit 0001 and 0002 no later
than tomorrow.

--
Thanks, Amit Langote