Add base64 binary encoding as default format for knn_vector docvalue_fields

[navneet1v]

Description

Add base64 binary encoding as default format for knn_vector docvalue_fields

Summary

Adds binary (base64) encoding as the default output format for docvalue_fields on knn_vector fields. When no format is specified in the request, vectors are now returned as
base64-encoded little-endian float byte strings instead of JSON numeric arrays.

This builds on #3321 which added docvalue_fields support with array format only. The binary default provides ~2x throughput improvement over the array format for JSON
transport (the most common case), while maintaining the same performance for binary transports (CBOR/SMILE) where both formats are already efficient.

Benchmark highlights (768D Cohere vectors, k=1000)

Quick Setup to validate: 768-dimensional Cohere vectors, k=1000, 1000 queries, single node on my macbook. JVM: 4gb.

Scenario (JSON transport)	p50 latency	QPS	Speedup vs _source
_source (baseline)	63.5 ms	15.3	—
docvalue_fields (array)	44.5 ms	20.5	1.34x
docvalue_fields (binary, default)	21.7 ms	40.9	2.68x

Binary format also reduces JSON response payload by ~30-40% for high-dimensional vectors since base64 encodes 768 floats in ~4 KB vs ~6 KB for a JSON numeric array.

Format selection

Users can explicitly choose the format:

"docvalue_fields": [{"field": "my_vector", "format": "binary"}]   // base64 string (default)
"docvalue_fields": [{"field": "my_vector", "format": "array"}]    // JSON numeric array
"docvalue_fields": ["my_vector"]                                   // base64 string (default)

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]

PR Reviewer Guide 🔍

(Review updated until commit 2d2b804)

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 encodeToBinary method allocates a ByteBuffer but does not flip it after writing. While buffer.array() returns the backing array directly, this works only because the buffer is fully written. If the buffer were used for reading or partial writes, missing flip() would cause incorrect behavior. This is not a bug in the current code path, but it's fragile and could break if the method is refactored to read from the buffer instead of using array(). public static String encodeToBinary(final float[] vector) { final ByteBuffer buffer = ByteBuffer.allocate(vector.length * Float.BYTES).order(ByteOrder.LITTLE_ENDIAN); buffer.asFloatBuffer().put(vector); // Bulk operation optimized by the JVM final byte[] bytes = buffer.array(); return BASE64_ENCODER.encodeToString(bytes); Possible Issue The comment on line 168 states "We don't need a conditional clone here since encodeToBinary will convert the array to string." However, if encodeToBinary is later changed to return the array directly or if the binary path is modified, this could lead to vector array reuse bugs where the same array reference is shared across multiple documents. The current implementation is safe, but the reasoning is tied to implementation details that may change. // We don't need a conditional clone here since encodeToBinary will convert the array to string. if (isBinary) { return KNNVectorDocValueFormat.encodeToBinary((float[]) vectorValues.getVector()); } // We need a conditional clone since vector returned from here will be added in a map, so we do the clone // since vectorValues keep a single copy of vector array for all docs. return vectorValues.conditionalCloneVector();

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 2d2b804

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add explicit null check before instanceof The null check for format should occur before the instanceof check. Currently, if format is null, the instanceof will return false and throw an IllegalArgumentException with a confusing message showing "null" as the format type. src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [121-125] +if (format == null) { + throw new IllegalArgumentException( + "DocValueFormat cannot be null for knn_vector field '" + fieldName + "'" + ); +} if (!(format instanceof KNNVectorDocValueFormat knnFormat)) { throw new IllegalArgumentException( "Unsupported DocValueFormat [" + format + "] for knn_vector field '" + fieldName + "'. Expected KNNVectorDocValueFormat." ); } Suggestion importance[1-10]: 5 __ Why: The suggestion improves error messaging by distinguishing between null and wrong type. However, the test testGetLeafValueFetcher_nullFormat_throwsIllegalArgument at line 254 expects the current error message for null, so this would require test updates. The improvement is moderate since instanceof already handles null safely.	Low

---

Previous suggestions

Suggestions up to commit 95f3738

Category	Suggestion	Impact
General	Ensure buffer position consistency The asFloatBuffer() creates a view that shares the same backing array but has independent position tracking. After put(vector), the main buffer's position remains at 0, so buffer.array() returns the correct bytes. However, for clarity and to avoid potential issues if buffer operations are added later, explicitly set the buffer's limit or use the FloatBuffer's position. src/main/java/org/opensearch/knn/index/KNNVectorDocValueFormat.java [62-67] public static String encodeToBinary(@NonNull final float[] vector) { final ByteBuffer buffer = ByteBuffer.allocate(vector.length * Float.BYTES).order(ByteOrder.LITTLE_ENDIAN); buffer.asFloatBuffer().put(vector); + buffer.position(buffer.capacity()); // Explicitly advance position to match written data final byte[] bytes = buffer.array(); return BASE64_ENCODER.encodeToString(bytes); } Suggestion importance[1-10]: 2 __ Why: The suggestion to set buffer.position(buffer.capacity()) is unnecessary because buffer.array() returns the entire backing array regardless of position. The current code works correctly, and the suggested change adds no functional benefit. While it might marginally improve code clarity for future modifications, the impact is minimal.	Low

Suggestions up to commit d7fbc2f

Category	Suggestion	Impact
Possible issue	Add null check for vector parameter Add null check for the input vector parameter to prevent NullPointerException. The method should validate the input before attempting to allocate a buffer or perform encoding operations. src/main/java/org/opensearch/knn/index/KNNVectorDocValueFormat.java [61-66] public static String encodeToBinary(final float[] vector) { + if (vector == null) { + throw new IllegalArgumentException("Vector cannot be null"); + } final ByteBuffer buffer = ByteBuffer.allocate(vector.length * Float.BYTES).order(ByteOrder.LITTLE_ENDIAN); buffer.asFloatBuffer().put(vector); // Bulk operation optimized by the JVM final byte[] bytes = buffer.array(); return BASE64_ENCODER.encodeToString(bytes); } Suggestion importance[1-10]: 7 __ Why: Adding a null check for the vector parameter in encodeToBinary is a valid defensive programming practice that prevents NullPointerException. However, the method is called internally from controlled contexts where null vectors are unlikely, limiting the practical impact.	Medium
	Validate vector before binary encoding Add validation to ensure the vector returned by vectorValues.getVector() is not null before casting and encoding. This prevents potential NullPointerException when encoding binary format. src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [163-165] if (binary) { - return KNNVectorDocValueFormat.encodeToBinary((float[]) vectorValues.getVector()); + Object vector = vectorValues.getVector(); + if (vector == null) { + throw new IllegalStateException("Vector value is null for document"); + } + return KNNVectorDocValueFormat.encodeToBinary((float[]) vector); } Suggestion importance[1-10]: 6 __ Why: Adding null validation before casting and encoding the vector is a reasonable safety measure. However, vectorValues.getVector() is expected to return valid vectors in normal operation, and the null check in encodeToBinary (from suggestion 1) would also catch this case, reducing the necessity of this check.	Low
General	Add null safety for format parameter Add null check for the format parameter before performing instanceof check and casting. If format is null, the instanceof check will return false, but the code should explicitly handle this case to avoid potential issues. src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [140] -final boolean binary = format instanceof KNNVectorDocValueFormat && ((KNNVectorDocValueFormat) format).isBinary(); +final boolean binary = format != null && format instanceof KNNVectorDocValueFormat && ((KNNVectorDocValueFormat) format).isBinary(); Suggestion importance[1-10]: 3 __ Why: While adding explicit null check improves code clarity, the instanceof operator already handles null safely (returns false for null). The suggestion is technically correct but offers minimal practical improvement since the existing code already handles null correctly.	Low

Suggestions up to commit d7fbc2f

Category	Suggestion	Impact
Possible issue	Add null check for vector parameter Add null check for the vector parameter to prevent NullPointerException. Without this check, passing a null vector will cause the method to fail when accessing vector.length. src/main/java/org/opensearch/knn/index/KNNVectorDocValueFormat.java [61-66] public static String encodeToBinary(final float[] vector) { + if (vector == null) { + throw new IllegalArgumentException("Vector cannot be null"); + } final ByteBuffer buffer = ByteBuffer.allocate(vector.length * Float.BYTES).order(ByteOrder.LITTLE_ENDIAN); buffer.asFloatBuffer().put(vector); // Bulk operation optimized by the JVM final byte[] bytes = buffer.array(); return BASE64_ENCODER.encodeToString(bytes); } Suggestion importance[1-10]: 7 __ Why: Adding a null check prevents NullPointerException when vector is null. However, this is a defensive programming practice rather than a critical bug fix, as the calling code should ensure valid input.	Medium
	Add type safety before casting vector Add type safety check before casting to float[]. If vectorValues.getVector() returns a non-float array type, the cast will fail with ClassCastException at runtime. src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [163-165] if (binary) { - return KNNVectorDocValueFormat.encodeToBinary((float[]) vectorValues.getVector()); + Object vector = vectorValues.getVector(); + if (!(vector instanceof float[])) { + throw new IllegalStateException("Expected float[] vector but got: " + vector.getClass().getName()); + } + return KNNVectorDocValueFormat.encodeToBinary((float[]) vector); } Suggestion importance[1-10]: 6 __ Why: Adding a type check before casting prevents potential ClassCastException. However, the method already throws UnsupportedOperationException for non-FLOAT types (line 116-117), making this additional check somewhat redundant but still useful for defensive programming.	Low
General	Add null safety for format parameter Add null check for the format parameter before performing instanceof check. Although instanceof handles null safely, explicitly checking prevents potential issues if the format is used elsewhere in the method without null safety. src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java [140] -final boolean binary = format instanceof KNNVectorDocValueFormat && ((KNNVectorDocValueFormat) format).isBinary(); +final boolean binary = format != null && format instanceof KNNVectorDocValueFormat && ((KNNVectorDocValueFormat) format).isBinary(); Suggestion importance[1-10]: 3 __ Why: While adding explicit null check improves code clarity, instanceof already handles null safely (returns false), making this a minor defensive improvement rather than a bug fix.	Low

[github-actions]

Persistent review updated to latest commit d7fbc2f

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.47%. Comparing base (67033ce) to head (2d2b804).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3324      +/-   ##
============================================
- Coverage     83.53%   83.47%   -0.07%     
- Complexity     4287     4296       +9     
============================================
  Files           450      451       +1     
  Lines         15552    15584      +32     
  Branches       2015     2018       +3     
============================================
+ Hits          12991    13008      +17     
- Misses         1770     1782      +12     
- Partials        791      794       +3     

☔ 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.

[github-actions]

Persistent review updated to latest commit 95f3738

[github-actions]

Persistent review updated to latest commit eced12b

[github-actions]

Persistent review updated to latest commit 1bb11f9

[github-actions]

Persistent review updated to latest commit a6a9b04

[github-actions]

Persistent review updated to latest commit 7b4daf7

[github-actions]

Persistent review updated to latest commit 5d92e71

[github-actions]

Persistent review updated to latest commit 2d2b804

[Vikasht34]

Changes Looks Good , I don't have any concern on Changes or Code.