`wrap: false` does not correctly return some cases

[cburgmer]

The following queries provide results that do not match those of other implementations of JSONPath
(compare https://github.com/cburgmer/json-path-comparison/tree/master/comparison):

• $[-1:]
  Input:

  ["first", "second", "third"]
  

  Expected output:

  ["third"]
  

  Actual output:

  "third"
  

• $[0:1]
  Input:

  ["first", "second"]
  

  Expected output:

  ["first"]
  

  Actual output:

  "first"
  

• $.key
  Input:

  {"key": ["first", "second"]}
  

  Expected output:

  ["first", "second"]
  

  Actual output:

  [["first", "second"]]
  

For reference, the output was generated by the program in https://github.com/cburgmer/json-path-comparison/tree/master/implementations/JavaScript_jsonpath-plus.

[brettz9]

Having such a library is fantastic! Along with the likes of https://github.com/andykais/json-querying-performance-testing (and https://naturalintelligence.github.io/comparejs/?q=jsonpath-comparison ), jsonpath is really getting some great tools for comparison.

I don't have time to explore the issues on our end at the moment, but I'd be happy to offer you commit access if you were interested.

You might want to note the presence of custom syntax, as our library at least offers some additional selectors, and it would be interesting to know if other implementations may have implemented them, using our syntax or in their own way.

It would also be interesting to review each library's tests for incorporation within a comprehensive testing library.

If I had to guess, the differences in this issue could be tied to the wrap: false setting. (though wrap: true is the only safe approach if needing to distinguish between lack of a result and finding an empty array--and when querying JavaScript objects, also distinguishing between an explicit undefined and the lack of a result).

[cburgmer]

Your guess seems about right to me - I changed the local adapter to use wrap: true with it's own handling of scalar values, and all seems fine. Changed the title of this bug report accordingly. Also updated the comparison to show that those queries are properly supported.

Thanks a lot for your feedback, and the links! Helpful to see similar goals.

My personal interested started very much with trying to understand the power of JSONPath (comparing it to other query languages, like jq), and the table helps with shining a light on anything that's not called out in the initial proposal.

As for comprehensive testing library: I added a roadmap to call this out, I think this could be in scope of this undertaking!

[brettz9]

Great stuff...

As far as comparing JSONPath to other query languages, that is a very helpful approach, and in fact part of the reason behind our adding those additional features not available in the original spec. Even plain XPath can do quite a bit, and if you add XQuery on top of that (or the more familiar XSLT which basically adds templates to XQuery and uses XML), I think a light is shone on what more could be done, particularly with the schema-awareness of XQuery/XSLT perhaps pointing to the utility of JSON-schema aware queries on our part).

[CacheControl]

hi @s3u and @brettz9 I'm the maintainer of json-rules-engine, and we recently added your project as a dependency for supporting a json-path feature - we liked that your library was isomorphic, performant, and had a small bundle size - so thanks!

Since our library evaluates conditions where the input data type is important, this bug is causing problems in that for the same json path, sometimes we get an array returned, and other times we do not. I also don't think using wrap: true is a great option for us either, as it'll create the opposite confusion; users write a json-path that ought to return a single value, and instead they'll get an array.

I'm curious what your current take is on this bug and what our options might be for fixing it. thanks!

[brettz9]

@CacheControl : Your project looks great; kind of a Schematron-for-JSON perhaps?

I am afraid I just am too tied up in other projects these days to do much here. There's not too much code here to dive into though, so I'm happy to review a PR with tests if someone can provide.

[CacheControl]

@brettz9 I think I can carve out some time to take a crack at this. Can you help me confirm the requirements? I'm relatively familiar with json-path, but no expert.

I've been playing around with the https://jsonpath.herokuapp.com/ with the various operators and reviewing some articles, and the following rules seem to hold:

The result should always be an array if the path contains:

• .. - performs a deep scan
• * - selects all elements in object/array
• [n1:n2], [:n], [-n:] - these specify a range within an array
• [1,2,N] - multiple array elements
• [?(expression)] - filters array

Conversely, the result may be a scalar/object/array if none of the above operators are found in the path; this is the most simple path traversal e.g. store.bicycle.color, store.books[0]

I would apply the above rules when wrap: false is passed, and leave the default behavior alone.

What do you think?

[cburgmer]

In the comparison https://cburgmer.github.io/json-path-comparison/ you'll find a few implementations which return scalar results. Hopefully they include answers for your questions. The Haskell one seems pretty complete, for example.

[brettz9]

@cburgmer has given a helpful cross-platform comparisons. Just look at our README though if you want an understanding of our library's operators. And really, IIRC, there's not much difference behind the wrapping code; it's just post-result-retrieval where I think the wrapping problem came into play (as far as I remember anyways).

[brettz9]

@cburgmer : Do you think you could take a look at the expectation changes in #111 ? I'm planning to offer some suggestions on implementation details now, but I'm mostly asking if you could take a look just on the test additions/changes and whether they appear to match other jsonpath implementations' behavior. I've also asked @CacheControl to see if he can look at your library in case it may give ideas on how wrap: false might be expected to behave in comparison (despite the testing implementation looking at wrap: true).

[cburgmer]

I've cloned the setup with wrap: false and put it on a branch. Here's a human readable form of the errors, although it includes other parts of where this library diverges from the consensus as well: https://github.com/cburgmer/json-path-comparison/blob/JavaScript_jsonpath-plus_nowrap/bug_reports/JavaScript_jsonpath-plus_nowrap.md

More helpful maybe is a side by side diff of the regression report (https://github.com/cburgmer/json-path-comparison/blob/JavaScript_jsonpath-plus_nowrap/regression_suite/JavaScript_jsonpath-plus_nowrap.yaml):

$ diff -y regression_suite/JavaScript_jsonpath-plus*
# This file was generated by src/compile_regression_suite.sh 	# This file was generated by src/compile_regression_suite.sh
# You probably don't want to change this manually but rather 	# You probably don't want to change this manually but rather

implementation: JavaScript_jsonpath-plus		      |	implementation: JavaScript_jsonpath-plus_nowrap
queries:							queries:
...
  - id: array_index_slice_last_no_end				  - id: array_index_slice_last_no_end
    selector: $[-1:]						    selector: $[-1:]
    document: ["first", "second", "third"]			    document: ["first", "second", "third"]
    result: ["third"]					      |	    result: "third"
    status: pass					      |	    status: fail
							      >	    consensus: ["third"]
...