On Fri, Sep 9, 2022 at 4:54 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Andres Freund <and...@anarazel.de> writes: > > Something here doesn't look to be quite right. Starting with this commit CI > > [1] started to fail on freebsd (stack trace [2]), and in the meson branch > > I've > > also seen the crash on windows (CI run[3], stack trace [4]). > > The crash seems 100% reproducible if I remove the early-exit optimization > from GetForeignKeyActionTriggers:
Indeed, reproduced here. > diff --git a/src/backend/commands/tablecmds.c > b/src/backend/commands/tablecmds.c > index 53b0f3a9c1..112ca77d97 100644 > --- a/src/backend/commands/tablecmds.c > +++ b/src/backend/commands/tablecmds.c > @@ -10591,8 +10591,6 @@ GetForeignKeyActionTriggers(Relation trigrel, > Assert(*updateTriggerOid == InvalidOid); > *updateTriggerOid = trgform->oid; > } > - if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid)) > - break; > } > > if (!OidIsValid(*deleteTriggerOid)) > > With that in place, it's probabilistic whether the Asserts notice anything > wrong, and mostly they don't. But there are multiple matching triggers: > > regression=# select oid, tgconstraint, tgrelid,tgconstrrelid, tgtype, tgname > from pg_trigger where tgconstraint = 104301; > oid | tgconstraint | tgrelid | tgconstrrelid | tgtype | tgname > --------+--------------+---------+---------------+--------+------------------------------- > 104302 | 104301 | 104294 | 104294 | 9 | > RI_ConstraintTrigger_a_104302 > 104303 | 104301 | 104294 | 104294 | 17 | > RI_ConstraintTrigger_a_104303 > 104304 | 104301 | 104294 | 104294 | 5 | > RI_ConstraintTrigger_c_104304 > 104305 | 104301 | 104294 | 104294 | 17 | > RI_ConstraintTrigger_c_104305 > (4 rows) > > I suspect that the filter conditions being applied are inadequate > for the case of a self-referential FK, which this evidently is > given that tgrelid and tgconstrrelid are equal. Yes, the loop in GetForeignKeyActionTriggers() needs this: + /* Only ever look at "action" triggers on the PK side. */ + if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_PK) + continue; Likewise, GetForeignKeyActionTriggers() needs this: + /* Only ever look at "check" triggers on the FK side. */ + if (RI_FKey_trigger_type(trgform->tgfoid) != RI_TRIGGER_FK) + continue; We evidently missed this in f4566345cf40b0. > I'd counsel dropping the early-exit optimization; it doesn't > save much I expect, and it evidently hides bugs. Or maybe > make it conditional on !USE_ASSERT_CHECKING. While neither of these functions are called in hot paths, I am inclined to keep the early-exit bit in non-assert builds. Attached a patch. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
action-check-trigger-filter.patch
Description: Binary data
