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.
Regards,
Dean