-
Notifications
You must be signed in to change notification settings - Fork 333
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
feat(inverted_index.search): add index applier #2868
Conversation
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
…' into zhongzc/index-applier
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
…' into zhongzc/index-applier
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #2868 +/- ##
===========================================
- Coverage 84.78% 84.33% -0.46%
===========================================
Files 745 749 +4
Lines 117025 117881 +856
===========================================
+ Hits 99223 99417 +194
- Misses 17802 18464 +662 |
Signed-off-by: Zhenchi <[email protected]>
…selectivity Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
Signed-off-by: Zhenchi <[email protected]>
74c2c67
to
91a1338
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! A few suggestions attached.
Another suggestion: maybe we can add some benchmarks for inverted index performance to get a baseline and prepare for future improvements.
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Zhenchi <[email protected]>
Agreed. I will construct some bench cases with local files as the baseline in the upcoming PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Zhenchi <[email protected]>
src/index/src/inverted_index/search/index_apply/predicates_apply.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: Zhenchi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
Introduce
IndexApplier
to apply the predefined predicates to the data read by the given index reader, returning a list of relevant indices.Applier instances are reusable and work with various
InvertedIndexReader
instances, avoiding repeated compilation of fixed predicates such as regex patterns.Checklist
Refer to a related PR or issue link (optional)
Prerequisite merging GreptimeTeam/greptime-proto#126
#2705