On 2018/11/15 15:22, Michael Paquier wrote:
>> If there are no partitions, nparts is 0, and other fields are NULL, though
>> rd_partdesc itself is never NULL.
>
> I find a bit confusing that both concepts have the same meaning, aka
> that a relation has no partition, and that it is actually relkind which
> decides rd_partdesc should be NULL or set up. This stuff also does
> empty allocations.
>
>> If we want to redesign that and allow it to be NULL until some code in the
>> backend wants to use it, then maybe we can consider doing what you say.
>> But, many non-trivial operations on partitioned tables require the
>> PartitionDesc, so there is perhaps not much point to designing it such
>> that rd_partdesc is set only when needed, because it will be referenced
>> sooner than later. Maybe, we can consider doing that sort of thing for
>> boundinfo, because it's expensive to build, and not all operations want
>> the canonicalized bounds.
>
> I am fine if that's the consensus of this thread. But as far as I can
> see it is possible to remove a bit of the memory handling mess by doing
> so. My 2c.
Perhaps, we can discuss this another thread. I know this thread contains
important points about partition descriptor creation and modification, but
memory context considerations seems like a separate topic. The following
message could be a starting point, because there we were talking about
perhaps a similar design as you're saying:
https://www.postgresql.org/message-id/143ed9a4-6038-76d4-9a55-502035815e68%40lab.ntt.co.jp
Also, while I understood Alvaro's and your comment on the other thread
that memory handling is messy as is, but sorry, it's not clear to me why
you say this patch makes it messier. It reduces context switch calls so
that RelationBuildPartitionDesc roughly looks like this after the patch:
Start with CurrentMemoryContext...
1. read catalogs and make bounddescs and oids arrays
2. partition_bounds_create(...)
3. create and switch to rd_pdcxt
4. create PartitionDesc, copy partdesc->oids and partdesc->boundinfo
5. switch back to the old context
Thanks,
Amit