Re: tableam scan-API patch broke foreign key validation

Hi,

On April 6, 2019 11:07:55 AM PDT, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>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.

I posted pretty much that patch nearby, with some other questions. Was waiting for David to respond.... Let me dig that out.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.