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

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).

--
Thanks, Amit Langote

[1] https://www.postgresql.org/message-id/CA%2BHiwqFaipkMsZ8XP-9sMk21h0Q7rih%3DNpJQXsBAaD39VtnrOA%40mail.gmail.com