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 support for approximate k-NN queries #548

Merged
merged 9 commits into from
Jul 5, 2023

Conversation

Xtansia
Copy link
Collaborator

@Xtansia Xtansia commented Jul 3, 2023

Description

Adds support for the approximate k-NN query type to pair with the support for the knn_vector type added in #524. Along with a couple bug fixes for that support.

Issues Resolved

#539

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.

Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
@Xtansia Xtansia added the backport 2.x Backport to 2.x branch label Jul 3, 2023
Signed-off-by: Thomas Farr <[email protected]>
@@ -1066,7 +1066,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {

}
if (this.knnAlgoParamEfSearch != null) {
generator.writeKey("knn.algo_param_ef_search");
generator.writeKey("knn.algo_param.ef_search");
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -1066,7 +1066,7 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {

}
if (this.knnAlgoParamEfSearch != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to this change, but it seems like a bunch of KNN index settings are missed:

"settings" : {
        "index": {
            "knn": true,
            "knn.algo_param.m":90,
            "knn.algo_param.ef_construction":200,
            "knn.algo_param.ef_search":300
        }
    },

https://github.com/opensearch-project/k-NN/blob/main/RFC.md?plain=1

@@ -28,8 +28,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
## [Unreleased 2.x]

### Added
- Add support for knn_vector field type ([#529](https://github.com/opensearch-project/opensearch-java/pull/524))
- Add support for knn_vector field type ([#524](https://github.com/opensearch-project/opensearch-java/pull/524))
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

reta
reta previously approved these changes Jul 4, 2023
@wbeckler wbeckler mentioned this pull request Jul 5, 2023
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Do we need changes to USER_GUIDE? Can be followed up later.

@dblock dblock merged commit f427f27 into opensearch-project:main Jul 5, 2023
30 checks passed
@VachaShah VachaShah added backport 2.x Backport to 2.x branch and removed backport 2.x Backport to 2.x branch labels Jul 5, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 5, 2023
* Add Knn query type

Signed-off-by: Thomas Farr <[email protected]>

* Integration test

Signed-off-by: Thomas Farr <[email protected]>

* Checkstyle fix

Signed-off-by: Thomas Farr <[email protected]>

* Run unreleased test

Signed-off-by: Thomas Farr <[email protected]>

* Fixes

Signed-off-by: Thomas Farr <[email protected]>

* Assume knn plugin installed

Signed-off-by: Thomas Farr <[email protected]>

* Changelog

Signed-off-by: Thomas Farr <[email protected]>

* Add to QueryBuilders

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
(cherry picked from commit f427f27)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Jul 5, 2023
* Add support for approximate k-NN queries (#548)

* Add Knn query type

Signed-off-by: Thomas Farr <[email protected]>

* Integration test

Signed-off-by: Thomas Farr <[email protected]>

* Checkstyle fix

Signed-off-by: Thomas Farr <[email protected]>

* Run unreleased test

Signed-off-by: Thomas Farr <[email protected]>

* Fixes

Signed-off-by: Thomas Farr <[email protected]>

* Assume knn plugin installed

Signed-off-by: Thomas Farr <[email protected]>

* Changelog

Signed-off-by: Thomas Farr <[email protected]>

* Add to QueryBuilders

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
(cherry picked from commit f427f27)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Fixing compile errors

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Vacha Shah <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Vacha Shah <[email protected]>
@Xtansia Xtansia deleted the feat/approx-knn-query branch June 14, 2024 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants