Fixed issue with POJO reading when binary strings are enabled

Author: chernser

client-v2/src/main/java/com/clickhouse/client/api/data_formats/StringValue.java

@@ -6,9 +6,17 @@
 import java.util.Objects;
 
 /**
- * Holder for ClickHouse {@code String} or {@code FixedString} values that preserves raw bytes
+ * Read-time holder for ClickHouse {@code String} or {@code FixedString} values that preserves raw bytes
  * to avoid lossy decoding and unnecessary allocations.
  * <p>
+ * <b>This is an internal value holder, not a general-purpose type for user code.</b> It is produced only
+ * by the read path when the binary-string feature is enabled (for example {@code GenericRecord.getObject}
+ * or {@code BinaryStreamReader.readValue} without a type hint), so that callers that need exact bytes can
+ * obtain them via {@link #toByteArray()} and callers that need text can decode via {@link #asString()}.
+ * It is <b>not</b> a supported field type for POJO binding: declare POJO fields for String/FixedString
+ * columns as {@link String} or {@code byte[]} instead. Normal application code should generally consume
+ * {@link String} or {@code byte[]} rather than holding onto a {@code StringValue}.
+ * <p>
  * <b>This is a mutable structure and must be used with care.</b> To avoid copying, it does not
  * duplicate the bytes it is given: the constructor wraps the supplied array/buffer instead of
  * copying it, and {@link #toByteArray()} returns a direct reference to the backing array rather

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

@@ -27,6 +27,7 @@
 import java.math.BigInteger;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
+import java.nio.charset.StandardCharsets;
 import java.sql.Timestamp;
 import java.time.Duration;
 import java.time.Instant;
@@ -944,6 +945,48 @@ public static String convertToString(Object value) {
         return java.lang.String.valueOf(value);
     }
 
+    /**
+     * Converts a value read for a top-level {@code String}/{@code FixedString} column into a {@link String}
+     * for POJO binding. With the binary-string feature enabled the reader produces a {@link StringValue}
+     * holder (a read-time value holder, not a POJO field type); otherwise it produces a plain {@link String}.
+     * A {@code null} value (e.g. a SQL {@code NULL}) is passed through. Used from the bytecode generated by
+     * {@link #compilePOJOSetter(Method, ClickHouseColumn)}.
+     *
+     * @param value value returned by {@code BinaryStreamReader.readValue}
+     * @return the decoded string, or {@code null}
+     */
+    public static String stringValueToString(Object value) {
+        if (value == null) {
+            return null;
+        }
+        if (value instanceof StringValue) {
+            return ((StringValue) value).asString();
+        }
+        return (String) value;
+    }
+
+    /**
+     * Converts a value read for a top-level {@code String}/{@code FixedString} column into the raw bytes for
+     * POJO binding. A {@link StringValue} yields its preserved bytes (no lossy decoding); a plain
+     * {@link String} is encoded as UTF-8, matching the historical behaviour. A {@code null} value is passed
+     * through. Used from the bytecode generated by {@link #compilePOJOSetter(Method, ClickHouseColumn)}.
+     *
+     * @param value value returned by {@code BinaryStreamReader.readValue}
+     * @return the raw bytes, or {@code null}
+     */
+    public static byte[] stringValueToByteArray(Object value) {
+        if (value == null) {
+            return null;
+        }
+        if (value instanceof StringValue) {
+            return ((StringValue) value).toByteArray();
+        }
+        if (value instanceof String) {
+            return ((String) value).getBytes(StandardCharsets.UTF_8);
+        }
+        return (byte[]) value;
+    }
+
     /**
      * Writes raw bytes as a ClickHouse {@code FixedString(length)} value. The bytes are written as-is and
      * right-padded with zero bytes when shorter than {@code length}.
@@ -1073,7 +1116,26 @@ public static POJOFieldDeserializer compilePOJOSetter(Method setterMethod, Click
                                 Type.getType(Class.class)),
                         false);
 
-                if (List.class.isAssignableFrom(targetType) && column.getDataType() == ClickHouseDataType.Tuple) {
+                boolean stringLikeColumn = column.getDataType() == ClickHouseDataType.String
+                        || column.getDataType() == ClickHouseDataType.FixedString;
+
+                if (stringLikeColumn && targetType == String.class) {
+                    // With binary-string support enabled readValue yields a StringValue holder; convert it (or a
+                    // plain String when the feature is off) to the String the setter expects. StringValue is a
+                    // read-time holder, not a supported POJO field type.
+                    mv.visitMethodInsn(INVOKESTATIC,
+                            Type.getInternalName(SerializerUtils.class),
+                            "stringValueToString",
+                            Type.getMethodDescriptor(Type.getType(String.class), Type.getType(Object.class)),
+                            false);
+                } else if (stringLikeColumn && targetType == byte[].class) {
+                    // Convert the StringValue (or plain String) to raw bytes for a byte[] field.
+                    mv.visitMethodInsn(INVOKESTATIC,
+                            Type.getInternalName(SerializerUtils.class),
+                            "stringValueToByteArray",
+                            Type.getMethodDescriptor(Type.getType(byte[].class), Type.getType(Object.class)),
+                            false);
+                } else if (List.class.isAssignableFrom(targetType) && column.getDataType() == ClickHouseDataType.Tuple) {
                     mv.visitTypeInsn(CHECKCAST, Type.getInternalName(Object[].class));
                     mv.visitMethodInsn(INVOKESTATIC,
                             Type.getInternalName(Arrays.class),

client-v2/src/test/java/com/clickhouse/client/api/data_formats/StringValueTests.java

@@ -6,6 +6,7 @@
 import com.clickhouse.client.api.data_formats.internal.SerializerUtils;
 import com.clickhouse.client.api.metadata.TableSchema;
 import com.clickhouse.client.api.query.QuerySettings;
+import com.clickhouse.client.api.serde.POJOFieldDeserializer;
 import com.clickhouse.data.ClickHouseColumn;
 import com.clickhouse.data.format.BinaryStreamUtils;
 import org.testng.Assert;
@@ -15,6 +16,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -286,6 +288,83 @@ public void testDefaultBehaviorReturnsString() throws IOException {
         Assert.assertEquals(read, "still a string");
     }
 
+    // ---- POJO binding (queryAll/readToPOJO) over String columns with the feature enabled ----
+
+    /**
+     * Minimal POJO with the only two field representations supported for top-level String/FixedString
+     * columns: {@link String} and {@code byte[]}. {@link StringValue} is a read-time holder and is not a
+     * supported POJO field type.
+     */
+    public static class StringPojo {
+        private String asString;
+        private byte[] asBytes;
+
+        public void setAsString(String asString) { this.asString = asString; }
+        public void setAsBytes(byte[] asBytes) { this.asBytes = asBytes; }
+    }
+
+    private static byte[] stringWire(byte[] value) throws IOException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        BinaryStreamUtils.writeString(baos, value);
+        return baos.toByteArray();
+    }
+
+    private static POJOFieldDeserializer setterFor(String name, ClickHouseColumn column) throws Exception {
+        Method setter = StringPojo.class.getMethod(name,
+                name.equals("setAsString") ? String.class : byte[].class);
+        return SerializerUtils.compilePOJOSetter(setter, column);
+    }
+
+    @Test
+    public void testPojoSetterStringFieldDecodesWhenFeatureEnabled() throws Exception {
+        // Regression: with the feature enabled the reader produces StringValue, but a String setter must
+        // still receive a decoded String (the compiled setter casts the readValue result to String).
+        ClickHouseColumn column = ClickHouseColumn.of("s", "String");
+        byte[] wire = stringWire("hello".getBytes(StandardCharsets.UTF_8));
+
+        StringPojo pojo = new StringPojo();
+        setterFor("setAsString", column).setValue(pojo, reader(wire, true), column);
+        Assert.assertEquals(pojo.asString, "hello");
+    }
+
+    @Test
+    public void testPojoSetterByteArrayFieldReceivesRawBytesWhenFeatureEnabled() throws Exception {
+        // A byte[] setter must receive the raw bytes (preserving non-UTF-8 content) instead of a StringValue.
+        ClickHouseColumn column = ClickHouseColumn.of("s", "String");
+        byte[] binary = {(byte) 0xDE, (byte) 0xAD, (byte) 0x00, (byte) 0xBE, (byte) 0xEF};
+        byte[] wire = stringWire(binary);
+
+        StringPojo pojo = new StringPojo();
+        setterFor("setAsBytes", column).setValue(pojo, reader(wire, true), column);
+        Assert.assertEquals(pojo.asBytes, binary);
+    }
+
+    @Test
+    public void testPojoSetterFixedStringByteArrayFieldWhenFeatureEnabled() throws Exception {
+        // A byte[] setter over a FixedString column must receive the raw bytes, preserving binary content.
+        ClickHouseColumn column = ClickHouseColumn.of("s", "FixedString(3)");
+        byte[] binary = {(byte) 0xAA, (byte) 0xBB, (byte) 0xCC};
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        baos.write(binary); // FixedString(3) is exactly 3 raw bytes on the wire
+
+        StringPojo pojo = new StringPojo();
+        setterFor("setAsBytes", column).setValue(pojo, reader(baos.toByteArray(), true), column);
+        Assert.assertEquals(pojo.asBytes, binary);
+    }
+
+    @Test
+    public void testPojoSetterFixedStringStringFieldWhenFeatureEnabled() throws Exception {
+        // A String setter over a FixedString column must receive the decoded String.
+        ClickHouseColumn column = ClickHouseColumn.of("s", "FixedString(3)");
+        byte[] bytes = "abc".getBytes(StandardCharsets.UTF_8);
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        baos.write(bytes);
+
+        StringPojo pojo = new StringPojo();
+        setterFor("setAsString", column).setValue(pojo, reader(baos.toByteArray(), true), column);
+        Assert.assertEquals(pojo.asString, "abc");
+    }
+
     // ---- Writing binary String values ----
 
     @Test

docs/features.md

@@ -16,7 +16,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t
 - Parameterized SQL: Accepts named query parameters and can send them through supported HTTP request encodings.
 - Result materialization helpers: Provides streaming `Records`, generic row access, and convenience APIs that materialize all rows into generic records or typed POJOs.
 - Binary format readers: Reads ClickHouse binary result formats including `Native`, `RowBinary`, `RowBinaryWithNames`, and `RowBinaryWithNamesAndTypes`.
-- Binary string support: Opt-in through the `binary_string_support` property (or `Client.Builder#binaryStringSupport(boolean)`), disabled by default. When enabled, top-level `String` and `FixedString` columns are read as `StringValue`, which preserves the raw bytes (`toByteArray()`/`asByteBuffer()`) and lazily decodes a `String` (`asString()`), instead of eagerly decoding to a `String`. Strings nested inside containers (`Array`, `Map`, `Tuple`, `Nested`, `Variant`) are still read as `String`.
+- Binary string support: Opt-in through the `binary_string_support` property (or `Client.Builder#binaryStringSupport(boolean)`), disabled by default. When enabled, untyped reads (e.g. `GenericRecord.getObject(...)`/`BinaryStreamReader.readValue(...)`) of top-level `String` and `FixedString` columns return a `StringValue`, which preserves the raw bytes (`toByteArray()`/`asByteBuffer()`) and lazily decodes a `String` (`asString()`), instead of eagerly decoding to a `String`. `StringValue` is a read-time holder, not a supported POJO field type: typed `queryAll(...)`/`readToPOJO` binding still maps these columns to `String` (decoded) or `byte[]` (raw bytes) according to the POJO field type. Strings nested inside containers (`Array`, `Map`, `Tuple`, `Nested`, `Variant`) are still read as `String`.
 - JSONEachRow text reader: Can stream `JSONEachRow` responses through a caller-supplied `JsonParser`, with Jackson and Gson parser factory implementations available as optional classpath dependencies, and infers a best-effort schema from the first row.
 - Data type conversion: Maps ClickHouse types to Java values for binary reads, POJO binding, and SQL parameter formatting, including date/time handling.
 - Geometry type support: For ClickHouse `25.11+`, where `Geometry` changed from a string alias to `Variant(Point, Ring, LineString, MultiLineString, Polygon, MultiPolygon)`, the client reads and writes `Geometry` values through generic records, binary readers, POJO binding, and SQL parameter formatting, using Java array dimensionality to represent the geometry shape.

jdbc-v2/src/main/java/com/clickhouse/jdbc/ResultSetImpl.java

@@ -287,6 +287,7 @@ public InputStream getUnicodeStream(int columnIndex) throws SQLException {
 
     @Override
     public InputStream getBinaryStream(int columnIndex) throws SQLException {
+        checkClosed();
         try {
             if (reader.hasValue(columnIndex)) {
                 byte[] bytes = reader.getByteArray(columnIndex);