Add Querying Support to Lucene Byte Sized Vector #956

naveentatikonda · 2023-07-07T23:41:13Z

Description

The changes in this PR adds querying support to lucene byte sized vector.

Issues Resolved

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.

Signed-off-by: Naveen Tatikonda <[email protected]>

jmazanec15 · 2023-07-11T18:00:28Z

src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java

@@ -252,6 +256,15 @@ protected Query doToQuery(QueryShardContext context) {
            );
        }

+        byte[] byteVector = new byte[0];


why set this to empty array?

We need to initialize it or else it is not letting us use it while building the QueryRequest because the logic to add values into this array is inside an if statement.

jmazanec15 · 2023-07-11T18:00:51Z

src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java

+            byteVector = new byte[vector.length];
+            for (int i = 0; i < vector.length; i++) {
+                validateByteVectorValue(vector[i]);
+                byteVector[i] = (byte) vector[i];


Why can we cast like this?

To convert it from float to byte type. The value will still remain the same after casting because we have already validated to make sure it is within the byte range and without any decimal values.

src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java

navneet1v · 2023-07-11T18:30:42Z

src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java

            );
-            return new KnnFloatVectorQuery(fieldName, vector, k, filterQuery);
+            if (VectorDataType.BYTE.equals(vectorDataType)) {


Also, I see there are duplicated checks. Can we just abstract them in a private function which will give us right Vector Query.

src/test/java/org/opensearch/knn/index/LuceneEngineIT.java

Signed-off-by: Naveen Tatikonda <[email protected]>

navneet1v · 2023-07-11T19:27:38Z

@naveentatikonda please check why github checks are failing.

vamshin

LGTM! Thanks

naveentatikonda · 2023-07-11T20:51:46Z

@naveentatikonda please check why github checks are failing.

They are still failing because of the changes in core. The latest core changes are not yet synced properly to some of the dependencies. The integ tests are still failing looking for old method signature where as the unit tests are succeeding because they got synced.

) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]>

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]>

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> (cherry picked from commit bf04854)

* Add Indexing Support for Lucene Byte Sized Vector (#937) * Add Indexing Support for Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add tests for Indexing Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add Querying Support to Lucene Byte Sized Vector (#956) * Add Querying Support to Lucene Byte Sized Vector Signed-off-by: Naveen Tatikonda <[email protected]> * Add CHANGELOG Signed-off-by: Naveen Tatikonda <[email protected]> * Address Review Comments Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> * Add DocValues Support for Lucene Byte Sized Vector (#953) Signed-off-by: Naveen Tatikonda <[email protected]> * Update Release Notes Signed-off-by: Naveen Tatikonda <[email protected]> --------- Signed-off-by: Naveen Tatikonda <[email protected]> (cherry picked from commit bf04854) Signed-off-by: Naveen Tatikonda <[email protected]>

naveentatikonda added 2 commits July 7, 2023 18:32

Add Querying Support to Lucene Byte Sized Vector

7af1cce

Signed-off-by: Naveen Tatikonda <[email protected]>

Add CHANGELOG

76b447d

Signed-off-by: Naveen Tatikonda <[email protected]>

naveentatikonda added Features Introduces a new unit of functionality that satisfies a requirement v2.9.0 labels Jul 7, 2023

naveentatikonda self-assigned this Jul 7, 2023

naveentatikonda requested review from heemin32, navneet1v, VijayanB, vamshin, jmazanec15, junqiu-lei and martin-gaievski as code owners July 7, 2023 23:41

junqiu-lei approved these changes Jul 11, 2023

View reviewed changes

jmazanec15 reviewed Jul 11, 2023

View reviewed changes

naveentatikonda requested a review from jmazanec15 July 11, 2023 18:27

navneet1v reviewed Jul 11, 2023

View reviewed changes

src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java Outdated Show resolved Hide resolved

navneet1v reviewed Jul 11, 2023

View reviewed changes

src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java Outdated Show resolved Hide resolved

navneet1v reviewed Jul 11, 2023

View reviewed changes

src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java Outdated Show resolved Hide resolved

navneet1v reviewed Jul 11, 2023

View reviewed changes

src/test/java/org/opensearch/knn/index/LuceneEngineIT.java Outdated Show resolved Hide resolved

Address Review Comments

6caca10

Signed-off-by: Naveen Tatikonda <[email protected]>

navneet1v approved these changes Jul 11, 2023

View reviewed changes

vamshin approved these changes Jul 11, 2023

View reviewed changes

naveentatikonda merged commit 8877c4c into opensearch-project:feature/lucene_byte_vector Jul 11, 2023
4 of 34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Querying Support to Lucene Byte Sized Vector #956

Add Querying Support to Lucene Byte Sized Vector #956

naveentatikonda commented Jul 7, 2023

jmazanec15 Jul 11, 2023

naveentatikonda Jul 11, 2023

jmazanec15 Jul 11, 2023

naveentatikonda Jul 11, 2023

navneet1v Jul 11, 2023

navneet1v commented Jul 11, 2023

vamshin left a comment

naveentatikonda commented Jul 11, 2023

Add Querying Support to Lucene Byte Sized Vector #956

Add Querying Support to Lucene Byte Sized Vector #956

Conversation

naveentatikonda commented Jul 7, 2023

Description

Issues Resolved

Check List

jmazanec15 Jul 11, 2023

Choose a reason for hiding this comment

naveentatikonda Jul 11, 2023

Choose a reason for hiding this comment

jmazanec15 Jul 11, 2023

Choose a reason for hiding this comment

naveentatikonda Jul 11, 2023

Choose a reason for hiding this comment

navneet1v Jul 11, 2023

Choose a reason for hiding this comment

navneet1v commented Jul 11, 2023

vamshin left a comment

Choose a reason for hiding this comment

naveentatikonda commented Jul 11, 2023