From c767150178fc98da5e5e8521a3300f86329f05cf Mon Sep 17 00:00:00 2001 From: Brian Sam-Bodden Date: Wed, 22 Oct 2025 16:39:41 -0700 Subject: [PATCH] feat: implement skip_decode parameter for binary fields (port of Python PR #389) Add skipDecodeFields parameter to query builders to control binary field decoding: - FilterQuery, VectorQuery, VectorRangeQuery, TextQuery - Accepts List or varargs of field names - Returns immutable copy via getSkipDecodeFields() Implementation details: - Field names that should not be decoded from binary format - Useful for binary data like embeddings and image data - Validates field names cannot be null - Maintains backward compatibility (defaults to empty list) Test coverage: - 10 unit tests in SkipDecodeFieldsTest - Tests for all query types - Validation tests for null handling - Varargs convenience method tests Python reference: - skip_decode parameter in return_fields() method - Prevents automatic decoding of specified binary fields - Allows retrieval of raw bytes for embeddings/images Refs: https://github.com/redis/redis-vl-python/pull/389 Fixes: #252 --- .../java/com/redis/vl/query/FilterQuery.java | 65 +++++++ .../java/com/redis/vl/query/TextQuery.java | 58 ++++++ .../java/com/redis/vl/query/VectorQuery.java | 64 ++++++- .../com/redis/vl/query/VectorRangeQuery.java | 56 ++++++ .../redis/vl/query/SkipDecodeFieldsTest.java | 171 ++++++++++++++++++ 5 files changed, 412 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/com/redis/vl/query/SkipDecodeFieldsTest.java diff --git a/core/src/main/java/com/redis/vl/query/FilterQuery.java b/core/src/main/java/com/redis/vl/query/FilterQuery.java index 1d66d4d..d1455fb 100644 --- a/core/src/main/java/com/redis/vl/query/FilterQuery.java +++ b/core/src/main/java/com/redis/vl/query/FilterQuery.java @@ -44,6 +44,8 @@ private FilterQuery(FilterQueryBuilder builder) { this.sortAscending = builder.sortAscending; this.inOrder = builder.inOrder; this.params = builder.params != null ? Map.copyOf(builder.params) : Map.of(); + this.skipDecodeFields = + builder.skipDecodeFields != null ? List.copyOf(builder.skipDecodeFields) : List.of(); } /** @@ -92,6 +94,11 @@ public static FilterQueryBuilder builder() { /** Additional parameters for the query. Python: params (Optional[Dict[str, Any]]) */ private final Map params; + /** + * Fields that should not be decoded from binary format. Python: skip_decode parameter (PR #389) + */ + private final List skipDecodeFields; + /** * Build Redis Query object from FilterQuery. * @@ -194,6 +201,17 @@ public Map getParams() { return params; } + /** + * Get the list of fields that should not be decoded from binary format. + * + *

Python equivalent: _skip_decode_fields attribute (PR #389) + * + * @return List of field names to skip decoding + */ + public List getSkipDecodeFields() { + return skipDecodeFields; + } + /** Builder for FilterQuery with defensive copying. */ public static class FilterQueryBuilder { private Filter filterExpression; @@ -204,6 +222,7 @@ public static class FilterQueryBuilder { private boolean sortAscending = true; // Default to ascending private boolean inOrder = false; private Map params = Map.of(); + private List skipDecodeFields = List.of(); FilterQueryBuilder() {} @@ -359,6 +378,52 @@ public FilterQueryBuilder params(Map params) { return this; } + /** + * Set fields that should not be decoded from binary format. + * + *

Python equivalent: skip_decode parameter in return_fields() (PR #389) + * + * @param skipDecodeFields List of field names + * @return this builder + * @throws IllegalArgumentException if list contains null values + */ + public FilterQueryBuilder skipDecodeFields(List skipDecodeFields) { + if (skipDecodeFields == null) { + this.skipDecodeFields = List.of(); + return this; + } + // Validate no null values + for (String field : skipDecodeFields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.copyOf(skipDecodeFields); + return this; + } + + /** + * Set fields that should not be decoded from binary format (varargs). + * + * @param fields Field names + * @return this builder + * @throws IllegalArgumentException if any field is null + */ + public FilterQueryBuilder skipDecodeFields(String... fields) { + if (fields == null || fields.length == 0) { + this.skipDecodeFields = List.of(); + return this; + } + // Validate no null values + for (String field : fields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.of(fields); + return this; + } + /** * Build the FilterQuery instance. * diff --git a/core/src/main/java/com/redis/vl/query/TextQuery.java b/core/src/main/java/com/redis/vl/query/TextQuery.java index 297a3bb..0b33b49 100644 --- a/core/src/main/java/com/redis/vl/query/TextQuery.java +++ b/core/src/main/java/com/redis/vl/query/TextQuery.java @@ -52,6 +52,9 @@ public class TextQuery { /** Whether to sort in descending order */ private final boolean sortDescending; + /** Fields that should not be decoded from binary format */ + private final List skipDecodeFields; + private TextQuery(Builder builder) { this.text = builder.text; this.scorer = builder.scorer; @@ -60,6 +63,8 @@ private TextQuery(Builder builder) { this.fieldWeights = new HashMap<>(builder.fieldWeights); this.sortBy = builder.sortBy; this.sortDescending = builder.sortDescending; + this.skipDecodeFields = + builder.skipDecodeFields != null ? List.copyOf(builder.skipDecodeFields) : List.of(); } /** @@ -81,6 +86,15 @@ public Map getFieldWeights() { return new HashMap<>(fieldWeights); } + /** + * Get the list of fields that should not be decoded from binary format. + * + * @return List of field names to skip decoding + */ + public List getSkipDecodeFields() { + return skipDecodeFields; + } + /** * Build the Redis query string for text search with weighted fields. * @@ -174,6 +188,7 @@ public static class Builder { private Map fieldWeights = new HashMap<>(); private String sortBy; private boolean sortDescending = false; + private List skipDecodeFields = List.of(); /** * Set the text to search for. @@ -327,6 +342,49 @@ public Builder sortDescending(boolean descending) { return this; } + /** + * Set fields that should not be decoded from binary format. + * + * @param skipDecodeFields List of field names + * @return This builder + * @throws IllegalArgumentException if list contains null values + */ + public Builder skipDecodeFields(List skipDecodeFields) { + if (skipDecodeFields == null) { + this.skipDecodeFields = List.of(); + return this; + } + // Validate no null values + for (String field : skipDecodeFields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.copyOf(skipDecodeFields); + return this; + } + + /** + * Set fields that should not be decoded from binary format (varargs). + * + * @param fields Field names + * @return This builder + * @throws IllegalArgumentException if any field is null + */ + public Builder skipDecodeFields(String... fields) { + if (fields == null || fields.length == 0) { + this.skipDecodeFields = List.of(); + return this; + } + for (String field : fields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.of(fields); + return this; + } + /** * Build the TextQuery instance. * diff --git a/core/src/main/java/com/redis/vl/query/VectorQuery.java b/core/src/main/java/com/redis/vl/query/VectorQuery.java index 12c636c..1ee3de4 100644 --- a/core/src/main/java/com/redis/vl/query/VectorQuery.java +++ b/core/src/main/java/com/redis/vl/query/VectorQuery.java @@ -61,6 +61,9 @@ public class VectorQuery { /** Whether to require terms in field to have same order as in query */ private boolean inOrder; + /** Fields that should not be decoded from binary format */ + private List skipDecodeFields; + /** Private constructor */ private VectorQuery( String field, @@ -77,7 +80,8 @@ private VectorQuery( boolean normalizeVectorDistance, String sortBy, boolean sortDescending, - boolean inOrder) { + boolean inOrder, + List skipDecodeFields) { this.field = field; this.vector = vector != null ? vector.clone() : null; // Defensive copy this.numResults = numResults; @@ -93,6 +97,8 @@ private VectorQuery( this.sortBy = sortBy; this.sortDescending = sortDescending; this.inOrder = inOrder; + this.skipDecodeFields = + skipDecodeFields != null ? new ArrayList<>(skipDecodeFields) : new ArrayList<>(); } /** @@ -505,6 +511,15 @@ public boolean isInOrder() { return inOrder; } + /** + * Get the list of fields that should not be decoded from binary format. + * + * @return List of field names to skip decoding + */ + public List getSkipDecodeFields() { + return skipDecodeFields != null ? new ArrayList<>(skipDecodeFields) : new ArrayList<>(); + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); @@ -550,6 +565,7 @@ public Builder() { private String sortBy; private boolean sortDescending = false; private boolean inOrder = false; + private List skipDecodeFields = new ArrayList<>(); private static float[] toFloatArray(double[] doubles) { if (doubles == null) return null; @@ -904,6 +920,49 @@ public Builder inOrder(boolean inOrder) { return this; } + /** + * Set fields that should not be decoded from binary format. + * + * @param skipDecodeFields List of field names + * @return This builder + * @throws IllegalArgumentException if list contains null values + */ + public Builder skipDecodeFields(List skipDecodeFields) { + if (skipDecodeFields == null) { + this.skipDecodeFields = new ArrayList<>(); + return this; + } + // Validate no null values + for (String field : skipDecodeFields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = new ArrayList<>(skipDecodeFields); + return this; + } + + /** + * Set fields that should not be decoded from binary format (varargs). + * + * @param fields Field names + * @return This builder + * @throws IllegalArgumentException if any field is null + */ + public Builder skipDecodeFields(String... fields) { + if (fields == null || fields.length == 0) { + this.skipDecodeFields = new ArrayList<>(); + return this; + } + for (String field : fields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = new ArrayList<>(Arrays.asList(fields)); + return this; + } + /** * Build the VectorQuery * @@ -940,7 +999,8 @@ public VectorQuery build() { normalizeVectorDistance, sortBy, sortDescending, - inOrder); + inOrder, + skipDecodeFields); } } } diff --git a/core/src/main/java/com/redis/vl/query/VectorRangeQuery.java b/core/src/main/java/com/redis/vl/query/VectorRangeQuery.java index 03028a3..ab847e8 100644 --- a/core/src/main/java/com/redis/vl/query/VectorRangeQuery.java +++ b/core/src/main/java/com/redis/vl/query/VectorRangeQuery.java @@ -22,6 +22,7 @@ public final class VectorRangeQuery { private final String sortBy; private final boolean sortDescending; private final boolean inOrder; + private final List skipDecodeFields; private VectorRangeQuery(Builder builder) { // Validate before modifying state to avoid partial initialization @@ -44,6 +45,8 @@ private VectorRangeQuery(Builder builder) { this.sortBy = builder.sortBy; this.sortDescending = builder.sortDescending; this.inOrder = builder.inOrder; + this.skipDecodeFields = + builder.skipDecodeFields != null ? List.copyOf(builder.skipDecodeFields) : List.of(); } /** @@ -188,6 +191,15 @@ public boolean isInOrder() { return inOrder; } + /** + * Get the list of fields that should not be decoded from binary format. + * + * @return List of field names to skip decoding + */ + public List getSkipDecodeFields() { + return skipDecodeFields; + } + /** * Build the query string for Redis range query * @@ -242,6 +254,7 @@ public static class Builder { private String sortBy; private boolean sortDescending = false; private boolean inOrder = false; + private List skipDecodeFields = List.of(); /** Package-private constructor used by builder() method. */ Builder() {} @@ -454,6 +467,49 @@ public Builder inOrder(boolean inOrder) { return this; } + /** + * Set fields that should not be decoded from binary format. + * + * @param skipDecodeFields List of field names + * @return This builder + * @throws IllegalArgumentException if list contains null values + */ + public Builder skipDecodeFields(List skipDecodeFields) { + if (skipDecodeFields == null) { + this.skipDecodeFields = List.of(); + return this; + } + // Validate no null values + for (String field : skipDecodeFields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.copyOf(skipDecodeFields); + return this; + } + + /** + * Set fields that should not be decoded from binary format (varargs). + * + * @param fields Field names + * @return This builder + * @throws IllegalArgumentException if any field is null + */ + public Builder skipDecodeFields(String... fields) { + if (fields == null || fields.length == 0) { + this.skipDecodeFields = List.of(); + return this; + } + for (String field : fields) { + if (field == null) { + throw new IllegalArgumentException("skipDecodeFields cannot contain null values"); + } + } + this.skipDecodeFields = List.of(fields); + return this; + } + /** * Build the VectorRangeQuery instance. * diff --git a/core/src/test/java/com/redis/vl/query/SkipDecodeFieldsTest.java b/core/src/test/java/com/redis/vl/query/SkipDecodeFieldsTest.java new file mode 100644 index 0000000..f48e5ca --- /dev/null +++ b/core/src/test/java/com/redis/vl/query/SkipDecodeFieldsTest.java @@ -0,0 +1,171 @@ +package com.redis.vl.query; + +import static org.assertj.core.api.Assertions.*; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for skip_decode parameter in query classes (issue #252). + * + *

Ported from Python: tests/unit/test_skip_decode_fields.py + * + *

Python reference: PR #389 - skip_decode parameter for return_fields method + */ +@DisplayName("Skip Decode Fields Tests") +class SkipDecodeFieldsTest { + + @Test + @DisplayName("FilterQuery: Should accept skip_decode for single field") + void testFilterQuerySkipDecodeSingleField() { + // Python: query.return_fields("title", "year", "embedding", skip_decode=["embedding"]) + FilterQuery query = + FilterQuery.builder() + .returnFields(List.of("title", "year", "embedding")) + .skipDecodeFields(List.of("embedding")) + .build(); + + // Check that fields are set correctly + assertThat(query.getReturnFields()).contains("title", "year", "embedding"); + + // Check that skip_decode settings are tracked + assertThat(query.getSkipDecodeFields()).contains("embedding"); + } + + @Test + @DisplayName("FilterQuery: Should accept skip_decode for multiple fields") + void testFilterQuerySkipDecodeMultipleFields() { + // Python: skip_decode=["embedding", "image_data"] + FilterQuery query = + FilterQuery.builder() + .returnFields(List.of("title", "year", "embedding", "image_data")) + .skipDecodeFields(List.of("embedding", "image_data")) + .build(); + + assertThat(query.getReturnFields()).hasSize(4); + assertThat(query.getSkipDecodeFields()).contains("embedding", "image_data").hasSize(2); + } + + @Test + @DisplayName("VectorQuery: Should accept skip_decode for single field") + void testVectorQuerySkipDecodeSingleField() { + // Python: query.return_fields("id", "vector_field", "metadata", skip_decode=["vector_field"]) + VectorQuery query = + VectorQuery.builder() + .field("vector_field") + .vector(new float[] {0.1f, 0.2f, 0.3f}) + .returnFields("id", "vector_field", "metadata") + .skipDecodeFields(List.of("vector_field")) + .build(); + + assertThat(query.getReturnFields()).contains("id", "vector_field", "metadata"); + assertThat(query.getSkipDecodeFields()).contains("vector_field"); + } + + @Test + @DisplayName("VectorRangeQuery: Should accept skip_decode") + void testVectorRangeQuerySkipDecode() { + // Python: query.return_fields("doc_id", "text", "embedding", skip_decode=["embedding"]) + VectorRangeQuery query = + VectorRangeQuery.builder() + .field("embedding") + .vector(new float[] {0.1f, 0.2f, 0.3f}) + .returnFields("doc_id", "text", "embedding") + .skipDecodeFields(List.of("embedding")) + .build(); + + assertThat(query.getReturnFields()).contains("doc_id", "text", "embedding"); + assertThat(query.getSkipDecodeFields()).contains("embedding"); + } + + @Test + @DisplayName("Should handle empty skip_decode list") + void testSkipDecodeEmptyList() { + // Python: skip_decode=[] + FilterQuery query = + FilterQuery.builder() + .returnFields(List.of("field1", "field2", "field3")) + .skipDecodeFields(Collections.emptyList()) + .build(); + + assertThat(query.getReturnFields()).hasSize(3); + assertThat(query.getSkipDecodeFields()).isEmpty(); + } + + @Test + @DisplayName("Should default to empty skip_decode (backwards compatible)") + void testSkipDecodeNoneDefault() { + // Python: No skip_decode parameter (backwards compatibility) + FilterQuery query = FilterQuery.builder().returnFields(List.of("field1", "field2")).build(); + + assertThat(query.getReturnFields()).hasSize(2); + // Should have empty skip_decode by default + assertThat(query.getSkipDecodeFields()).isEmpty(); + } + + @Test + @DisplayName("Should accept skip_decode with varargs") + void testSkipDecodeVarargs() { + // Java convenience: accept varargs in addition to List + FilterQuery query = + FilterQuery.builder() + .returnFields(List.of("field1", "field2", "field3")) + .skipDecodeFields("field1", "field3") + .build(); + + assertThat(query.getSkipDecodeFields()).contains("field1", "field3").hasSize(2); + } + + @Test + @DisplayName("TextQuery: Should accept skip_decode") + void testTextQuerySkipDecode() { + TextQuery query = + TextQuery.builder() + .text("search query") + .textField("description") + .skipDecodeFields(List.of("embedding", "image_data")) + .build(); + + assertThat(query.getSkipDecodeFields()).contains("embedding", "image_data"); + } + + @Test + @DisplayName("Should validate skip_decode field names are not null") + void testSkipDecodeValidation() { + // Null list should be handled gracefully (empty) + FilterQuery query1 = + FilterQuery.builder() + .returnFields(List.of("field1")) + .skipDecodeFields((List) null) + .build(); + + assertThat(query1.getSkipDecodeFields()).isEmpty(); + + // Null values in list should be rejected + assertThatThrownBy( + () -> + FilterQuery.builder() + .returnFields(List.of("field1")) + .skipDecodeFields(Arrays.asList("field1", null)) + .build()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("cannot contain null"); + } + + @Test + @DisplayName("Should allow fields in skip_decode that are not in return_fields") + void testSkipDecodeFieldNotInReturnFields() { + // Python: Allows this - skip_decode field might not be in return_fields + FilterQuery query = + FilterQuery.builder() + .returnFields(List.of("field1", "field2")) + .skipDecodeFields(List.of("field3")) + .build(); + + assertThat(query.getReturnFields()).hasSize(2); + assertThat(query.getSkipDecodeFields()).contains("field3"); + } +}