Re: should check collations when creating partitioned index

On 13.11.23 21:04, Laurenz Albe wrote:
> On Mon, 2023-11-13 at 10:24 +0100, Peter Eisentraut wrote:
>> * If this table is partitioned and we're creating a unique index, primary
>> * key, or exclusion constraint, make sure that the partition key is a
>> * subset of the index's columns. Otherwise it would be possible to
>> * violate uniqueness by putting values that ought to be unique in
>> * different partitions.
>>
>> But this currently doesn't check that the collations between the
>> partition key and the index definition match. So you can construct a
>> unique index that fails to enforce uniqueness.
>>
>> Here is a partitioned case that doesn't work correctly:
>>
>> create collation case_insensitive (provider=icu,
>> locale='und-u-ks-level2', deterministic=false);
>> create table t1 (a int, b text) partition by hash (b);
>> create table t1a partition of t1 for values with (modulus 2, remainder 0);
>> create table t1b partition of t1 for values with (modulus 2, remainder 1);
>> create unique index i1 on t1 (b collate case_insensitive);
>> insert into t1 values (1, 'a'), (2, 'A'); -- this succeeds
>>
>> The attached patch adds the required collation check. In the example,
>> it would not allow the index i1 to be created.
>
> The patch looks good, but I think the error message needs love:
>
>> + ereport(ERROR,
>> + errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>> + errmsg("collation of column \"%s\" does not match between partition key and index definition",
>> + NameStr(att->attname)));
>
> "does not match between" sounds weird. How about
>
> collation of index column \"%s\" must match collation of the partitioning key column
>
> This will be backpatched, right? What if somebody already created an index like that?
> Does this warrant an entry in the "however" for the release notes, or is the case
> exotic enough that we can assume that nobody is affected?

I think it's exotic enough that I wouldn't bother backpatching it. But
I welcome input on this.