Skip to content
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

Add filter function to KNNQueryBuilder #2585

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chloewqg
Copy link

@chloewqg chloewqg commented Mar 6, 2025

Description

Add filter function to KNNQueryBuilder

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

* @param filterToBeAdded filter to be added
* @return return itself with underlying filter combined with passed in filter
*/
public QueryBuilder filter(QueryBuilder filterToBeAdded) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a Builder created to be able to set the filter - why is this needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the issue here opensearch-project/neural-search#1135 where we want to add common filter from HybridQueryBuilder to all queries below, including KNNQueryBuilder. So it means adding additional filter to previous filter.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying an existing query builder is not a great idea, the builder is created to make sure its immutable in a way and every parameter is validated together rather than individually - you need to create a copy of it in neural side to support the use case

KNNQueryBuilder initialqueryBuidler = //whatever it is

KNNQueryBuilder filterQueryBuilder = KNNQueryBuilder.builder().filter(filterTobeAdded).k(initialQuery.k()) and so on before calling .build()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants