Re: 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: Re: tableam scan-API patch broke foreign key validation
Date: 2019-04-06 18:34:34
Message-ID: 19906.1554575674@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> The relevant thread is:
> https://www.postgresql.org/message-id/20190325180405.jytoehuzkeozggxx%40alap3.anarazel.de

Yeah, I just found that --- would have seen it sooner if David had
not elected to make it a new thread.

> Wonder if you have an opinion on:

>> I've also noticed that we should free the tuple - that doesn't matter
>> for heap, but it sure does for other callers.

Why should this code need to free anything? That'd be the responsibility
of the slot code, no?

>> But uh, is it actually ok
>> to validate an entire table's worth of foreign keys without a memory
>> context reset? I.e. shouldn't we have a memory context that we reset
>> after each iteration?
>> Also, why's there no CHECK_FOR_INTERRUPTS()? heap has some internally on
>> a page level, but that doesn't seem all that granular?

These are good questions. Just eyeing RI_FKey_check(), I think
that it might not have any significant leaks because most of the work
is done in an SPI context, but obviously that's pretty fragile.

The memory-context stuff in your WIP patch seems wrong, btw;
the second or later iteration of the loop would trash oldcxt.

But clearly we need a test case here. I'll adjust Hadi's example
so that there's more than one tuple to check, and push it.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-04-06 18:38:34 Re: tableam scan-API patch broke foreign key validation
Previous Message Andres Freund 2019-04-06 18:22:58 Re: tableam scan-API patch broke foreign key validation