Re: Define jsonpath functions as stable

From: "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: Chapman Flack <chap(at)anastigmatix(dot)net>, Erik Rijkers <er(at)xs4all(dot)nl>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Define jsonpath functions as stable
Date: 2019-09-19 18:37:29
Message-ID: 1272d4ea-28c3-f0d5-4672-3f7a073baccb@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-09-19 19:11:13 Re: pgbench - allow to create partitioned tables
Previous Message Pavel Stehule 2019-09-19 18:10:23 Re: dropdb --force