Re: Enhanced error message to include hint messages for redundant options error
vignesh C <vignesh21@gmail.com>
From: vignesh C <vignesh21@gmail.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Daniel Gustafsson <daniel@yesql.se>,
Alvaro Herrera <alvherre@alvh.no-ip.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Dilip Kumar <dilipbalaut@gmail.com>
Date: 2021-07-22T06:09:23Z
Lists: pgsql-hackers
On Thu, Jul 15, 2021 at 5:12 PM vignesh C <vignesh21@gmail.com> wrote: > > On Thu, Jul 15, 2021 at 1:40 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > > > On Tue, 13 Jul 2021 at 15:30, vignesh C <vignesh21@gmail.com> wrote: > > > > > > On Tue, Jul 13, 2021 at 4:25 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > > > > > > > As it stands, the improvements from (3) seem quite worthwhile. Also, > > > > the patch saves a couple of hundred lines of code, and for me an > > > > optimised executable is around 30 kB smaller, which is more than I > > > > expected. > > > > > > Agreed, it can be handled as part of the 2nd patch. The changes you > > > made apply neatly and the test passes. > > > > Pushed. > > > > I noticed that it's actually safe to call parser_errposition() with a > > null ParseState, so I simplified the ereport() code to just call it > > unconditionally. Also, I decided to not bother using the new function > > in cases with a null ParseState anyway since it doesn't provide any > > meaningful benefit in those cases, and those are the cases most likely > > to targeted next, so it didn't seem sensible to change that code, only > > for it to be changed again later. > > > > Probably the thing to think about next is the few remaining cases that > > throw this error directly and don't have any errdetail or errhint to > > help the user identify the offending option. My preference remains to > > leave the primary error text unchanged, but just add some suitable > > errdetail. Also, it's probably not worth adding a new function for > > those remaining errors, since there are only a handful of them. > > Thanks for pushing this patch. I have changed the commitfest status to > "waiting for author" till 0002 patch is posted. I'm marking this entry in commitfest as committed, I'm planning to work on the other comments later once I finish my current project works. Regards, Vignesh
Commits
-
Improve reporting of "conflicting or redundant options" errors.
- 2bfb50b3df11 15.0 landed
-
Refactor function parse_subscription_options.
- 8aafb0261675 15.0 cited