Re: Asymmetry in opening and closing indices for partition routing

Hi Ashutosh,

On 2020-Jun-22, Ashutosh Bapat wrote:

> I couldn't find code where this can happen but I don't see any code which
> prevents this. This looks like a recipe for memory and reference leaks.
> 
> We could fix this by
> 1. Make ExecOpenIndices and ExecCloseIndices so that they can be called
> multiple times on the same relation similar to heap_open. The second time
> onwards ExecOpenIndices doesn't allocate memory and open indexes but
> increases a refcount. ExecCloseIndices releases memory and closes the index
> relations when the refcount drops to 0. Then we don't need to
> check leaf_part_rri->ri_IndexRelationDescs == NULL in
> ExecInitPartitionInfo().

I think there are a couple of places in executor related to partition
tuple routing where code is a bit weirdly structured.  It might be nice
to improve on that if you either find inefficiencies that can be fixed,
or clear code structure improvements, as long as they don't make
performance worse.  Feel free to have a look around and see if you can
propose some concrete proposals.

I'm not sure that expecting the relcache entry's refcount drops to zero
at the right time is a good approach; that may cause leaks some other
place might have refcounts you're not expecting (say, an open cursor
that's not fully read).

(I'm not terribly worried about refcount leakage as a theoretical
concern, since the ResourceOwner mechanism will warn us about that if it
happens.)

> 2. Throw an error in ExecOpenIndices if all the arrays are present. We will
> need to check leaf_part_rri->ri_IndexRelationDescs == NULL in
> ExecInitPartitionInfo().

This sounds like a job for an assert rather than an error.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services