-
Notifications
You must be signed in to change notification settings - Fork 619
Vector search on repository level #3037
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
ec86ddb to
8586fb2
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.
Awesome work, congrats. I left a few high-level thoughts on the PR which might be useful to reduce repetition.
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.
Could QueryMethod.isSearchQuery() be useful here?
Would it also make sense to check for the presence of the @VectorSearch annotation?
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.
Design-wise, using a builder could help to avoid introducing additional constructors for such a class when extending arguments. That's a learning from various other Spring Data modules that can make your changes less painful.
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.
Nit: It makes sense to check how these exceptions translate during runtime and whether an enclosing exception provides sufficient context to identify the query method that has caused this verification error.
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.
Nit: A bit of Javadoc helps with dev experience, also, @since tags for feature discoverability.
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.
Package-protected records FTW. We use these in a similar manner.
Nerd sniping: OptionalDouble 😵💫
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.
Another nerd-snipe: Inject a Session that gets closed after the test run to reduce try repetition.
Signed-off-by: Gerrit Meier <meistermeier@gmail.com>
Signed-off-by: Gerrit Meier <meistermeier@gmail.com> Closes #3002
baab1c7 to
d1b7692
Compare
Closed with merge of d1b7692
This will introduce the support for Neo4j vector search.
For now it's focused on the repository level to enhance the derived finder methods.