Hi,
On 2019-04-06 14:34:34 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.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?
Well, not really. If a slot doesn't hold heap tuples internally,
ExecFetchSlotHeapTuple() will return a fresh heap tuple (but signal so
by setting *should_free = true if not NULL). That's why I was saying it
doesn't matter for heap (where the slot just holds a heap tuple
internally), but it does matter for other AMs.
> >> 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.
Yea. And especially with potentially needing to free the tuple as above,
using an explicit context seems more robust to me.
> 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.
Cool.
Greetings,
Andres Freund