On Tue, 13 Jul 2021 at 15:30, vignesh C <vignes...@gmail.com> wrote: > > On Tue, Jul 13, 2021 at 4:25 PM Dean Rasheed <dean.a.rash...@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. Regards, Dean
