Skip to content

Review jsonpath_rs #10

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

Closed
wants to merge 2 commits into from
Closed

Review jsonpath_rs #10

wants to merge 2 commits into from

Conversation

oshadmi
Copy link

@oshadmi oshadmi commented Aug 18, 2022

Few comments up til now:

  • Do we need to support/test filter with full_scan or all? (currently it seems to be allowed by the grammar)
  • We should probably add more comments in code
  • Did not get the flat_arrays_on_filter - you mean we should not iterate all array entries, but only the relevant ones (e.g., if using a union, no need to iterate all?)
    And not sure I understand, for example, in calc_internal in Rule::filter in the else clause we pass true and not the param we got flat_arrays_on_filter
  • Added tests which panicked (filter_inner, filter_nested) so they are commented out to be reviewed later

MeirShpilraien and others added 2 commits May 3, 2022 18:21
1. Pop last token from json path
2. Check if the json path is static path (lead to single result)
3. Check the amount of tokens on the json path
@oshadmi
Copy link
Author

oshadmi commented Aug 28, 2022

Replaced by #11

@oshadmi oshadmi closed this Aug 28, 2022
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.

2 participants