On Mon, Apr 26, 2021 at 6:18 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Apr 26, 2021 at 5:49 PM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Mon, Apr 26, 2021 at 5:29 PM vignesh C <vignesh21@gmail.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.
Thanks for the comment, I have modified and shared the v2 patch
attached in the previous mail.
Regards,
Vignesh
