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

Support for Functions in Predicates and Json Paths #103

Merged
merged 9 commits into from
Oct 15, 2015
Merged

Support for Functions in Predicates and Json Paths #103

merged 9 commits into from
Oct 15, 2015

Conversation

mgreenwood1001
Copy link
Contributor

Implementation of math and array length support in predicates and paths. Initial implementation - uses Function interface as a pattern for writing new functions in the future.

Ready for review - code coverage should be fairly high given all use-cases are supported via functional tests.

@jochenberger
Copy link
Contributor

This is a nice idea, but that implementation will not be compatible with custom JsonProviders. Checking with instanceof will not always work, for example you'll need to use JsonProvider.isArray(Object).

@kallestenflo
Copy link
Contributor

I come back to this functionality all the time (even after merging it). There are a couple of things that needs to be answered.

1. The choice of function prefix character '%'

Should there be a function prefix at all?

$.numbers.%max()
or
$.numbers.max()

2. Function target

Today the function targets only the value that the path points to. How can you apply a function to the result of an evaluation like $..numbers.%max(). Should there be some other notation?

@mgreenwood1001
Copy link
Contributor Author

The prefix character - certainly not wedded to the chosen literal '%', it could/perhaps shouldn't exist at all. Not having one is a more involved parser change. Ideally, functions are denoted by the parenthesis that exist in the signature of a call and registration by function name, but I had a concern that this may start to pattern match on other uses for that notation and thus the token.

Regarding the function target, if you wanted to invert the call such that the function took several paths rather than being the tail execution of a path as it exists today - my belief was that was no longer a json-path tool, but rather just an odd macro language and was perhaps beyond the bounds of this tool? It can be done and I'd be willing to make that change if you think it would be worthwhile.

What I tried to provide was an implements/pattern that solved for function expressions in criteria, and generally function expressions with a pattern for creating user-defined functions without refactoring the path parser code.
Thanks,
--Matt

@kallestenflo
Copy link
Contributor

I think it would be a lot easier to remove the prefix character today if we think this improves the syntax. I have simplified the parsing and intend to continue this rewrite (post 2.1 release).

I agree there is a risk introducing operations that are not bound to the tail of the path. My current goal with the project is to simplify and improve the implementation so I guess you are right, an odd macro language will not help. What nags me is that I think the feature would be more useful if not bound to the tail of the path. Operations like 'give me the item with the lowest price' just seems more useful. How can this be done in a good way without violation the original spec to much.

I'm a bit uncomfortable with making all these decisions so I appreciate suggestions and input.

Cheers
Kalle

@kallestenflo
Copy link
Contributor

% prefix is now removed from functions.

@atomcat1978
Copy link

In the previous release (2.0.0) the path lookup $.store.book[(@.length-1)].title was working. Can You confirm, that it is not valid any longer and instead the following syntax should be used: $.store.book[-1:].title
If yes, than the intro doc should be updated.

@mgreenwood1001
Copy link
Contributor Author

@kallestenflo I have a branch in progress that will solve for passing JsonPath's as arguments to functions - effectively nested/recursive functions. I used the brace notation as that wasn't already in use, but open to syntax suggestions.

Example:
$.sum({$.numbers.min()}, {$.numbers.max()})
You could also do something such as:
$.numbers.add({$.numbers.min()})
where add for each element in the array took another JsonPath parameter that we'd add to resulting in an array result with the min value of numbers added to each element in the number's array. Alternatively:

    $.numbers.add(1234)

Obviously there's better examples than the above - but these changes allow the PathCompiler to parse the function parameters for nested JsonPath's.

@kallestenflo
Copy link
Contributor

Nice! Can you drop the braces?

@mgreenwood1001
Copy link
Contributor Author

I'll see if I can remove the braces and still be able to get reasonable error reporting from the tokenizer.
The reason I added them is I'm concerned without them the tokenizer of the function parameters will run away into subsequent tokens and will report conditions that don't help identify & how to address syntax errors created by developers. Additionally, the function parameter tokenizer is expected to understand the distinction between jsonpath sub-expression(s) & function parameters that are literals (numeric, text values) within the same function call.

Leaving the braces in denotes the end of a nested sub-jsonpath expression and gives the tokenizer a start/end character stream to begin reporting syntax issues. i.e. 'unable to find end of 2nd parameter, to function avg, starting at char: N'.

The changes I'm making the function path parameters are a recursive call to the tokenizer and as a result you could nest these function calls to the depth supported via the stack, which I thought makes error reporting even more important.

@kallestenflo
Copy link
Contributor

I hear you, but I still think the syntax would be cleaner without the braces. I also think it should be possible to pass JSON into a funtion:

$.colors.add({"name" : "green"}) or $.colors.add(?) where ? a is a argument passed to evaluation.

I think we need an AST to formalize all tokens in the path, including functions, filters and path predicates. Not saying this has to be done now but things need to be formalized.

@mgreenwood1001
Copy link
Contributor Author

I didn't implement add, but the {} notation no longer exists for sub-path expressions as parameters and you can pass JSON to a function and/or paths in #167 First pass, needs cleanup and error handling.

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.

None yet

4 participants