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 for AbstractQueryBuilder, BoolQueryBuilder, ConstantScoreQueryBuilder #17409

Merged
merged 1 commit into from
Mar 3, 2025

Conversation

chloewqg
Copy link
Contributor

@chloewqg chloewqg commented Feb 21, 2025

Description

The filter function will combine a filter with the query builder. If the query builder itself has a filter we will combine the filter and return the query builder itself. If no we will use a bool query builder to combine the query builder and the filter and then return the bool query builder.

Related Issues

Resolves #17409

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Copy link
Contributor

❌ Gradle check result for 6704b99: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 7577d38: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@chloewqg chloewqg force-pushed the local-main branch 2 times, most recently from bb06326 to 4b29713 Compare February 21, 2025 05:09
Copy link
Contributor

❌ Gradle check result for 4b29713: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@chloewqg chloewqg changed the title Add filter function for AbstractQueryBuilder Add filter function for AbstractQueryBuilder, BoolQueryBuilder, ConstantScoreQueryBuilder Feb 21, 2025
@chloewqg chloewqg marked this pull request as ready for review February 21, 2025 05:45
Copy link
Contributor

❌ Gradle check result for a2983d4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

dbwiddis
dbwiddis previously approved these changes Feb 26, 2025
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

LGTM pending passing gradle checks (and resolving the discussion on need for an enum)

@dbwiddis dbwiddis dismissed their stale review February 26, 2025 18:07

Still an open conversation on whether an enum is required.

Copy link
Contributor

❌ Gradle check result for a2983d4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

chloewqg added a commit to chloewqg/OpenSearch that referenced this pull request Feb 26, 2025
…antScoreQueryBuilder. (opensearch-project#17409)

* The filter function will combine a filter with the query builder. If the query builder itself has a filter we will combine the filter and return the query builder itself. If no we will use a bool query builder to combine the query builder and the filter and then return the bool query builder.

Signed-off-by: Chloe Gao <chloewq@amazon.com>
Copy link
Contributor

❕ Gradle check result for 7625494: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.41%. Comparing base (0ffed5e) to head (4de6f56).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...nsearch/index/query/ConstantScoreQueryBuilder.java 66.66% 1 Missing and 1 partial ⚠️
...g/opensearch/index/query/SpanNearQueryBuilder.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17409      +/-   ##
============================================
- Coverage     72.42%   72.41%   -0.01%     
- Complexity    65611    65653      +42     
============================================
  Files          5304     5306       +2     
  Lines        304743   304608     -135     
  Branches      44189    44164      -25     
============================================
- Hits         220701   220592     -109     
- Misses        65888    65923      +35     
+ Partials      18154    18093      -61     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dbwiddis
Copy link
Member

LGTM. Will wait until tomorrow to merge in case any other maintainer has any comments.

@msfroh
Copy link
Collaborator

msfroh commented Feb 28, 2025

Thanks, @chloewqg! Looks good to me!

chloewqg added a commit to chloewqg/OpenSearch that referenced this pull request Mar 1, 2025
…antScoreQueryBuilder. (opensearch-project#17409)

* The filter function will combine a filter with the query builder. If the query builder itself has a filter we will combine the filter and return the query builder itself. If no we will use a bool query builder to combine the query builder and the filter and then return the bool query builder.

Signed-off-by: Chloe Gao <chloewq@amazon.com>
@chloewqg chloewqg requested a review from bugmakerrrrrr as a code owner March 1, 2025 23:01
Copy link
Contributor

github-actions bot commented Mar 1, 2025

❌ Gradle check result for 0c28541: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

…antScoreQueryBuilder. (opensearch-project#17409)

* The filter function will combine a filter with the query builder. If the query builder itself has a filter we will combine the filter and return the query builder itself. If no we will use a bool query builder to combine the query builder and the filter and then return the bool query builder.

Signed-off-by: Chloe Gao <chloewq@amazon.com>
Copy link
Contributor

github-actions bot commented Mar 2, 2025

❕ Gradle check result for 4de6f56: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@dbwiddis dbwiddis merged commit bfdf019 into opensearch-project:main Mar 3, 2025
34 checks passed
mayanksharma27 pushed a commit to mayanksharma27/OpenSearch that referenced this pull request Mar 5, 2025
…antScoreQueryBuilder. (opensearch-project#17409) (opensearch-project#17409)

* The filter function will combine a filter with the query builder. If the query builder itself has a filter we will combine the filter and return the query builder itself. If no we will use a bool query builder to combine the query builder and the filter and then return the bool query builder.

Signed-off-by: Chloe Gao <chloewq@amazon.com>
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.

6 participants