-
Notifications
You must be signed in to change notification settings - Fork 711
feat: add env variable to toggle full text search type #4471
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
Conversation
WalkthroughThe changes introduce a new public field Changes
Possibly related PRs
Suggested labels
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/config/src/config.rs (2 hunks)
- src/config/src/meta/inverted_index/search.rs (2 hunks)
- src/service/search/grpc/storage.rs (3 hunks)
Additional context used
Path-based instructions (3)
src/config/src/meta/inverted_index/search.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
src/service/search/grpc/storage.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
src/config/src/config.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Additional comments not posted (4)
src/config/src/meta/inverted_index/search.rs (1)
45-81: Review ofPrefixSearchstruct and its methods
Constructor (
newmethod, lines 51-56): The method correctly clones the terms and stores a reference toColumnIndexMeta. This is efficient and follows Rust's ownership rules.Search Method (lines 58-69): The method uses the
filterfunction to preprocess terms and then constructs matchers using theStartsWithautomaton. This is a good use of thefstcrate for efficient prefix matching. However, ensure that theIndexReaderand its methods (fstandget_bitmap) are implemented to handle asynchronous operations correctly.Filter Method (lines 71-80): The method retains terms based on length and value constraints. The logic correctly checks the conditions using
metaproperties. This method efficiently reduces the number of terms to be processed in the search method, which can improve performance.Overall, the implementation of
PrefixSearchappears robust and well-suited for the intended functionality of prefix-based searching. Ensure that associated unit tests cover various edge cases, such as terms at the boundary of length and value constraints.src/service/search/grpc/storage.rs (1)
22-22: Update import statement to includePrefixSearch.The addition of
PrefixSearchin the import statement is necessary for the new functionality introduced in this PR. This change is consistent with the PR's objective to allow toggling between different search types.src/config/src/config.rs (2)
709-714: Review: New configuration fieldfull_text_search_typeThe addition of the
full_text_search_typefield in theCommonstruct is well implemented. The field is properly annotated with#[env_config]to specify the environment variableZO_FULL_TEXT_SEARCH_TYPE, a default value of"contains", and a helpful description. This aligns with the PR's objective to allow toggling between "Contains" and "Prefix" search types.
1146-1147: Review: Updated help descriptions forreplicasandhistoryin theNatsstructThe updated help descriptions for the
replicasandhistoryfields provide clearer guidance on their usage and limitations, specifically noting that these settings cannot be modified after the bucket is initialized. This is a good improvement for user documentation and helps prevent configuration errors.Also applies to: 1154-1155
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/service/search/cluster/mod.rs (1 hunks)
- src/service/search/grpc/storage.rs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/service/search/grpc/storage.rs
Additional context used
Path-based instructions (1)
src/service/search/cluster/mod.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/config/src/config.rs (2 hunks)
- src/service/search/grpc/storage.rs (4 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/config/src/config.rs
- src/service/search/grpc/storage.rs
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/service/search/grpc/storage.rs (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/service/search/grpc/storage.rs
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/service/search/cluster/mod.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/service/search/cluster/mod.rs
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/config/src/config.rs (2 hunks)
- src/config/src/meta/inverted_index/search.rs (3 hunks)
- src/service/search/cluster/mod.rs (3 hunks)
- src/service/search/grpc/storage.rs (7 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/config/src/config.rs
- src/config/src/meta/inverted_index/search.rs
- src/service/search/cluster/mod.rs
- src/service/search/grpc/storage.rs
New environment variable to check toggle between Contains and Prefix search.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation