refactor: change InvertedIndexWriter method signature to offsets to f… #4250
Conversation
…acilliate caching
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe recent changes modify how the Changes
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/index/src/inverted_index/format/reader.rs (2 hunks)
- src/index/src/inverted_index/format/reader/blob.rs (4 hunks)
- src/index/src/inverted_index/format/writer/blob.rs (2 hunks)
- src/index/src/inverted_index/search/fst_values_mapper.rs (2 hunks)
- src/index/src/inverted_index/search/index_apply/predicates_apply.rs (6 hunks)
Additional comments not posted (19)
src/index/src/inverted_index/format/reader.rs (2)
33-34: LGTM! But verify the function usage in the codebase.The method signature for
fsthas been updated to useoffsetandsizeparameters.However, ensure that all function calls to
fstmatch the new signature.
36-37: LGTM! But verify the function usage in the codebase.The method signature for
bitmaphas been updated to useoffsetandsizeparameters.However, ensure that all function calls to
bitmapmatch the new signature.Verification successful
All function calls to
bitmapmatch the new signature.The verification confirms that all instances of the
bitmapfunction have been updated to use theoffset: u64andsize: u32parameters as specified in the new method signature.
src/index/src/inverted_index/search/fst_values_mapper.rssrc/index/src/inverted_index/format/writer/blob.rssrc/index/src/inverted_index/format/reader/blob.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust $'bitmap'Length of output: 24853
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature with `offset: u64` and `size: u32`. # Pattern to match function calls to `bitmap` with the new signature. ast-grep --lang rust --pattern '$_ .bitmap($_, $_)'Length of output: 2127
src/index/src/inverted_index/search/fst_values_mapper.rs (2)
51-51: LGTM! But verify the function usage in the codebase.The
bitmapmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.Verification successful
Verify all function calls to
bitmapmatch the new signature.The
bitmapmethod is used in multiple files. Ensure that all instances follow the new signature with the updated offset calculation.
All
bitmapmethod calls follow the new signature.The
bitmapmethod calls across the codebase have been verified, and they all include the updated offset calculation as expected.
src/index/src/inverted_index/search/fst_values_mapper.rssrc/index/src/inverted_index/search/index_apply/predicates_apply.rssrc/index/src/inverted_index/format/reader.rssrc/index/src/inverted_index/format/writer/blob.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust $'bitmap'Length of output: 24853
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Extract all instances of the `bitmap` method calls and their surrounding lines for context. rg 'bitmap\(' -A 2 -B 2Length of output: 22265
82-82: LGTM! But verify the function usage in the codebase.The mock implementation for
bitmaphas been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.src/index/src/inverted_index/format/writer/blob.rs (4)
177-183: LGTM! But verify the function usage in the codebase.The
fstmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
fstmatch the new signature.
211-217: LGTM! But verify the function usage in the codebase.The
fstmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
fstmatch the new signature.
186-201: LGTM! But verify the function usage in the codebase.The
bitmapmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.
220-235: LGTM! But verify the function usage in the codebase.The
bitmapmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.Verification successful
All function calls to
bitmapmatch the new signature.The
bitmapmethod calls in the following files have been verified to use the new offset calculation:
src/index/src/inverted_index/format/writer/blob.rssrc/index/src/inverted_index/search/fst_values_mapper.rssrc/index/src/inverted_index/format/reader/blob.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust $'bitmap'Length of output: 24853
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Searching for the definition of the `bitmap` function to understand its signature. rg 'async fn bitmap' --type rust # Searching for all instances of the `bitmap` function call to verify their arguments. rg '\.bitmap\(' --type rustLength of output: 1635
src/index/src/inverted_index/format/reader/blob.rs (6)
64-69: LGTM! But verify the function usage in the codebase.The
fstmethod implementation has been updated to use the new offset calculation.However, ensure that all function calls to
fstmatch the new signature.
216-222: LGTM! But verify the function usage in the codebase.The
fstmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
fstmatch the new signature.
244-246: LGTM! But verify the function usage in the codebase.The
bitmapmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.
204-210: LGTM! But verify the function usage in the codebase.The
fstmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
fstmatch the new signature.
236-238: LGTM! But verify the function usage in the codebase.The
bitmapmethod call has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.Verification successful
Verification Successful
All function calls to
bitmapmatch the new signature with the parametersoffsetandsize.
- src/index/src/inverted_index/search/fst_values_mapper.rs
- src/index/src/inverted_index/format/reader/blob.rs
- src/index/src/inverted_index/format/writer/blob.rs
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust $'bitmap'Length of output: 24853
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type rust 'bitmap\('Length of output: 7350
75-79: LGTM! But verify the function usage in the codebase.The
bitmapmethod implementation has been updated to use the new offset calculation.However, ensure that all function calls to
bitmapmatch the new signature.Verification successful
All
bitmapfunction calls match the new signature.The
bitmapfunction calls found in the codebase correctly use the new signature with two parameters:offsetandsize.
src/index/src/inverted_index/search/fst_values_mapper.rs: .bitmap(self.metadata.base_offset + relative_offset as u64, size)src/index/src/inverted_index/format/reader/blob.rs: let bitmap = blob_reader.bitmap(meta.base_offset, 2).await.unwrap();src/index/src/inverted_index/format/writer/blob.rs: .bitmap(tag0.base_offset + offset as u64, size)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `bitmap` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust $'bitmap'Length of output: 24853
Script:
#!/bin/bash # Search for function calls to `bitmap` and verify their signatures. ast-grep --lang rust --pattern '$_.$bitmap($_, $_)' --jsonLength of output: 408
Script:
#!/bin/bash # Search for function calls to `bitmap` and verify their signatures. ast-grep --lang rust --pattern '$_.$bitmap($_, $_)' --jsonLength of output: 408
Script:
#!/bin/bash # Search for function calls to `bitmap` and verify their signatures. rg '\.bitmap\(' --type rustLength of output: 1354
src/index/src/inverted_index/search/index_apply/predicates_apply.rs (5)
79-80: LGTM!The calculation of
fst_offsetandfst_sizeis correct and aligns with the new method signature.
81-81: LGTM!The call to
reader.fstwith the new parameters is correct and aligns with the new method signature.
164-164: LGTM!The
mock_metasfunction signature is updated correctly to accept tuples of(&'static str, u32).
204-204: LGTM!The
mock_readeris correctly updated to return metadata with the new structure.
205-207: LGTM!The test setup is correctly updated to call
reader.fstwith the new parameters.
|
@coderabbitai pause |
Actions performedReviews paused. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4250 +/- ##
==========================================
- Coverage 84.96% 84.69% -0.28%
==========================================
Files 1052 1052
Lines 186787 186830 +43
==========================================
- Hits 158705 158233 -472
- Misses 28082 28597 +515 |
|
@zhongzc PTAL |
…acilliate caching
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Change methods signatures of
InvertedIndexWriterto offset/size so that we can add a cache layer from arguments provided.Checklist
Summary by CodeRabbit
Refactor
fstandbitmapmethods to improve performance by using direct offsets and sizes instead of metadata references.Tests