Re: Multi column range partition table

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, amul sul <sulamul(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Multi column range partition table
Date: 2017-07-02 16:59:53
Message-ID: CAEZATCVcBCBZsMcHj37TF+dcsjCtKZdZ_FAaJjaFMvfoXRqZMg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 30 June 2017 at 09:06, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> When testing the patch, I realized that the current code in
> check_new_partition_bound() that checks for range partition overlap had a
> latent bug that resulted in false positives for the new cases that the new
> less restrictive syntax allowed. I spent some time simplifying that code
> while also fixing the aforementioned bug. It's implemented in the
> attached patch 0001.
>

I haven't had time to look at 0002 yet, but looking at 0001, I'm not
convinced that this really represents much of a simplification, but I
do prefer the way it now consistently reports the first overlapping
partition in the error message.

I'm not entirely convinced by this change either:

- if (equal || off1 != off2)
+ if (off2 > off1 + 1 || ((off2 == off1 + 1) && !equal))

Passing probe_is_bound = true to partition_bound_bsearch() will I
think cause it to return equal = false when the upper bound of one
partition equals the lower bound of another, so relying on the "equal"
flag here seems a bit dubious. I think I can just about convince
myself that it works, but not for the reasons stated in the comments.

It also seems unnecessary for this code to be doing 2 binary searches.
I think a better simplification would be to just do one binary search
to find the gap that the lower bound fits in, and then test the upper
bound of the new partition against the lower bound of the next
partition (if there is one), as in the attached patch.

Regards,
Dean

Attachment Content-Type Size
simplify-new-range-partition-bounds-check.patch text/x-patch 5.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mithun Cy 2017-07-02 17:02:00 Re: Proposal : For Auto-Prewarm.
Previous Message Tom Lane 2017-07-02 16:22:27 Re: Using postgres planner as standalone component