-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Explore Search Results #4960
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
Explore Search Results #4960
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.
seems to have ended up in the wrong directory. should have been src/test/java, right?
| * Configuration whether to use ANN or ENN for the search. ANN is the default. | ||
| * | ||
| * @return the search type to use. | ||
| */ | ||
| VectorSearchOperation.SearchType searchType() default VectorSearchOperation.SearchType.ENN; |
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.
Documentation (default: ANN) contradicted by implementation (default: ENN).
MongoDB itself will use ANN by default (so the doc) is closer to what happens than the actual default value.
I think the MongoDB restrictions here do not allow much flexibility regarding defaulting of values. Like on the one hand ANN ist stated as default but requires numCandidates while the exact flag is labelled optional.
We could argue DEFAULT is a viable option here. Checking the numCandidates attribute, using ENN if no value present, ANN otherwise.
| * @author Mark Paluch | ||
| * @since 5.0 | ||
| */ | ||
| class VectorSearchExecution implements MongoQueryExecution { |
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.
curious if we can derive a more AOT friendly variant of this.
| if (query.isLimited()) { | ||
| limit = query.getLimit(); | ||
| } else { | ||
| limit = Math.max(1, numCandidates() != null ? numCandidates() / 20 : 1); |
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.
limit cannot exceed numCandidates and numCandidates should be at least 20x higher than limit but is deriving the limit the way it's done here valid?
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.
Probably doesn't make sense.
511ac4a to
f5cab5d
Compare
fix invalid domain type reference and make sure to include required arguemtns
f5cab5d to
72806aa
Compare
Original Pull Request: #4960
|
merged to 5.0.x |
Original Pull Request: #4960
Return
SearchResultfrom Repository Query methods.Depends on spring-projects/spring-data-commons#3285