Merge pull request #2911 from ClickHouse/07/06/26/binary_string_support

[v0.10.0-rc1] Binary String Support

Author: chernser

CHANGELOG.md

@@ -46,6 +46,8 @@
   previously set in milliseconds but mistakenly retrieved and used in seconds in some places. Now it correctly uses
   milliseconds consistently. (https://github.com/ClickHouse/clickhouse-java/issues/2358)
 
+- **[client-v2]** The public `ClickHouseBinaryFormatWriter` interface gained two methods, `setString(String, byte[])` and `setString(int, byte[])`, for writing raw `String`/`FixedString` bytes. Code that only *uses* the interface is unaffected, but any third party that *implements* `ClickHouseBinaryFormatWriter` directly is source- and binary-incompatible until it adds these methods (recompiling against the new version is required; otherwise an `AbstractMethodError` can occur at runtime).
+
 - **[client-v2]** HTTP `503 Service Unavailable` responses are now surfaced as a connection-style failure (
   `java.net.ConnectException`) and are retried by default. Previously a `503` was treated as a server error (
   `ServerException`) and fell under the `ServerRetryable` fault cause. It has been moved to the `ConnectTimeout` fault
@@ -106,12 +108,25 @@
   to be returned in their native form regardless of the user-supplied map. Existing maps keyed only by JDBC `SQLType`
   names continue to work unchanged. (https://github.com/ClickHouse/clickhouse-java/pull/2865)
 
-  - **[jdbc-v2]** Added support of custom mapping for JDBC types. Mainly used in cases when big integers should be 
+- **[jdbc-v2]** Added support of custom mapping for JDBC types. Mainly used in cases when big integers should be 
   presented as string. Use `DriverProperties.JDBC_TYPE_MAPPINGS` (`jdbc_type_mappings`) and set needed type mapping 
   as `key=value[,]` list (For example, `Int32=Long,UInt64=String`). Deprecation notice: V1 property `typeMappings` is 
   supported but will be removed. Please migrate to the new property. 
   (https://github.com/ClickHouse/clickhouse-java/issues/2858)
 
+- **[client-v2, jdbc-v2]** Added opt-in binary string support through the `binary_string_support` configuration property
+  (or `Client.Builder#binaryStringSupport(boolean)`), disabled by default. The setting is resolved per operation from 
+  the merged client and query settings, so it can be overridden for a single request via the `binary_string_support` 
+  operation option (e.g. `QuerySettings#setOption(ClientConfigProperties.BINARY_STRING_SUPPORT.getKey(), true)`) 
+  independently of the client-level default. When enabled, top-level `String` and `FixedString` columns are read 
+  into a `StringValue` that preserves the raw bytes instead of decoding them into a `String`, allowing non-UTF-8/binary 
+  content to round-trip byte-for-byte. `StringValue` exposes the bytes via `toByteArray()`/`asByteBuffer()` and 
+  lazily decodes a `String` via `asString()` (UTF-8 by default, or a caller-supplied `Charset`). Values nested inside 
+  containers (`Array`, `Map`, `Tuple`, `Nested`, `Variant`) continue to be read as `String`, since those types are not 
+  expected to carry large/binary strings. On the JDBC side, `ResultSet#getBinaryStream(int)` and 
+  `ResultSet#getBinaryStream(String)` are now implemented (previously unsupported) and, together with `getBytes(...)`, 
+  return the raw column bytes.
+  
 - **[client-v2]** Added `Client#cancelTransportRequest(String queryId)` to cancel an in-flight request that has not yet
   received a response from the server, identified by the query id supplied in the operation settings. This aborts the
   request on the client side (cancels the underlying IO operation) but does **not** issue a `KILL QUERY` on the server,
@@ -159,6 +174,11 @@ of `NULL` was not set and read. (https://github.com/ClickHouse/clickhouse-java/i
 
 - **[jdbc-v2, client-v2]** Fixed writing nullable marker for nested `Tuple` and `Map values. (https://github.com/ClickHouse/clickhouse-java/issues/2721)
 
+- **[jdbc-v2]** Fixed `ResultSet.getObject` leaking the internal `StringValue` holder for `String`/`FixedString`
+  columns when `binary_string_support` is enabled. `getObject(column, byte[].class)` now returns the exact raw bytes,
+  and `getObject(column, Object.class)` and the no-type `getObject(column)` overloads now return a decoded `String`
+  instead of the internal holder.
+
 ## 0.9.8
 
 ### Improvements 

client-v2/src/main/java/com/clickhouse/client/api/Client.java

@@ -3,6 +3,7 @@
 import com.clickhouse.client.api.command.CommandResponse;
 import com.clickhouse.client.api.command.CommandSettings;
 import com.clickhouse.client.api.data_formats.ClickHouseBinaryFormatReader;
+import com.clickhouse.client.api.data_formats.ClickHouseFormatReader;
 import com.clickhouse.client.api.data_formats.NativeFormatReader;
 import com.clickhouse.client.api.data_formats.RowBinaryFormatReader;
 import com.clickhouse.client.api.data_formats.RowBinaryWithNamesAndTypesFormatReader;
@@ -1123,6 +1124,20 @@ public Builder typeHintMapping(Map<ClickHouseDataType, Class<?>> typeHintMapping
             return this;
         }
 
+        /**
+         * Enables reading {@code String} and {@code FixedString} columns into an intermediate {@code byte[]}
+         * (a new array each time) instead of decoding them into a {@link String}. This improves working with
+         * large strings and allows {@link ClickHouseFormatReader#getByteArray} to be used more effectively. Can also be configured
+         * per operation.
+         *
+         * @param enable - if the feature is enabled
+         * @return this builder instance
+         */
+        public Builder binaryStringSupport(boolean enable) {
+            this.configuration.put(ClientConfigProperties.BINARY_STRING_SUPPORT.getKey(), String.valueOf(enable));
+            return this;
+        }
+
 
         /**
          * SNI SSL parameter that will be set for each outbound SSL socket.

client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java

@@ -1,5 +1,6 @@
 package com.clickhouse.client.api;
 
+import com.clickhouse.client.api.data_formats.ClickHouseFormatReader;
 import com.clickhouse.client.api.data_formats.internal.AbstractBinaryFormatReader;
 import com.clickhouse.client.api.enums.SSLMode;
 import com.clickhouse.client.api.internal.ClickHouseLZ4OutputStream;
@@ -181,6 +182,13 @@ public Object parseValue(String value) {
      */
     TYPE_HINT_MAPPING("type_hint_mapping", Map.class),
 
+    /**
+     * When enabled, {@code String} and {@code FixedString} columns are read into an intermediate {@code byte[]}
+     * instead of decoding them into a {@link String}. Improves working with large strings and lets
+     * {@link ClickHouseFormatReader#getByteArray} be used more effectively. Can be configured per operation.
+     */
+    BINARY_STRING_SUPPORT("binary_string_support", Boolean.class, "false"),
+
     /**
      * SNI SSL parameter that will be set for each outbound SSL socket.
      */

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

@@ -84,6 +84,10 @@ public interface ClickHouseBinaryFormatWriter {
 
     void setString(int colIndex, String value);
 
+    void setString(String column, byte[] value);
+
+    void setString(int colIndex, byte[] value);
+
     void setDate(String column, LocalDate value);
 
     void setDate(int colIndex, LocalDate value);

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

@@ -126,10 +126,18 @@ public void writeString(String value) throws IOException {
         BinaryStreamUtils.writeString(out, value);
     }
 
+    public void writeString(byte[] value) throws IOException {
+        BinaryStreamUtils.writeString(out, value);
+    }
+
     public void writeFixedString(String value, int len) throws IOException {
         BinaryStreamUtils.writeFixedString(out, value, len);
     }
 
+    public void writeFixedString(byte[] value, int len) throws IOException {
+        SerializerUtils.writeFixedStringBytes(out, value, len);
+    }
+
     public void writeDate(ZonedDateTime value) throws IOException {
         SerializerUtils.writeDate(out, value, value.getZone());
     }

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

@@ -203,6 +203,16 @@ public void setString(int colIndex, String value) {
         setValue(colIndex, value);
     }
 
+    @Override
+    public void setString(String column, byte[] value) {
+        setValue(column, value);
+    }
+
+    @Override
+    public void setString(int colIndex, byte[] value) {
+        setValue(colIndex, value);
+    }
+
     @Override
     public void setDate(String column, LocalDate value) {
         setValue(column, value);

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

@@ -23,7 +23,7 @@ public RowBinaryWithNamesFormatReader(InputStream inputStream,
                                           TableSchema schema,
                                           BinaryStreamReader.ByteBufferAllocator byteBufferAllocator,
                                           Map<ClickHouseDataType, Class<?>> typeHintMapping) {
-        super(inputStream, querySettings, schema, byteBufferAllocator,  typeHintMapping);
+        super(inputStream, querySettings, schema, byteBufferAllocator, typeHintMapping);
         int nCol = 0;
         try {
             nCol = BinaryStreamReader.readVarInt(input);

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

@@ -76,7 +76,7 @@ public abstract class AbstractBinaryFormatReader implements ClickHouseBinaryForm
     private long row = -1; // before first row
     private long lastNextCallTs; // for exception to detect slow reader
 
-    protected AbstractBinaryFormatReader(InputStream inputStream, QuerySettings querySettings, TableSchema schema,BinaryStreamReader.ByteBufferAllocator byteBufferAllocator, Map<ClickHouseDataType, Class<?>> defaultTypeHintMap) {
+    protected AbstractBinaryFormatReader(InputStream inputStream, QuerySettings querySettings, TableSchema schema, BinaryStreamReader.ByteBufferAllocator byteBufferAllocator, Map<ClickHouseDataType, Class<?>> defaultTypeHintMap) {
         this.input = inputStream;
         Map<String, Object> settings = querySettings == null ? Collections.emptyMap() : querySettings.getAllSettings();
         Boolean useServerTimeZone = (Boolean) settings.get(ClientConfigProperties.USE_SERVER_TIMEZONE.getKey());
@@ -88,8 +88,12 @@ protected AbstractBinaryFormatReader(InputStream inputStream, QuerySettings quer
         }
         boolean jsonAsString = MapUtils.getFlag(settings,
                 ClientConfigProperties.serverSetting(ServerSettings.OUTPUT_FORMAT_BINARY_WRITE_JSON_AS_STRING), false);
+        // Binary string support is resolved from the (already merged client + operation) query settings so it can be
+        // toggled per operation, not just per client.
+        boolean binaryStringSupport = MapUtils.getFlag(settings,
+                ClientConfigProperties.BINARY_STRING_SUPPORT.getKey(), false);
         this.binaryStreamReader = new BinaryStreamReader(inputStream, timeZone, LOG, byteBufferAllocator, jsonAsString,
-                defaultTypeHintMap);
+                defaultTypeHintMap, binaryStringSupport);
         if (schema != null) {
             setSchema(schema);
         }
@@ -532,8 +536,9 @@ private <T> T getPrimitiveArray(int index, Class<?> componentType) {
                 }
                 return (T)array;
             } else if (componentType == byte.class) {
-                if (value instanceof String) {
-                    return (T) ((String) value).getBytes(StandardCharsets.UTF_8);
+                byte[] bytes = stringLikeToBytes(value);
+                if (bytes != null) {
+                    return (T) bytes;
                 } else if (value instanceof InetAddress) {
                     return (T) ((InetAddress) value).getAddress();
                 }
@@ -676,6 +681,24 @@ public Instant getInstant(int index) {
         throw new ClientException("Column of type " + column.getDataType() + " cannot be converted to Instant");
     }
 
+    /**
+     * Converts a string-like value into its raw bytes. For a {@link StringValue} the original bytes are
+     * returned without re-encoding (so binary content is preserved). For a {@link String} the bytes are
+     * produced using UTF-8, matching the historical behaviour. Returns {@code null} when the value is not
+     * a string-like type so callers can fall back to other handling.
+     *
+     * @param value value to convert
+     * @return raw bytes or {@code null} if the value is not string-like
+     */
+    public static byte[] stringLikeToBytes(Object value) {
+        if (value instanceof StringValue) {
+            return ((StringValue) value).toByteArray();
+        } else if (value instanceof String) {
+            return ((String) value).getBytes(StandardCharsets.UTF_8);
+        }
+        return null;
+    }
+
     static Instant objectToInstant(Object value) {
         if (value instanceof LocalDateTime) {
             LocalDateTime dateTime = (LocalDateTime) value;
@@ -866,6 +889,10 @@ public String[] getStringArray(int index) {
             BinaryStreamReader.ArrayValue array = (BinaryStreamReader.ArrayValue) value;
             if (array.itemType == String.class) {
                 return (String[]) array.getArray();
+            } else if (array.itemType == StringValue.class) {
+                StringValue[] stringValues = (StringValue[]) array.getArray();
+                return Arrays.stream(stringValues)
+                        .map(sv -> sv == null ? null : sv.asString()).toArray(String[]::new);
             } else if (array.itemType == BinaryStreamReader.EnumValue.class) {
                 BinaryStreamReader.EnumValue[] enumValues = (BinaryStreamReader.EnumValue[]) array.getArray();
                 return Arrays.stream(enumValues).map(BinaryStreamReader.EnumValue::getName).toArray(String[]::new);