Hi Amit,
On 2017/08/17 21:18, Amit Khandekar wrote:
> Anyways, some more comments :
>
> In ExecSetupPartitionTupleRouting(), not sure why ptrinfos array is an
> array of pointers. Why can't it be an array of
> PartitionTupleRoutingInfo structure rather than pointer to that
> structure ?
AFAIK, assigning pointers is less expensive than assigning struct and we
end up doing a lot of assigning of the members of that array to a local
variable in get_partition_for_tuple(), for example. Perhaps, we could
avoid those assignments and implement it the way you suggest.
> diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
> + * Close all the leaf partitions and their indices.
> *
> Above comment needs to be shifted a bit down to the subsequent "for"
> loop where it's actually applicable.
That's right, done.
> * node->mt_partition_dispatch_info[0] corresponds to the root partitioned
> * table, for which we didn't create tupslot.
> Above : node->mt_partition_dispatch_info[0] => node->mt_ptrinfos[0]
Oops, fixed.
> /*
> * XXX- do we need a pinning mechanism for partition descriptors
> * so that there references can be managed independently of
> * the parent relcache entry? Like PinPartitionDesc(partdesc)?
> */
> pd->partdesc = partdesc;
>
> Any idea if the above can be handled ? I am not too sure.
A similar mechanism exists for TupleDesc ref-counting (see the usage of
PinTupleDesc and ReleaseTupleDesc across the backend code.) I too am
currently unsure if such an elaborate mechanism is actually *necessary*
for rd_partdesc.
Attached updated patches.
Thanks,
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers