On Thu, 29 Sept 2022 at 18:30, David Rowley <dgrowleyml@gmail.com> wrote:
> Does anyone have any opinions on this?
I by no means think I've nailed the fix in
other_ideas_to_fix_MemoryContextContains.patch, so it would be good to
see if anyone else has any new ideas on how to solve this issue.
Andres did mention to me off-list about perhaps adding a boolean field
to FunctionCallInfoBaseData to indicate if the return value can be
assumed to be in CurrentMemoryContext. I feel like that might be
quite a bit of work to go and change all functions to ensure that
that's properly populated. For example, look at split_part() in
varlena.c, it's going to be a little tedious to ensure we set that
field correctly there as that function sometimes returns it's input,
sometimes returns a string constant and sometimes allocates new
memory. I feel fixing it this way will be error-prone and cause lots
of future bugs.
I'm also aware that the change made in b76fb6c2a becomes less
temporary with each day that passes, so I really would like to find a
solution to the MemoryContextContains issue. I'll reiterate that I
don't think reverting c6e0fe1f2 fixes MemoryContextContains. That
would just put back the behaviour of it returning true based on the
owning MemoryContext and/or the direction that the wind is coming from
on the particular day the function is called.
Although I do think other_ideas_to_fix_MemoryContextContains.patch
does fix the problem. I also fear a few people would be reaching for
their pitchforks if I was to go and commit it. However, as of now, I'm
starting to look more favourably at it as more time passes.
David
