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

From: Guancheng Luo <prajnamort(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Check operator when creating unique index on partition table
Date: 2020-03-28 03:28:41
Message-ID: B63F443B-C5CC-4590-8011-EF3E5BB17823@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Mar 26, 2020, at 01:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Guancheng Luo <prajnamort(at)gmail(dot)com> 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.

Attachment Content-Type Size
0001-Check-operator-when-creating-UNIQUE-index-on-PARTITI.patch application/octet-stream 13.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2020-03-28 04:12:11 Re: Berserk Autovacuum (let's save next Mandrill)
Previous Message Masahiko Sawada 2020-03-28 03:28:27 Re: Online checksums verification in the backend