Re: jsonpath

On Wed, Apr 17, 2019 at 11:14 PM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Wed, Apr 17, 2019 at 8:43 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> writes:
> > > Attached is a patch to fix some minor issues:
> > > -misspelling of an error message
> >
> > Yeah, I'd noticed that one too :-(. I think the whole jsonpath patch
> > needs a sweep to bring its error messages into line with our style
> > guidelines, but no harm in starting with the obvious bugs.
>
> I'll go trough the jsonpath error messages and post a patch for fixing them.
>
> > > -Commit 550b9d26f80f failed to update the Makefile comment to reflect
> > > how the build changed, and also removed the clean target, which we now
> > > have use for since we later got rid of backtracking in the scanner.
> >
> > Right. I'm not really sure why we're bothering with anti-backtracking
> > here, or with using speed-rather-than-code-space lexer optimization
> > options. It's hard for me to credit that any practically-useful jsonpath
> > pattern would be long enough for lexer speed to matter, and even harder to
> > credit that the speed of the flex code itself would be an important factor
> > in the overall processing cost of a long jsonpath. Still, as long as we
> > have the code it needs to be right.
>
> Actually I found that non of in-core lexers are backtracking. So, I
> understood no backtracking as kind of standard and didn't want to
> break that :)
>
> Nevertheless, I could imagine use-case involving parsing a lot of
> jsonpath'es. For example we may construct jsonpath based on table
> data and check that for just few jsonb's. For sure, that wouldn't be
> a common use-case, but still.
>
> > > Also, while I have the thought in my head, for v13 we should consider
> > > replacing the keyword binary search with the perfect hash technique
> > > added in c64d0cd5ce2 -- it might give a small performance boost to the
> > > scanner.
> >
> > I doubt it's worth the trouble, per above.
> >
> > Patch LGTM, pushed.
>
> Thank you for pushing this!

I went trough the jsonpath errors and made some corrections. See the
attached patch.

One thing makes me uneasy. jsonpath_yyerror() substitutes its
"message" argument to the errdetail(). Out style guide requires
errdetails to be complete sentences, while bison passes
non-capitalized error messages. Should we bother capitalize first
character of "message"? I wonder how "portable" this solution would
be for various languages. cubescan.l reports error in the similar way
to jsonpath and doesn't bother about making errdetail a complete
sentence. Or should we move bison error to errmsg()?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company