On Mon, Apr 26, 2021 at 5:49 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Mon, Apr 26, 2021 at 5:29 PM vignesh C <vignes...@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. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com