tidwall / gjson

Get JSON values quickly - JSON parser for Go
MIT License
14.1k stars 846 forks source link

Add support to return multiple indexes when multiple matches are found #222

Closed sspaink closed 3 years ago

sspaink commented 3 years ago

This pull request adds a new field to the Result structure that will contain multiple indexes when multiple matches are found. Currently only a single index is provided that is set to 0 when you provide a query to gather multiple matches (a query to access multiple child paths "friends.#.first" or to find all matches #(...)#). I called the new field HashtagIndexes because I believe this should only apply when a query uses a hashtag (but definitely open to any other name suggestions).

I am working on a project that would benefit from knowing all the indexes, I added a unit test called TestHashtagIndexesMatchesRaw that is very similar to what I would like to do with this feature. Basically run two separate queries and walk over the results of one and use the indexes to find matches. Thank you for looking over this pull request! Hopefully this is the right approach to achieve this.

tidwall commented 3 years ago

On the surface I like it and I can see various uses for this feature.

I'm not sure about the name either, it's kinda long and I don't think the term Hashtag is used anywhere in the docs. But that's a minor thing. Perhaps just HIndexes or simply Indexes would suffice.

One issue might be how it works with modifiers (or mulitpaths).

For example, with this document:

{
    "objectArray":[
        {"first": "Dale", "age": 44},
        {"first": "Roger", "age": 68},
    ]
}

The path objectArray.#.first returns:

gjson.Result{
  Type: 5, 
  Raw: "[\"Dale\",\"Roger\"]", 
  Str: "",
  Num: 0, 
  Index: 0, 
  HashtagIndexes: []int{ 33, 66 },
}

While the path objectArray.@reverse.#.first returns:

gjson.Result{
  Type: 5, 
  Raw: "[\"Roger\",\"Dale\"]", 
  Str: "",
  Num: 0, 
  Index: 0, 
  HashtagIndexes: []int{ 11, 41 },
}

Because the modifier changed the order of the array, the 11 and 41 no longer applies to the original document.

We would either need to remap the indexes to the original document, or maybe we just return nil for the HashtagIndexes array.

sspaink commented 3 years ago

Thank you for the feedback! I like the idea of just using the name Indexes, seems nicer. Good catch when using modifiers as well, seems the easiest solution would be just to set Indexes to nil. I am not sure how to remap the indexes without passing a unmodified json string around.

I've updated the pull request to use Indexes and set it to nil when using modifiers, added the @reverse example to the unit test as well.

sspaink commented 3 years ago

@tidwall Is there anything else you would like me to do to help get this merged? Thanks!

tidwall commented 3 years ago

LGTM. I just pushed a new version that includes your PR. :)

sspaink commented 3 years ago

Awesome! Thanks :D