On Fri, Nov 16, 2018 at 1:00 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Fri, Nov 16, 2018 at 10:57:57AM +0900, Amit Langote wrote:
> > Maybe partition_bounds_create() should've had a MemoryContext argument to
> > pass it the context we want it to create the PartitionBoundInfo in. That
> > way, we can simply pass rd_pdcxt to it and avoid making a copy. As is,
> > we're now allocating two copies of PartitionBoundInfo, one in the
> > CurrentMemoryContext and another in rd_pdcxt, whereas the previous code
> > would only allocate the latter. Maybe we should fix it as being a regression.
>
> Not sure about what you mean by regression here,
The regression is, as I mentioned, that the new code allocates two
copies of PartitionBoundInfo whereas only one would be allocated
before.
> but passing the memory
> context as an argument has sense as you can remove the extra partition
> bound copy, as it has sense to use an array instead of a list for
> performance, which may matter if many partitions are handled when
> building the cache. So cleaning up both things at the same time would
> be nice.
Maybe, the patch to add the memory context argument to
partition_bound_create and other related static functions in
partbound.c should be its own patch, as that seems to be a separate
issue. OTOH, other changes needed to implement Robert's proposal of
using PartitionBoundSpec and Oid arrays instead of existing lists
should be in the same patch.
Thanks,
Amit