| 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: | Whole Thread | Raw Message | 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 |
| 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 |