From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | 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 13:31:55 |
Message-ID: | CALDaNm2vv7g975XqOmikbZm51W=6GBQi8ekjvW7eekniYmXOcQ@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. A more detailed error will be useful in a rare scenario like users
> have specified duplicate options along with a lot of other options,
> they will know for which option the error is thrown by the server.
>
> Instead of adding errhint or errdetail, how about just changing the
> error message itself to something like "option \"%s\" specified more
> than once" or "parameter \"%s\" specified more than once" like we have
> in other places in the code?
>
Both seemed fine but I preferred using errdetail as I felt it is
slightly better for the details to appear in a new line.
> 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")));
>
Modified it.
> 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.
Modified it.
Attached patch for the same.
Regards,
Vignesh
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Enhance-error-message.patch | text/x-patch | 23.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2021-04-26 13:33:06 | Re: Enhanced error message to include hint messages for redundant options error |
Previous Message | Tomas Vondra | 2021-04-26 13:31:02 | Re: Performance degradation of REFRESH MATERIALIZED VIEW |