-
Notifications
You must be signed in to change notification settings - Fork 161
fix: don't extract fields from lexical search indexes #725
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
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.
Pull Request Overview
This PR fixes a bug where the code incorrectly attempted to extract filterable fields from all search indexes, including lexical (Atlas Search) indexes, when it should only process vector search indexes. The fix ensures that only vector search indexes are considered when validating filter fields in aggregation pipelines.
Key changes:
- Added filtering logic to exclude non-vector search indexes before validation
- Renamed parameters to clarify that only vector search indexes are expected
- Added comprehensive test coverage for the mixed index scenario
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/helpers/assertVectorSearchFilterFieldsAreIndexed.ts |
Added type field to VectorSearchIndex type and filtering logic to exclude lexical search indexes; renamed parameter for clarity |
src/tools/mongodb/read/aggregate.ts |
Added filtering to exclude non-vector search indexes before passing to validation function; updated parameter name |
tests/unit/helpers/assertVectorSearchFilterFieldsAreIndexed.test.ts |
Updated all test cases with renamed parameter and added new test case for mixed vector and Atlas search indexes |
himanshusinghs
left a comment
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.
Looks good, left a note about duplicate filtering.
Pull Request Test Coverage Report for Build 19330851331Details
💛 - Coveralls |
While testing things out, I came across a corner case where running an aggregation on a cluster that has both vector and atlas search indexes results in an error being thrown when trying to extract the filterable fields from the index definition. The problem was caused by the fact we were assuming we're always operating on a vector search definition, but in reality, we could be working with an atlas search one.