tableam scan-API patch broke foreign key validation

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Hadi Moshayedi <hadi(at)moshayedi(dot)net>
Subject: tableam scan-API patch broke foreign key validation
Date: 2019-04-06 18:07:55
Message-ID: 19030.1554574075@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

It seems that the fire-the-triggers code path in
validateForeignKeyConstraint isn't being exercised; at least, that's
what coverage.postgresql.org says right now, and I'm afraid that may
have been true for quite some time. The attached regression-test
addition causes it to be exercised, and guess what: it blows up real
good.

This is a slightly adapted version of the test Hadi proposed in
https://postgr.es/m/CAK=1=WonwcuN_0KiZwQO3SQxse41jZ5hOJRpFCvZ3qa8n9cssw@mail.gmail.com
Since he didn't mention anything about core dumps or assertion
failures, one assumes that it did work as of the version he was
testing against.

What it looks like to me is that because of this hunk in c2fe139c2:

@@ -8962,7 +8981,8 @@ validateForeignKeyConstraint(char *conname,
trigdata.type = T_TriggerData;
trigdata.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;
trigdata.tg_relation = rel;
- trigdata.tg_trigtuple = tuple;
+ trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot, true, NULL);
+ trigdata.tg_trigslot = slot;
trigdata.tg_newtuple = NULL;
trigdata.tg_trigger = &trig;

validateForeignKeyConstraint asks ExecFetchSlotHeapTuple to materialize
the tuple, which causes it to no longer be associated with a buffer,
which causes heapam_tuple_satisfies_snapshot to be very unhappy.

I can make the case not crash by s/true/false/ in the above call,
but I wonder whether that's an appropriate fix. It seems rather
fragile that things work like this.

I plan to go ahead and commit Hadi's fix with that change included
(as below), but I wonder whether anything else needs to be revisited.

regards, tom lane

Attachment Content-Type Size
fix-foreign-key-check-3.patch text/x-diff 4.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2019-04-06 18:08:39 Re: initdb recommendations
Previous Message Tom Lane 2019-04-06 16:35:47 Re: [PATCH] Implement uuid_version()