Hi Amit, Thanks for committing this, and for the extra analysis. Appreciate the message and comment polish too.
On Wed, Jun 17, 2026 at 10:18 AM Amit Langote <[email protected]> wrote: > > On Tue, Jun 16, 2026 at 8:19 PM Amit Langote <[email protected]> wrote: > > On Fri, Jun 12, 2026 at 1:02 PM Amit Langote <[email protected]> > > wrote: > > > On Fri, Jun 12, 2026 at 12:36 PM Ewan Young <[email protected]> wrote: > > > > > > > > Hi, > > > > > > > > Commit 2da86c1 ("Add fast path for foreign key constraint checks") makes > > > > a foreign-key column whose type is a domain over a type different from > > > > the referenced PK reject every valid row: > > > > > > > > CREATE DOMAIN int8dom AS int8; > > > > CREATE TABLE pk (a int4 PRIMARY KEY); > > > > CREATE TABLE fk (b int8dom REFERENCES pk(a)); > > > > INSERT INTO fk VALUES (1); > > > > ERROR: no conversion function from int8dom to integer > > > > > > > > It's new in v19 (verified by building 2da86c1^, where the insert > > > > succeeds); no released version is affected. The SPI path still handles > > > > it fine, e.g. with a partitioned PK. > > > > > > > > The fast path is the first caller to pass the cross-type pf_eq_oprs > > > > operator to ri_HashCompareOp(). Its "no cast needed" test, > > > > > > > > if (typeid == righttype) > > > > > > > > fails when the FK column is a domain over righttype (typeid is the > > > > domain OID), so it wrongly concludes no conversion exists and errors > > > > out. > > > > Looking through the domain fixes it -- conpfeqop is chosen against the > > > > FK base type, so getBaseType(typeid) == righttype holds for any valid > > > > FK: > > > > > > > > - if (typeid == righttype) > > > > + if (getBaseType(typeid) == righttype) > > > > > > > > Patch attached, with a regression test in foreign_key.sql. make check > > > > and the isolation suite pass. > > > > > > Thanks for the report and the patch. Will look next week. > > > > Your analysis looks correct and the patch makes sense. I looked > > through the code and convinced myself the new check is valid. > > > > One nuance: "holds for any valid FK" is true when pfeqop comes > > directly from the index opfamily (right-hand input > > getBaseType(fktype)). The PK = PK fallback uses opcintype instead, but > > there getBaseType can't equal it either, so the test still fails and > > the existing cast lookup runs unchanged. > > > > I polished it a bit for commit: tightened the message and trimmed the > > code comment. > > > > Will push to master tomorrow barring objections. > > Done: 68ace967c. > > -- > Thanks, Amit Langote
