Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Teach jsonpath string() to unwrap in lax mode

  1. Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-08T22:49:38Z

    Hackers,

Most of the jsonpath methods auto-unwrap in lax mode:

david=# select jsonb_path_query('[-2,5]', '$.abs()');
 jsonb_path_query
------------------
 2
 5
(2 rows)

The obvious exceptions are size() and type(), which apply directly to arrays, so no need to unwrap:

david=# select jsonb_path_query('[-2,5]', '$.size()');
 jsonb_path_query
------------------
 2
(1 row)

david=# select jsonb_path_query('[-2,5]', '$.type()');
 jsonb_path_query
------------------
 "array"

But what about string()? Is there some reason it doesn’t unwrap?

david=# select jsonb_path_query('[-2,5]', '$.string()');
ERROR:  jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value

What I expect:

david=# select jsonb_path_query('[-2,5]', '$.string()');
 jsonb_path_query
—————————
 "2"
 "5"
(2 rows)

However, I do see a test[1] for this behavior, so maybe there’s a reason for it?

Best,

David

[1]: https://github.com/postgres/postgres/blob/REL_17_BETA1/src/test/regress/expected/jsonb_jsonpath.out#L2527-L2530

  2. Re: Shouldn't jsonpath .string() Unwrap?

    David G. Johnston <david.g.johnston@gmail.com> — 2024-06-12T20:02:44Z

    On Sat, Jun 8, 2024 at 3:50 PM David E. Wheeler <david@justatheory.com>
wrote:

> Hackers,
>
> Most of the jsonpath methods auto-unwrap in lax mode:
>
> david=# select jsonb_path_query('[-2,5]', '$.abs()');
>  jsonb_path_query
> ------------------
>  2
>  5
> (2 rows)
>
> The obvious exceptions are size() and type(), which apply directly to
> arrays, so no need to unwrap:
>
> david=# select jsonb_path_query('[-2,5]', '$.size()');
>  jsonb_path_query
> ------------------
>  2
> (1 row)
>
> david=# select jsonb_path_query('[-2,5]', '$.type()');
>  jsonb_path_query
> ------------------
>  "array"
>
> But what about string()? Is there some reason it doesn’t unwrap?
>
> david=# select jsonb_path_query('[-2,5]', '$.string()');
> ERROR:  jsonpath item method .string() can only be applied to a bool,
> string, numeric, or datetime value
>
> What I expect:
>
> david=# select jsonb_path_query('[-2,5]', '$.string()');
>  jsonb_path_query
> —————————
>  "2"
>  "5"
> (2 rows)
>
> However, I do see a test[1] for this behavior, so maybe there’s a reason
> for it?
>
>
Adding Andrew.

I'm willing to call this an open item against this feature as I don't see
any documentation explaining that string() behaves differently than the
others.

David J.

  3. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-12T20:10:01Z

    On Jun 12, 2024, at 4:02 PM, David G. Johnston <david.g.johnston@gmail.com> wrote:

> Adding Andrew.

Thank you.

> I'm willing to call this an open item against this feature as I don't see any documentation explaining that string() behaves differently than the others.

Maybe there’s some wording in the standard on this topic?

I’m happy to provide a patch to auto-unwrap .string() in lax mode. Seems pretty straightforward.

D

  4. Re: Shouldn't jsonpath .string() Unwrap?

    Andrew Dunstan <andrew@dunslane.net> — 2024-06-13T19:53:17Z

    On 2024-06-12 We 16:02, David G. Johnston wrote:
> On Sat, Jun 8, 2024 at 3:50 PM David E. Wheeler 
> <david@justatheory.com> wrote:
>
>     Hackers,
>
>     Most of the jsonpath methods auto-unwrap in lax mode:
>
>     david=# select jsonb_path_query('[-2,5]', '$.abs()');
>      jsonb_path_query
>     ------------------
>      2
>      5
>     (2 rows)
>
>     The obvious exceptions are size() and type(), which apply directly
>     to arrays, so no need to unwrap:
>
>     david=# select jsonb_path_query('[-2,5]', '$.size()');
>      jsonb_path_query
>     ------------------
>      2
>     (1 row)
>
>     david=# select jsonb_path_query('[-2,5]', '$.type()');
>      jsonb_path_query
>     ------------------
>      "array"
>
>     But what about string()? Is there some reason it doesn’t unwrap?
>
>     david=# select jsonb_path_query('[-2,5]', '$.string()');
>     ERROR:  jsonpath item method .string() can only be applied to a
>     bool, string, numeric, or datetime value
>
>     What I expect:
>
>     david=# select jsonb_path_query('[-2,5]', '$.string()');
>      jsonb_path_query
>     —————————
>      "2"
>      "5"
>     (2 rows)
>
>     However, I do see a test[1] for this behavior, so maybe there’s a
>     reason for it?
>
>
> Adding Andrew.
>
> I'm willing to call this an open item against this feature as I don't 
> see any documentation explaining that string() behaves differently 
> than the others.
>
>

Hmm. You might be right. Many of these items have this code, but the 
string() branch does not:

    if (unwrap && JsonbType(jb) == jbvArray)
         return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
                                             false);


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  5. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-13T22:45:21Z

    On Jun 13, 2024, at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

> Hmm. You might be right. Many of these items have this code, but the string() branch does not:
> if (unwrap && JsonbType(jb) == jbvArray)
>    return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
>                                        false);

Exactly, would be pretty easy to add. I can work up a patch this weekend.

D

  6. Re: Shouldn't jsonpath .string() Unwrap?

    Chapman Flack <jcflack@acm.org> — 2024-06-14T01:55:04Z

    On 06/13/24 18:45, David E. Wheeler wrote:
> On Jun 13, 2024, at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
> 
>> Hmm. You might be right. Many of these items have this code, but the string() branch does not:
>> if (unwrap && JsonbType(jb) == jbvArray)
>>    return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
>>                                        false);
> 
> Exactly, would be pretty easy to add. I can work up a patch this weekend.

My opinion is yes, that should be done. 9.46, umm, General
Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method> is not
type or size, then let BASE be Unwrap(BASE)." No special exemption
there for string(), nor further below at C) XV) for the operation
of string().

Regards,
-Chap

  7. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-14T14:39:36Z

    On Jun 13, 2024, at 21:55, Chapman Flack <jcflack@acm.org> wrote:

> My opinion is yes, that should be done. 9.46, umm, General
> Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method> is not
> type or size, then let BASE be Unwrap(BASE)." No special exemption
> there for string(), nor further below at C) XV) for the operation
> of string().

Thank you! Cited that bit in the commit message in the attached patch (also available as a GitHub PR[1]).

D

[1]: https://github.com/theory/postgres/pull/5

  8. Re: Shouldn't jsonpath .string() Unwrap?

    Chapman Flack <jcflack@acm.org> — 2024-06-14T15:25:53Z

    On 06/14/24 10:39, David E. Wheeler wrote:
> Cited that bit in the commit message in the attached patch (also available as a GitHub PR[1]).
> 
> [1]: https://github.com/theory/postgres/pull/5

I would s/extepsions/exceptions/ in the added documentation. :)

Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
I would find this one a little hard to follow, being based at a point
28 unrelated commits ahead of the ref it's opened against. I suspect
'master' on theory/postgres could be fast-forwarded to f1affb6 and then
the PR would look much more approachable.

Regards,
-Chap

  9. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-14T16:03:54Z

    
> On Jun 14, 2024, at 11:25, Chapman Flack <jcflack@acm.org> wrote:
> 
> I would s/extepsions/exceptions/ in the added documentation. :)

Bah, fixed and attached, thanks.

> Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
> I would find this one a little hard to follow, being based at a point
> 28 unrelated commits ahead of the ref it's opened against. I suspect
> 'master' on theory/postgres could be fast-forwarded to f1affb6 and then
> the PR would look much more approachable.

Yeah, I pushed the PR and branch before I synced master, and GitHub was taking a while to notice and update the PR. I fixed it with `git commit --all --amend --date now --reedit-message HEAD` and force-pushed (then fixed the typo and fixed again).

D

  10. Re: Shouldn't jsonpath .string() Unwrap?

    Jeevan Chalke <jeevan.chalke@enterprisedb.com> — 2024-06-15T14:27:48Z

    Hi,

Sorry, I have missed this in the original patch. I am surprised how that
happened. But thanks for catching the same and fixing it.

The changes are straightforward and look good to me. However, I have kept
the existing test of an empty array as is, assuming that it is one of the
valid tests. It now returns zero rows instead of an error. Your added test
case covers this issue.

Thanks



On Fri, Jun 14, 2024 at 9:34 PM David E. Wheeler <david@justatheory.com>
wrote:

>
>
> > On Jun 14, 2024, at 11:25, Chapman Flack <jcflack@acm.org> wrote:
> >
> > I would s/extepsions/exceptions/ in the added documentation. :)
>
> Bah, fixed and attached, thanks.
>
> > Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
> > I would find this one a little hard to follow, being based at a point
> > 28 unrelated commits ahead of the ref it's opened against. I suspect
> > 'master' on theory/postgres could be fast-forwarded to f1affb6 and then
> > the PR would look much more approachable.
>
> Yeah, I pushed the PR and branch before I synced master, and GitHub was
> taking a while to notice and update the PR. I fixed it with `git commit
> --all --amend --date now --reedit-message HEAD` and force-pushed (then
> fixed the typo and fixed again).
>
> D
>
>
>
>

-- 



*Jeevan Chalke*

*Principal, ManagerProduct Development*

enterprisedb.com <https://www.enterprisedb.com>

  11. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-15T14:39:06Z

    On Jun 15, 2024, at 10:27, Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:

> Sorry, I have missed this in the original patch. I am surprised how that happened. But thanks for catching the same and fixing it.

No worries. :-)

> The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.

Makes sense, thank you.

D

  12. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-15T14:51:57Z

    On Jun 15, 2024, at 10:39, David E. Wheeler <david@justatheory.com> wrote:

>> The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.
> 
> Makes sense, thank you.

Added https://commitfest.postgresql.org/48/5039/.

D

  13. Re: Shouldn't jsonpath .string() Unwrap?

    Andrew Dunstan <andrew@dunslane.net> — 2024-06-15T16:48:53Z

    On 2024-06-15 Sa 10:51, David E. Wheeler wrote:
> On Jun 15, 2024, at 10:39, David E. Wheeler <david@justatheory.com> wrote:
>
>>> The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.
>> Makes sense, thank you.
> Added https://commitfest.postgresql.org/48/5039/.
>

Not really needed, I will commit shortly.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  14. Re: Shouldn't jsonpath .string() Unwrap?

    David E. Wheeler <david@justatheory.com> — 2024-06-15T20:28:57Z

    On Jun 15, 2024, at 12:48, Andrew Dunstan <andrew@dunslane.net> wrote:

> Not really needed, I will commit shortly.

Ah, okay, I wasn’t sure so just defaulted to making sure it was tracked. :-)

Thanks Andrew,

D