Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: allow relative paths #75

Closed
wants to merge 2 commits into from

Conversation

bettio
Copy link

@bettio bettio commented Mar 19, 2021

Draft not yet ready for merge or complete
See also GH #59

draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved

~~~~ abnf
json-path = root-selector *selector
json-path = start-selector *selector
start-selector = root-selector / current-item-selector
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether it makes sense to start a JSONPath with a current item selector as this is not "in the middle of" an array selection, for example, so what would the current item even mean?

Also, is there a consensus among existing implementations to support such a thing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the ABNF currently does not discuss nested queries, so there is no place where this could dock with.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether it makes sense to start a JSONPath with a current item selector as this is not "in the middle of" an array selection, for example, so what would the current item even mean?

TL;DR: it is the item set when calling the JSONPath evaluator, it is quite useful if we are going to embed/use JSONPath with advanced tooling.

Anyway please take a look to the following comments that explains that with further details:
#59 (comment)
#59 (comment)

Also supporting @ for all JSONPaths has the good side effect of simplifying grammar when defining filter expressions (no special cases).

Also, is there a consensus among existing implementations to support such a thing?

Jayway supports that, which is an influential implementation (I think we should consider some kind of weighted consensus) and I wish to support this also on my implementation ExJSONPath.

TBH I didn't check this for all existing implementations, so further implementations supporting this syntax might exist.

@glyn: Please, may you take a look to #59 that has all the information you are looking for.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I've been following the discussion in #59 and so far I haven't seen a compelling need for this feature. It seems to increase the surface area of the spec in a way which, as @timbray mentioned, isn't compatible with our charter.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it removes the need for any special case when using filters, since we may allow any jsonpath expression, simplifying grammar and existing implementations.
Also it has a XPath equivalent (which exists for good reasons), so we aren't introducing anything new here (I just added to this PR an example for it).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielaparker: does replacing

The current item selector @ when used outside a filter behaves as the document root selector unless a child node is given to the evaluator.

with

The current item selector @ can be used to represent the current node being evaluated. The initial current node defaults to the document root unless a child node is given to the evaluator as initial current node.

looks better to you?

Maybe another phrasing might be:

The current item selector @ can be used to represent the current node being evaluated. The initial current node defaults to the document root unless a different node is chosen.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I might also replace:

When a child node is used as current item, evaluator must select it as starting
node instead of the document root when an expression starting with @ is
evaluated.

with

Therefore when a child node is set as initial current item, evaluator must select it as starting node instead of the document root when an expression starting with @ is evaluated.

Copy link

@danielaparker danielaparker Mar 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bettio wrote:

@danielaparker: does replacing

The current item selector @ when used outside a filter behaves as the document root selector unless a child node is given to the evaluator.

with

The current item selector @ can be used to represent the current node being evaluated. The initial current node defaults to the document root unless a child node is given to the evaluator as initial current node.

looks better to you?

Maybe another phrasing might be:

The current item selector @ can be used to represent the current node being evaluated. The initial current node defaults to the document root unless a different node is chosen.

I think it's enough to say "The current item selector @ can be used to represent the current node being evaluated", full stop. That's about equivalent to the meaning of @ in JMESPath, and to the meaning of "." in XPATH 3.1, see here.

For a JSONPath evaluator, it only knows about one document, the one provided as an argument, along with the JSONPath expression. The current node is initialized to that document, and evaluation begins.

Sure, you can have higher level functions built on top of the JSONPath evaluator, that pass a different document to the evaluator depending on whether the path starts with $ or @. Implementers can support such functions if they wish. But the evaluator doesn't know about that, it only knows about one document.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielaparker:

For a JSONPath evaluator, it only knows about one document, the one provided as an argument, along with the JSONPath expression. The current node is initialized to that document, and evaluation begins.

I would rather stay explicit, so we make sure that it cannot be misunderstood. Therefore writing "The initial current node defaults to the document root" doesn't hurt and it helps an unambiguous understanding.

@danielaparker:

unless a child node is given to the evaluator as initial current node.

This helps remarking that @ at the beginning of an expression is not necessarily a $ synonymous according to the following proposal:

@gregsdennis:

Proposal: allow paths to start with @. It operates no differently than $, but can serve as an important indicator to tooling and other systems that consume JSON Path.

my goal is making sure that starting evaluation with current item pointing to a child node and optionally referencing it with @ at the beginning of an expression is compliant with the specification.
I feel like that just stopping at "The current item selector @ can be used to represent the current node being evaluated" might leave my goal in an unclear status.

By the way when filtering will be written we'll also remark that current item is updated.

@danielaparker:

Sure, you can have higher level functions built on top of the JSONPath evaluator, that pass a different document to the evaluator depending on whether the path starts with $ or @. Implementers can support such functions if they wish. But the evaluator doesn't know about that, it only knows about one document.

For the record, my evaluator has 3 parameters: json_path_eval(document_root, current_item, path). It is implemented as a recursive function and document_root is always passed unchanged, while current_item is updated. The caller may call it doing something like:
json_path_eval(root, root.foo, "@.bar") which is equivalent to json_path_eval(root, root, "$.foo.bar").
No higher level function magic here ;)
Anyway let's keep out of the discussion implementation detail, again I just need to make sure that @.bar (with the meaning that I just showed in the example) is not left out of the spec.

Lastly the PR just aims to allow @ at the beginning of the expression, I think more PRs are needed for a complete @ formalization.

@danielaparker thanks for your feedback, looking forward to further comments :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsdennis: wish to read any feedback about the wording from you too :) Your help has been valuable so far

Allow beginning paths using `@` instead of `$`.
@bettio bettio force-pushed the allow-relative-paths branch from c6423d8 to 79412af Compare March 19, 2021 17:17
@bettio
Copy link
Author

bettio commented Mar 21, 2021

As a summary I'm aiming to address with this PR:

  • allowing @ at the beginning of an expression
  • @ at the beginning of an expression defaults to document root
  • @ at the beginning of an expression can be != $ on certain use-cases

I'm not addressing:

  • @ scoping
  • complete description / formalization of @

@danielaparker
Copy link

@bettio wrote:

As a summary I'm aiming to address with this PR:

  • allowing @ at the beginning of an expression
  • @ at the beginning of an expression defaults to document root
  • @ at the beginning of an expression can be != $ on certain use-cases

Fair enough, it can be left open what the current node is initialized to, I have no objection.

Daniel

draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved
Comment on lines +524 to +531
The current item selector `@` when used outside a filter behaves as the document
root selector unless a child node is given to the evaluator.
When a child node is used as current item, evaluator must select it as starting
node instead of the document root when an expression starting with `@` is
evaluated.

When a child node is used as current item, the current item selector `@` selects
it as starting node instead of the document root.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of "node" here? I think this is one we've decided to avoid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know who "we" is; I think we are still discussing terminology.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm not so sure about "child node", the term "child" is usually used for direct descendants.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we're still discussing it, which means that it's something to avoid for now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I can replace node with item and child node with subdocument. I think that further improvements might be applied on future PRs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wait for PR#72 to settle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"document" certainly is not the terminology we want to use.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I can replace "The current item selector" ... "with @ is evaluated."

with

The current item selector `@` can be used to represent the current item being
evaluated. The initial current item defaults to the document root unless a
subdocument is given to the evaluator as initial current item.

I think I can safely remove "When a child node is used as current item, evaluator must select it as starting node instead of the document root when an expression starting with @ is evaluated." and "When a child node is used as current item, the current item selector @ selects it as starting node instead of the document root." since the proposed contains the same information but with a better phrasing.

@gregsdennis Let me know if it sounds good to you so I can push changes I described here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @cabo makes a good point about resolving it in another issue when we finish discussing "node."

No worries.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregsdennis: Ok, I'm keeping node for now, we can improve this with a future PR.

@cabo I removed document and replaced it again with "root node". I also replaced "child node" with "descendant node" which is more accurate.

So I will commit & push:

The current item selector `@` can be used to represent the current item being
evaluated. The initial current item defaults to the root node unless a
descendant node is given to the evaluator as initial current item.

Let me know if you have any further feedback, otherwise let me know you are ok for now so I can push it.

node instead of the document root when an expression starting with `@` is
evaluated.

When a child node is used as current item, the current item selector `@` selects
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure who's working with English as a first language, so please don't take offense, but this could use some grammatical massaging, specifically the use of the word "the" in a few places.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this could use some grammatical massaging

It is not my first language, so any advice or feedback is welcome

Anyway I think that I'll remove this and just keep:

The current item selector `@` can be used to represent the current item being
evaluated. The initial current item defaults to the document root unless a
subdocument is given to the evaluator as initial current item.

as described in the other conversation, if you agree too that "When a child node is used as current item, the"... is just redundant we can remove it and close this conversation since we don't need rephrasing it.

draft-ietf-jsonpath-base.md Outdated Show resolved Hide resolved
@goessner goessner mentioned this pull request Mar 22, 2021
@cabo cabo changed the base branch from master to main March 25, 2021 08:58
@glyn
Copy link
Collaborator

glyn commented Jul 6, 2021

@bettio What are your plans for this PR? Thanks.

@glyn
Copy link
Collaborator

glyn commented Jul 29, 2021

Closing as there was no response. Please re-open if you would like to continue working on this PR.

@glyn glyn closed this Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants