Skip to content

MOD-4602: handle jsonpath filter with 3 or more operands #890

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

Merged
merged 7 commits into from
Dec 22, 2022

Conversation

oshadmi
Copy link
Collaborator

@oshadmi oshadmi commented Dec 18, 2022

Fix #889

@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Base: 81.44% // Head: 81.36% // Decreases project coverage by -0.08% ⚠️

Coverage data is based on head (3e221bb) compared to base (1a978ec).
Patch coverage: 65.38% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #890      +/-   ##
==========================================
- Coverage   81.44%   81.36%   -0.09%     
==========================================
  Files          14       14              
  Lines        3865     3896      +31     
==========================================
+ Hits         3148     3170      +22     
- Misses        717      726       +9     
Impacted Files Coverage Δ
src/jsonpath/mod.rs 85.71% <ø> (ø)
src/jsonpath/json_path.rs 69.34% <65.38%> (+0.07%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@oshadmi oshadmi marked this pull request as ready for review December 22, 2022 08:28
Cargo.toml Outdated
@@ -16,6 +16,10 @@ categories = ["database-implementations"]
crate-type = ["cdylib", "rlib"]
name = "rejson"

[dev-dependencies]
env_logger = "0.10.0"
redis-module = { version="1.0", features = ["experimental-api", "test"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets just make sure this will not somehow effect the final .so file itself, we need to see that the redisjson.so are still using the Redis allocator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll comment it out

Copy link
Collaborator

@MeirShpilraien MeirShpilraien left a comment

Choose a reason for hiding this comment

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

Looks good,
One comment.

@oshadmi oshadmi merged commit 6b30093 into master Dec 22, 2022
@oshadmi oshadmi deleted the omer_jsonpath_filter_ignore_3rd_expr branch December 22, 2022 12:47
oshadmi added a commit that referenced this pull request Dec 22, 2022
* enable tracing jsonpath tests

* add flow test

* Evaluate all filter operands

* Add tests with precedence and nesting

* update spellcheck dictionary

* remove redis-module from dev-dependencies

(cherry picked from commit 6b30093)
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.

Filtering with more than two keys
2 participants