Re: Define jsonpath functions as stable
Jonathan S. Katz <jkatz@postgresql.org>
From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>,
Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Chapman Flack <chap@anastigmatix.net>, Erik Rijkers <er@xs4all.nl>,
pgsql-hackers@lists.postgresql.org
Date: 2019-09-19T18:37:29Z
Lists: pgsql-hackers
On 9/19/19 12:25 PM, Tom Lane wrote: > I wrote: >> I found a spot that seemed like a reasonable place, and added some >> coverage of the point. Updated patch attached. > > Doc patch pushed. Thanks! I did not get to review them last night but upon review not too long ago, they looked great. >> It seems to me that there are some discrepancies between what the spec >> says and what jsonpath_scan.l actually does, so maybe we should take a >> hard look at that code too. The biggest issue is that jsonpath_scan.l >> seems to allow single- and double-quoted strings interchangeably, which is >> OK per ECMAScript, but then the SQL/JSON spec seems to be saying that only >> double-quoted strings are allowed. I'd rather be conservative about this >> than get out in front of the spec and use syntax space that they might do >> something else with someday. I agree with erring on the side of the spec vs. what ECMAScript does. In JSON, strings, identifiers, etc. are double-quoted. Anything that is single quoted with throw an error in a compliant JSON parser. Looking at the user documentation for how some other databases with SQL/JSON support, this seems to back up your analysis. > > The attached proposed patch makes these changes: > > 1. Remove support for single-quoted literals in jsonpath. > > 2. Treat an unrecognized escape (e.g., "\z") as meaning the escaped > character, rather than throwing an error. > > 3. A few cosmetic adjustments to make the jsonpath_scan code shorter and > clearer (IMHO). If this refers to s/any/other/, yes I would agree it's clearer. > As for #1, although the SQL/JSON tech report does reference ECMAScript > which allows both single- and double-quoted strings, it seems to me > that their intent is to allow only the double-quoted variant. They > specifically reference JSON string literals at one point, and of course > JSON only allows double-quoted. Also, all of their discussion and > examples use double-quoted. Plus you'd have to be pretty nuts to want > to use single-quoted when writing a jsonpath string literal inside a SQL > literal (and the tech report seems to contemplate that jsonpaths MUST be > string literals, though of course our implementation does not require > that). I agree with the above (though wrt single-quoting and literals, I have seen stranger things). > As for #2, the existing code throws an error, but this is contrary > to clear statements in every single one of the relevant standards. Makes sense. I looked at the patch, but did not test it. From what I can see, it looks good, but perhaps we add a test in it to show that single-quoted literals are unsupported? Jonathan
Commits
-
Fix some minor spec-compliance issues in jsonpath lexer.
- e56cad84d542 13.0 landed
- 5f3bec0769c1 12.0 landed
-
Doc: improve documentation around jsonpath regular expressions.
- b9cf94c8c249 12.0 landed
- 0a97edb12ec4 13.0 landed
-
Fix bogus handling of XQuery regex option flags.
- d5b90cd64855 13.0 landed
- 148881454208 12.0 landed