Re: [HACKERS] PartitionSchemaData & partcollation (Re: pgsql: Associate partitioning information with each RelOptInfo.)

On 2017/09/28 16:13, Ashutosh Bapat wrote:
> On Thu, Sep 28, 2017 at 11:47 AM, Amit Langote wrote:
>> On 2017/09/21 12:42, Robert Haas wrote:
>>> Associate partitioning information with each RelOptInfo.
>>>
>>> This is not used for anything yet, but it is necessary infrastructure
>>> for partition-wise join and for partition pruning without constraint
>>> exclusion.
>>>
>>> Ashutosh Bapat, reviewed by Amit Langote and with quite a few changes,
>>> mostly cosmetic, by me. Additional review and testing of this patch
>>> series by Antonin Houska, Amit Khandekar, Rafia Sabih, Rajkumar
>>> Raghuwanshi, Thomas Munro, and Dilip Kumar.
>>
>> I noticed that this commit does not add partcollation field to
>> PartitionSchemeData, while it adds parttypcoll. I think it'd be necessary
>> to have partcollation too, because partitioning would have used the same,
>> not parttypcoll. For, example see the following code in
>> partition_rbound_datum_cmp:
>>
>> cmpval = DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[i],
>> key->partcollation[i],
>> rb_datums[i],
>> tuple_datums[i]));
>>
>> So, it would be wrong to use parttypcoll, if we are to use the collation
>> to match a clause with the partition bounds when doing partition-pruning.
>> Concretely, a clause's inputcollid should match partcollation for the
>> corresponding column, not the column's parttypcoll.
>>
>> Attached is a patch that adds the same. I first thought of including it
>> in the partition-pruning patch set [1], but thought we could independently
>> fix this.
>>
>
> I think PartitionSchemeData structure will grow as we need more
> information about partition key for various things. E.g. partsupfunc
> is not part of this structure right now, but it would be required to
> compare partition bound datums. Similarly partcollation. Please add
> this to partition pruning patchset. May be parttypcoll won't be used
> at all and we may want to remove it altogether.

Okay, I will post it with the partition-pruning patch set.

Thanks,
Amit