On 25 August 2017 at 23:58, Robert Haas <robertmh...@gmail.com> wrote: > > That just leaves indexes. In a world where keystate, tupslot, and > tupmap are removed from the PartitionDispatchData, you must need > indexes or there would be no point in constructing a > PartitionDispatchData object in the first place; any application that > needs neither indexes nor the executor-specific stuff could just use > the Relation directly.
But there is expand_inherited_rtentry() which neither requires indexes nor any executor stuff, but still requires to call RelationGetPartitionDispatchInfo(), and so these indexes get built unnecessarily. Looking at the latest patch, I can see that those indexes can be separately built in ExecSetupPartitionTupleRouting() where it is required, instead of in RelationGetPartitionDispatchInfo(). In the loop which iterates through the pd[] returned from RelationGetPartitionDispatchInfo(), we can build them using the exact code currently written to build them in RelationGetPartitionDispatchInfo(). In the future, if we require such applications where indexes are also required, we may have a separate function only to build indexes, and then ExecSetupPartitionTupleRouting() would also call that function. -------- On 21 August 2017 at 11:40, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: >> 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 I didn't get why exactly we would have to copy the structures. We could just pass the address of ptrinfos[index], no ? My only point for this was : we would not have to call palloc0() for each of the element of ptrinfos. Instead, just allocate memory for all of the elements in a single palloc0(). We anyways have to allocate memory for *each* of the element. > in get_partition_for_tuple(), for example. Perhaps, we could > avoid those assignments and implement it the way you suggest. You mean at these 2 places in get_partition_for_tuple() , right ? : 1. /* start with the root partitioned table */ parent = ptrinfos[0]; 2. else parent = ptrinfos[-parent->pd->indexes[cur_index]]; Both of the above places, we could just use &ptrinfos[...] instead of ptrinfos[...]. But I guess you meant something else. ------------ RelationGetPartitionDispatchInfo() opens all the partitioned tables. But in ExecSetupPartitionTupleRouting(), it again opens all the parents, that is all the partitioned tables, and closes them back. Instead, would it be possible to do this : Instead of the PartitionDispatch->parentoid field, PartitionDispatch can have parentrel Relation field, which will point to reldesc field of one of the pds[] elements. ------------ For me, the CopyStateData->ptrinfos and ModifyTableState.mt_ptrinfos field names sound confusing. How about part_tuple_routing_info or just tuple_routing_info ? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
