Re: tableam scan-API patch broke foreign key validation

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

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-04-06 18:13:29 Re: tableam scan-API patch broke foreign key validation
Previous Message Noah Misch 2019-04-06 18:08:39 Re: initdb recommendations