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 IllegalArgumentException for Lucene radial search with ef_search #1765

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Jun 17, 2024

Description

Add IllegalArgumentException for Lucene radial search with ef_search parameter. Context at #1537 (comment)

Issues Resolved

Part of #1537

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

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
Member

@ryanbogan ryanbogan left a comment

Choose a reason for hiding this comment

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

LGTM!

@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 9f5bdf4 to 2744953 Compare June 17, 2024 21:43
@junqiu-lei
Copy link
Member Author

The CI failure in Test k-NN on Secure Cluster is related to opensearch-project/security-dashboards-plugin#2009

@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from e008711 to 515f47e Compare June 18, 2024 01:25
@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 515f47e to 688b70d Compare June 18, 2024 01:30
Signed-off-by: Junqiu Lei <[email protected]>
@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 7f594a9 to 26ec968 Compare June 18, 2024 22:10
@junqiu-lei junqiu-lei requested review from shatejas and ryanbogan June 18, 2024 23:39
Copy link
Collaborator

@shatejas shatejas left a comment

Choose a reason for hiding this comment

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

Looks good overall. Some minor changes

@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 0a39e79 to 067cdc6 Compare June 19, 2024 21:03
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/ef-search@4838481). Learn more about missing BASE report.

Current head 9c8eab8 differs from pull request most recent head 4a1c373

Please upload reports for the commit 4a1c373 to get more accurate results.

Files Patch % Lines
...rg/opensearch/knn/index/query/KNNQueryBuilder.java 57.14% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##             feature/ef-search    #1765   +/-   ##
====================================================
  Coverage                     ?   84.84%           
  Complexity                   ?     1517           
====================================================
  Files                        ?      184           
  Lines                        ?     6196           
  Branches                     ?      667           
====================================================
  Hits                         ?     5257           
  Misses                       ?      662           
  Partials                     ?      277           

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

@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 067cdc6 to 94ba1c8 Compare June 20, 2024 16:49
@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from 94ba1c8 to 9c8eab8 Compare June 20, 2024 18:11
@junqiu-lei junqiu-lei force-pushed the radial-luene-ef-search branch from e57b482 to 4a1c373 Compare June 20, 2024 20:45
@junqiu-lei junqiu-lei merged commit 0c41dc5 into opensearch-project:feature/ef-search Jun 20, 2024
46 of 51 checks passed
@junqiu-lei junqiu-lei deleted the radial-luene-ef-search branch June 20, 2024 21:41
junqiu-lei added a commit to junqiu-lei/k-NN that referenced this pull request Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants