Re: Enhanced error message to include hint messages for redundant options error

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Enhanced error message to include hint messages for redundant options error
Date: 2021-04-26 12:48:29
Message-ID: CAFiTN-vxWXenXq-ypppVOeE59af1t+807bQ36Mso-ETX1JjpNw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 26, 2021 at 5:49 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Mon, Apr 26, 2021 at 5:29 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > While reviewing one of the logical replication patches, I found that
> > we do not include hint messages to display the actual option which has
> > been specified more than once in case of redundant option error. I
> > felt including this will help in easily identifying the error, users
> > will not have to search through the statement to identify where the
> > actual error is present. Attached a patch for the same.
> > Thoughts?
>

+1 for improving the error

> Comments on the patch:
>
> 1) generally errhint and errdetail messages should end with a period
> ".", please see their usage in other places.
> + errhint("Option \"streaming\" specified more
> than once")));
>
> 2) I think it should be errdetail instead of errhint, because you are
> giving more information about the error, but not hinting user how to
> overcome it. If you had to say something like "Remove duplicate
> \"password\" option." or "The \"password\" option is specified more
> than once. Remove all the duplicates.", then it makes sense to use
> errhint.

I agree this should be errdetail.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-04-26 13:29:16 Re: [BUG] "FailedAssertion" reported when streaming in logical replication
Previous Message Alvaro Herrera 2021-04-26 12:40:54 Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY