Re: Report error position in partition bound check

From: Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alexandra Wang <lewang(at)pivotal(dot)io>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Ashwin Agrawal <aagrawal(at)pivotal(dot)io>
Subject: Re: Report error position in partition bound check
Date: 2020-04-10 15:37:25
Message-ID: CAG-ACPWrdYjPwYDmeHrZ-QRK9fhLWB43GV7T7dEn_MsVimX=fg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 10 Apr 2020 at 14:31, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:

> On Thu, Apr 9, 2020 at 11:04 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > While I'm quite on board with providing useful error cursors,
> > the example cases in this patch don't seem all that useful:
> >
> > -- trying to create range partition with empty range
> > CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (1)
> TO (0);
> > ERROR: empty range bound specified for partition "fail_part"
> > +LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (1)
> T...
> > + ^
> > DETAIL: Specified lower bound (1) is greater than or equal to upper
> bound (0).
> >
> > As best I can tell from these examples, the cursor will always
> > point at the FROM keyword, making it pretty unhelpful. It seems
> > like in addition to getting the query string passed down, you
> > need to do some work on the code that's actually reporting the
> > error position. I'd expect at a minimum that the pointer allows
> > identifying which column of a multi-column partition key is
> > giving trouble. The phrasing of this particular message, for
> > example, suggests that it ought to point at the "1" expression.
>
> I agree with that. Tried that in the attached 0002, although trying
> to get the cursor to point to exactly the offending column seems a bit
> tough for partition overlap errors. The patch does allow to single
> out which one of the lower and upper bounds is causing the overlap
> with an existing partition, which is better than now and seems helpful
> enough.
>
> Also, updated Alexandra's patch to incorporate Ashutosh's comment such
> that we get the same output with ATTACH PARTITION commands too.
>

I looked at this briefly. It looks good, but I will review more in the next
CF. Do we have entry there yet? To nit-pick: for a multi-key value the ^
points to the first column and the reader may think that that's the
problematci column. Should it instead point to ( ?

--
Best Wishes,
Ashutosh

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-04-10 15:44:35 Re: [HACKERS] advanced partition matching algorithm for partition-wise join
Previous Message Tom Lane 2020-04-10 15:37:02 Re: pg_validatebackup -> pg_verifybackup?