[PATCH] Check operator when creating unique index on partition table

[Guancheng Luo]

Hi,

I found that things could go wrong in some cases, when the unique index and the 
partition key use different opclass.

For example:
```
CREATE TABLE ptop_test (a int, b int, c int) PARTITION BY LIST (a);
CREATE TABLE ptop_test_p1 PARTITION OF ptop_test FOR VALUES IN ('1');
CREATE TABLE ptop_test_m1 PARTITION OF ptop_test FOR VALUES IN ('-1');
CREATE UNIQUE INDEX ptop_test_unq_abs_a ON ptop_test (a abs_int_btree_ops);  -- 
this should fail
```
In this example, `abs_int_btree_ops` is a opclass whose equality operator is 
customized to consider ‘-1’ and ‘1’ as equal.
So the unique index should not be allowed to create, since ‘-1’ and ‘1’ will be 
put in different partition.

The attached patch should fix this problem.


0001-Check-operator-when-creating-UNIQUE-index-on-PARTITI.patch
Description: Binary data


Best Regards,
Guancheng Luo

Re: [PATCH] Check operator when creating unique index on partition table

[Guancheng Luo]

> On Mar 26, 2020, at 01:00, Tom Lane  wrote:
> 
> Guancheng Luo  writes:
>> I found that things could go wrong in some cases, when the unique index and 
>> the partition key use different opclass.
> 
> I agree that this is an oversight, but it seems like your solution is
> overcomplicated and probably still too forgiving.  Should we not just
> insist that the index opfamily match the partition key opfamily?
> It looks to me like that would reduce the code change to about like
> this:
> 
> -   if (key->partattrs[i] == indexInfo->ii_IndexAttrNumbers[j])
> +   if (key->partattrs[i] == indexInfo->ii_IndexAttrNumbers[j] &&
> +   key->partopfamily[i] == 
> get_opclass_family(classObjectId[j]))
> 
> which is a lot more straightforward and provides a lot more certainty
> that the index will act as the partition constraint demands.
> 
> This would reject, for example, a hash index associated with a btree-based
> partition constraint, but I'm not sure we're losing anything much thereby.
> (I do not think your patch is correct for the case where the opfamilies
> belong to different AMs, anyway.)

Since unique index cannot be using HASH, I think we only need to consider BTREE 
index here.

There is cases when a BTREE index associated with a HASH partition key, but I 
think we should allow them,
as long as their equality operators consider the same value as equal.
I’ve added some more test for this case.

> I'm not really on board with adding a whole new test script for this,
> either.

Indeed, I think `indexing.sql` might be more apporiate. I moved these tests in 
my new patch.



0001-Check-operator-when-creating-UNIQUE-index-on-PARTITI.patch
Description: Binary data


Best Regards,
Guancheng Luo