On 2018/01/04 23:29, Ashutosh Bapat wrote:
> On Fri, Dec 29, 2017 at 6:32 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> I happened to notice that Ashutosh's patch series at
>> https://www.postgresql.org/message-id/CAFjFpReJhFSoy6DqH0ipFSHd=sLNEkSzAtz4VWCaS-w2jZL=uw@mail.gmail.com
>> has a 0001 patch that modifies the partition_bound_cmp stuff too.
>> Are those conflicting?
>>
>> Ashutosh's commit message:
>> Modify bound comparision functions to accept members of PartitionKey
>>
>> Functions partition_bound_cmp(), partition_rbound_cmp() and
>> partition_rbound_datum_cmp() are required to merge partition bounds
>> from joining relations. While doing so, we do not have access to the
>> PartitionKey of either relations. So, modify these functions to accept
>> only required members of PartitionKey so that the functions can be
>> reused for merging bounds.
>>
>> Amit's:
>> Some interface changes for partition_bound_{cmp/bsearch}
>>
>> Introduces a notion of PartitionBoundCmpArg, which replaces the set
>> of arguments void *probe and bool probe_is_bound of both
>> partition_bound_cmp and partition_bound_bsearch. It wasn't possible
>> before to specify the number of datums when a non-bound type of
>> probe is passed. Slightly tweaking the existing interface to allow
>> specifying the same seems awkward. So, instead encapsulate that
>> into PartitionBoundCmpArg. Also, modify partition_rbound_datum_cmp
>> to compare caller-specifed number of datums, instead of
>> key->partnatts datums.
>>
>
> I haven't looked at Amit's changes, but we need a more flexible way to
> pass information required for datum comparison than using
> PartitionKey, since that's not available in the optimizer and can not
> be associated with join, aggregate relations. If we pass that
> information through a structure, there are two ways
> 1. it will need to be part of PartitionScheme; I am not sure if we can
> have a substructure in PartitionKey. But if we can do it that way, we
> can pass that structure to the functions.
> 2. we will need to construct the structure filling it with comparison
> information and pass it to the comparison functions. I think what we
> achieve out of this isn't worth the code we will need to add.
>
> I would prefer first approach over the other.
ISTM that they're non-conflicting for the most part. My patch is about
modifying the way to bring "datums" into partition_bound_cmp(), whereas
Ashutosh's is about modifying the way we bring the partition key
information. Changes seem orthogonal to me, although, the patches
definitely won't like each other when applying to the tree.
Thanks,
Amit