On 2018-Jul-24, Amit Langote wrote:
> Your patch takes care of allocation happening inside
> get_partition_for_tuple, but as you mention there might be others in its
> caller ExecFindPartition. So, I think we should switch to the per-tuple
> context in ExecFindPartition.
Right, makes sense. Pushed that way. I also moved the
ExecFetchSlotTuple call to happen after the memcxt change, because it
seemed to me that it may be possible for tuple_expand to allocate memory
(if not, it's not obvious). I also reworded some comments -- hope not
to have broken anything too bad there. I also renamed variable
"parent", which confused the heck out of me.
I had conflicts when applying this in master after developing it in
pg11, because of some new development there (and my variable rename). I
really hope we don't open the pg13 tree as early as we opened the pg12
one ...
> When I tried to do that, I discovered that we have to be careful about
> releasing some of the memory that's allocated in ExecFindPartition
> ourselves instead of relying on the reset of per-tuple context to take
> care of it. That's because some of the structures that ExecFindPartition
> assigns the allocated memory to are cleaned up at the end of the query, by
> when it's too late to try to release per-tuple memory. So, the patch I
> ended up with is slightly bigger than simply adding a
> MemoryContextSwitchTo() call at the beginning of ExecFindPartition.
Yeah, that stuff looks a bit brittle. I wish I had an idea on how to
make it less so. Thanks for taking care of that.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
