Skip to content

Add support for binary and byte field support in doc_values - #3340

Merged
navneet1v merged 1 commit into
opensearch-project:mainfrom
navneet1v:main
May 31, 2026
Merged

Add support for binary and byte field support in doc_values#3340
navneet1v merged 1 commit into
opensearch-project:mainfrom
navneet1v:main

Conversation

@navneet1v

@navneet1v navneet1v commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Add support for binary and byte field support in doc_values

Related Issues

Resolves #3315

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.

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 556ef1a)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible Issue

The toIntArray method converts signed bytes to ints without preserving the unsigned interpretation. For example, byte value -128 becomes int -128, not 128. If the consumer expects unsigned byte values (0-255 range), this will produce incorrect results. This matters when the API contract or downstream code expects unsigned byte semantics for BYTE/BINARY vectors.

private static int[] toIntArray(final byte[] bytes) {
    final int[] ints = new int[bytes.length];
    for (int i = 0; i < bytes.length; i++) {
        ints[i] = bytes[i];
    }
    return ints;
}

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to 556ef1a

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix byte-to-int sign extension issue

The conversion from byte[] to int[] doesn't handle byte sign extension correctly.
When a byte is cast to int, Java sign-extends it (e.g., byte -1 becomes int -1), but
the test expects unsigned interpretation (e.g., byte -86 should become int 170, not
-86). Apply bitwise AND with 0xFF to convert bytes to unsigned integers.

src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [182-188]

 private static int[] toIntArray(final byte[] bytes) {
     final int[] ints = new int[bytes.length];
     for (int i = 0; i < bytes.length; i++) {
-        ints[i] = bytes[i];
+        ints[i] = bytes[i] & 0xFF;
     }
     return ints;
 }
Suggestion importance[1-10]: 9

__

Why: This is a critical bug. Without the & 0xFF mask, negative bytes are sign-extended to negative integers (e.g., byte -86 becomes int -86 instead of 170). The test cases in the PR expect unsigned interpretation, as shown by binaryVector1 = { -86 } being compared against the result. This will cause incorrect serialization of byte/binary vectors in array format.

High

Previous suggestions

Suggestions up to commit 5302f07
CategorySuggestion                                                                                                                                    Impact
Possible issue
Convert bytes to unsigned integers

The conversion from byte[] to int[] doesn't preserve unsigned byte values correctly.
Java bytes are signed (-128 to 127), but when converting to int, negative bytes
remain negative. For proper representation of byte values (0-255), use bytes[i] &
0xFF to convert to unsigned int.

src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [182-188]

 private static int[] toIntArray(final byte[] bytes) {
     final int[] ints = new int[bytes.length];
     for (int i = 0; i < bytes.length; i++) {
-        ints[i] = bytes[i];
+        ints[i] = bytes[i] & 0xFF;
     }
     return ints;
 }
Suggestion importance[1-10]: 3

__

Why: The suggestion raises a valid point about signed byte conversion, but the impact depends on the intended use case. The PR's test cases show that signed byte values (e.g., -3, -128) are expected to remain negative in the int[] output, suggesting the current implementation may be intentional. Without clear requirements for unsigned conversion, this is a minor style/interpretation issue rather than a critical bug.

Low
Suggestions up to commit 5302f07
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix signed byte to int conversion

The conversion from byte[] to int[] does not preserve unsigned byte values
correctly. Java bytes are signed (-128 to 127), but when converting to int, negative
bytes remain negative. For proper unsigned representation (0-255), use bytes[i] &
0xFF to mask the sign extension.

src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [182-188]

 private static int[] toIntArray(final byte[] bytes) {
     final int[] ints = new int[bytes.length];
     for (int i = 0; i < bytes.length; i++) {
-        ints[i] = bytes[i];
+        ints[i] = bytes[i] & 0xFF;
     }
     return ints;
 }
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical bug in the toIntArray method. Java bytes are signed (-128 to 127), and direct assignment to int preserves the sign, resulting in negative values. For proper unsigned representation (0-255), the code must use bytes[i] & 0xFF to mask sign extension. This is essential for correct serialization of BYTE/BINARY vectors as JSON numeric arrays.

High

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 5302f07

Signed-off-by: Navneet Verma <navneev@amazon.com>
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 556ef1a

@navneet1v navneet1v added the Enhancements Increases software capabilities beyond original client specifications label May 23, 2026
@codecov

codecov Bot commented May 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.51%. Comparing base (11bd374) to head (556ef1a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3340      +/-   ##
============================================
- Coverage     83.58%   83.51%   -0.07%     
+ Complexity     4308     4303       -5     
============================================
  Files           451      451              
  Lines         15584    15588       +4     
  Branches       2018     2020       +2     
============================================
- Hits          13026    13019       -7     
- Misses         1770     1779       +9     
- Partials        788      790       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@navneet1v navneet1v moved this from 3.7.0 to Now(This Quarter) in Vector Search RoadMap May 27, 2026

@shatejas shatejas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good overall

Comment thread src/test/java/org/opensearch/knn/integ/DocValueFieldsIT.java
@navneet1v
navneet1v merged commit 0a604d7 into opensearch-project:main May 31, 2026
51 of 53 checks passed
@github-project-automation github-project-automation Bot moved this from Now(This Quarter) to ✅ Done in Vector Search RoadMap May 31, 2026
navneet1v added a commit that referenced this pull request Jun 1, 2026
…alues (#3340) (#3349)

Signed-off-by: Navneet Verma <navneev@amazon.com>
naveentatikonda pushed a commit that referenced this pull request Jun 18, 2026
* Enhance unit test coverage for 32x defaults

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Add BwC test coverage (#3329)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Add base64 binary encoding as default format for knn_vector docvalue_fields (#3324)

Signed-off-by: Navneet Verma <navneev@amazon.com>

* Add issues write permission to untriaged label workflow (#3332)

Signed-off-by: shreyah963 <shreyab963@gmail.com>

* Fix score to radius conversion for IP with faiss (#3336)

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Co-authored-by: Tejas Shah <shatejas@amazon.com>

* Add ci.opensearch.org maven2 mirror to avoid throttling (#3345)

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

* [AUTO] Add release notes for 3.7.0 (#3342)

Signed-off-by: opensearch-ci-bot <opensearch-infra@amazon.com>

* Fix derived source for mixed-case vector fields (#3313)

* Fix derived source for mixed-case vector fields

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Add BWC coverage for derived source field casing

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Add changelog entry for mixed-case derived source fix

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Handle case-insensitive conflicts by preferring vector field

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Avoid stream wrappers for derived field lookup

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Handle ambiguous case-insensitive matches without vector hints

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Update src/main/java/org/opensearch/knn/index/codec/KNN10010Codec/KNN10010DerivedSourceStoredFieldsFormat.java

Co-authored-by: Tejas Shah <shatejas@amazon.com>
Signed-off-by: Wonjae Lee <38933452+leewjae@users.noreply.github.com>

* Apply spotless formatting for derived source field resolution

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Avoid guessing when case-insensitive matches lack vector hints

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Simplify case-insensitive derived field matching

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Trigger CI rerun for BWC investigation

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

* Add native engine field info coverage

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>

---------

Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>
Signed-off-by: Wonjae Lee <38933452+leewjae@users.noreply.github.com>
Signed-off-by: Tejas Shah <shatejas@amazon.com>
Co-authored-by: Tejas Shah <shatejas@amazon.com>
Co-authored-by: Navneet Verma <navneev@amazon.com>

* Fixes RescoreParser to pass the rescore flag (#3343)

* Fixes RescoreParser to pass the rescore flag

For multinode or coordinator-data node setup, rescore set to false is
not passed through streams. This causes rescoring to execute even when
its not disabled explicitly by user

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Updates Changelogs, improves code cov

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Makes the coordinator port dynamic

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Adds BWC test for mode and compression

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Does not create compressed indices before 2.18

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Fixes bwc

Signed-off-by: Tejas Shah <shatejas@amazon.com>

---------

Signed-off-by: Tejas Shah <shatejas@amazon.com>

* Merge rescore-radial-quantized feature branch to main (#3347)

* Rescoring after radial search on quantized index. [Task 1 - 4] (#3300)

* Bumped gradle to 9.4.1 and jacoco to 0.8.14 (#3308)

Signed-off-by: Andrew Klepchick <aklepchi@amazon.com>

* Use KNN1040ScalarQuantizedVectorsFormat for Faiss SQ flat format (#3302)

The Faiss SQ format was using Lucene's Lucene104ScalarQuantizedVectorsFormat
directly, which lacks the prefetch-enabled raw vector reader that
KNN1040ScalarQuantizedVectorsFormat provides. This meant exact search
rescoring was missing I/O prefetch during graph traversal.

Changes:
- Switch faissSqFlatFormat from Lucene104ScalarQuantizedVectorsFormat to
  KNN1040ScalarQuantizedVectorsFormat in Faiss1040ScalarQuantizedKnnVectorsFormat
- Add @VisibleForTesting getFlatVectorsReader() to
  Faiss1040ScalarQuantizedKnnVectorsReader to replace reflection in tests
- Add testGetRandomVectorScorer_returnsPrefetchableScorer in
  KNN1040ScalarQuantizedVectorsFormatTests verifying the scorer is
  PrefetchableRandomVectorScorer via a real write/read cycle
- Replace reflection with getter in
  Faiss1040ScalarQuantizedKnnVectorsFormatTests.testFieldsReader_thenWrapsFlatReaderWithPrefetchSupport

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

* Allow minScore, maxDistance for 32x SQ index.

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

Pass compression and quantization config to RNN query builder.

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

Added RescoreRadialSearchQuery.

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

Wiring `RescoreRadialSearchQuery` wrapper in `RNNQueryFactory`

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

---------

Signed-off-by: Andrew Klepchick <aklepchi@amazon.com>
Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
Signed-off-by: Dooyong Kim <kdooyong@amazon.com>
Co-authored-by: Andrew Klepchick <aklepchi@amazon.com>
Co-authored-by: Vijayan Balasubramanian <balasvij@amazon.com>

* Rescore radial search quantized complete (#3337)

* Added exact search logic after radial.

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

* Adding 2nd rescoring after radial search on quantized index.

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

---------

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

* Update changelog

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>

---------

Signed-off-by: Andrew Klepchick <aklepchi@amazon.com>
Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
Signed-off-by: Dooyong Kim <kdooyong@amazon.com>
Co-authored-by: Andrew Klepchick <aklepchi@amazon.com>
Co-authored-by: Vijayan Balasubramanian <balasvij@amazon.com>

* Add support for binary and byte field support in doc_values (#3340)

Signed-off-by: Navneet Verma <navneev@amazon.com>

* Pin GitHub Actions to commit SHAs (#3339)

Signed-off-by: Divya Madala <divyaasm@amazon.com>
Co-authored-by: Tejas Shah <shatejas@amazon.com>

* Turn off ACORN for MOS (#3346)

Signed-off-by: Andrew Klepchick <aklepchi@amazon.com>

* Add base64 encoded vector indexing support for knn_vector fields (#3350)

Vectors can now be indexed as base64-encoded strings in addition to JSON
arrays. Float vectors use little-endian byte encoding (symmetric with
the doc_values binary output format), while byte/binary vectors use raw
byte encoding. This enables efficient bulk ingestion pipelines that
avoid JSON array serialization overhead.

Signed-off-by: Navneet Verma <navneev@amazon.com>

* Made MemoryOptimizedSearchWarmup skip MemoryOptimizedSearchOldIndicesNotSupportedException. (#3344)

Signed-off-by: Dooyong Kim <kdooyong@amazon.com>
Signed-off-by: Doo Yong Kim <kdooyong@amazon.com>

* Integrated proper ef_search functionality into MOS and Lucene with oversample_factor (#3331)

* Check to see if Lucene's search budget has exhausted when deciding to exact search (#3354)

* Update opensearch-build workflow references from commit SHA to main (#3363)

Signed-off-by: Divya Madala <divyaasm@amazon.com>

* Pinned the commit for tj-actions/changed-files for version v47.0.0 (#3367)

Signed-off-by: Navneet Verma <navneev@amazon.com>

---------

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Signed-off-by: Navneet Verma <navneev@amazon.com>
Signed-off-by: shreyah963 <shreyab963@gmail.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: opensearch-ci-bot <opensearch-infra@amazon.com>
Signed-off-by: Wonjae Lee <wonjae.lee@dremio.com>
Signed-off-by: Wonjae Lee <38933452+leewjae@users.noreply.github.com>
Signed-off-by: Tejas Shah <shatejas@amazon.com>
Signed-off-by: Andrew Klepchick <aklepchi@amazon.com>
Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
Signed-off-by: Dooyong Kim <kdooyong@amazon.com>
Signed-off-by: Divya Madala <divyaasm@amazon.com>
Signed-off-by: Doo Yong Kim <kdooyong@amazon.com>
Co-authored-by: Navneet Verma <navneev@amazon.com>
Co-authored-by: Shreya Bhatta <shreyab963@gmail.com>
Co-authored-by: Tejas Shah <shatejas@amazon.com>
Co-authored-by: Sayali Gaikawad <gaiksaya@amazon.com>
Co-authored-by: opensearch-ci <83309141+opensearch-ci-bot@users.noreply.github.com>
Co-authored-by: Wonjae Lee <38933452+leewjae@users.noreply.github.com>
Co-authored-by: Doo Yong Kim <kdooyong@amazon.com>
Co-authored-by: Andrew Klepchick <aklepchi@amazon.com>
Co-authored-by: Vijayan Balasubramanian <balasvij@amazon.com>
Co-authored-by: Divya Madala <113469545+Divyaasm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancements Increases software capabilities beyond original client specifications

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[FEATURE] Support docvalue_fields for retrieving KNN vectors without _source parsing

3 participants