Use null binary value instead of empty byte array

ClickHouse · Jan 25, 2022 · 241feb3 · 241feb3
1 parent b11e807
commit 241feb3
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 25 deletions.
diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseInputStream.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseInputStream.java
@@ -298,7 +298,6 @@ public byte[] readBytes(int length) throws IOException {
             ensureOpen();
 
             byte[] bytes = new byte[length];
-            int offset = 0;
             int counter = 0;
             while (counter < length) {
                 if (position >= limit && updateBuffer() < 0) {
@@ -314,9 +313,8 @@ public byte[] readBytes(int length) throws IOException {
                 }
 
                 int size = Math.min(limit - position, length - counter);
-                System.arraycopy(buffer, position, bytes, offset, size);
+                System.arraycopy(buffer, position, bytes, counter, size);
                 position += size;
-                offset += size;
                 counter += size;
             }
 

diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseValue.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseValue.java
@@ -541,7 +541,7 @@ default <T, E extends Enum<E>> T asObject(Class<T> clazz) {
     /**
      * Gets binary value as byte array.
      *
-     * @return non-null byte array
+     * @return byte array which could be null
      */
     default byte[] asBinary() {
         return asBinary(0, null);
@@ -551,7 +551,7 @@ default byte[] asBinary() {
      * Gets binary value as fixed length byte array.
      *
      * @param length byte length of value, 0 or negative number means no limit
-     * @return non-null byte array
+     * @return byte array which could be null
      */
     default byte[] asBinary(int length) {
         return asBinary(length, null);
@@ -561,7 +561,7 @@ default byte[] asBinary(int length) {
      * Gets binary value as byte array.
      *
      * @param charset charset, null is same as default(UTF-8)
-     * @return non-null byte array
+     * @return byte array which could be null
      */
     default byte[] asBinary(Charset charset) {
         return asBinary(0, charset);
@@ -572,11 +572,11 @@ default byte[] asBinary(Charset charset) {
      *
      * @param length  byte length of value, 0 or negative number means no limit
      * @param charset charset, null is same as default(UTF-8)
-     * @return non-null byte array
+     * @return byte array which could be null
      */
     default byte[] asBinary(int length, Charset charset) {
         if (isNullOrEmpty()) {
-            return ClickHouseValues.EMPTY_BYTE_ARRAY;
+            return null;
         }
 
         byte[] bytes = asString().getBytes(charset == null ? StandardCharsets.UTF_8 : charset);

diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/data/BinaryStreamUtils.java b/clickhouse-client/src/main/java/com/clickhouse/client/data/BinaryStreamUtils.java
@@ -1649,6 +1649,19 @@ public static void writeString(OutputStream output, String value, Charset charse
         output.write(bytes);
     }
 
+    /**
+     * Writes a binary string to given output stream.
+     *
+     * @param output non-null output stream
+     * @param value  non-null byte array
+     * @throws IOException when failed to write value to output stream or reached
+     *                     end of the stream
+     */
+    public static void writeString(OutputStream output, byte[] value) throws IOException {
+        writeVarInt(output, value.length);
+        output.write(value);
+    }
+
     /**
      * Read varint from given input stream.
      *

diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/data/ClickHouseRowBinaryProcessor.java b/clickhouse-client/src/main/java/com/clickhouse/client/data/ClickHouseRowBinaryProcessor.java
@@ -335,11 +335,7 @@ private void buildMappingsForDataTypes() {
                     (v, f, c, o) -> o.write(v.asBinary(c.getPrecision())), ClickHouseDataType.FixedString);
             buildMappings(deserializers, serializers,
                     (r, f, c, i) -> ClickHouseStringValue.of(r, i.readBytes(i.readVarInt())),
-                    (v, f, c, o) -> {
-                        byte[] bytes = v.asBinary();
-                        BinaryStreamUtils.writeVarInt(o, bytes.length);
-                        o.write(bytes);
-                    }, ClickHouseDataType.String);
+                    (v, f, c, o) -> BinaryStreamUtils.writeString(o, v.asBinary()), ClickHouseDataType.String);
             buildMappings(deserializers, serializers,
                     (r, f, c, i) -> ClickHouseUuidValue.of(r, BinaryStreamUtils.readUuid(i)),
                     (v, f, c, o) -> BinaryStreamUtils.writeUuid(o, v.asUuid()), ClickHouseDataType.UUID);

diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/data/ClickHouseStringValue.java b/clickhouse-client/src/main/java/com/clickhouse/client/data/ClickHouseStringValue.java
@@ -9,6 +9,7 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.util.Arrays;
 import java.util.Objects;
 import java.util.UUID;
 import com.clickhouse.client.ClickHouseChecker;
@@ -217,7 +218,7 @@ public byte[] asBinary() {
             bytes = value.getBytes(StandardCharsets.UTF_8);
         }
 
-        return bytes != null ? bytes : ClickHouseValues.EMPTY_BYTE_ARRAY;
+        return bytes;
     }
 
     @Override
@@ -229,7 +230,7 @@ public byte[] asBinary(int length, Charset charset) {
         if (bytes != null && length > 0) {
             return ClickHouseChecker.notWithDifferentLength(bytes, length);
         } else {
-            return bytes != null ? bytes : ClickHouseValues.EMPTY_BYTE_ARRAY;
+            return bytes;
         }
     }
 
@@ -380,6 +381,16 @@ public ClickHouseStringValue update(Object value) {
         return update(value == null ? null : value.toString());
     }
 
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + (binary ? 1231 : 1237);
+        result = prime * result + Arrays.hashCode(bytes);
+        result = prime * result + ((value == null) ? 0 : value.hashCode());
+        return result;
+    }
+
     @Override
     public boolean equals(Object obj) {
         if (this == obj) { // too bad this is a mutable class :<
@@ -389,12 +400,7 @@ public boolean equals(Object obj) {
         }
 
         ClickHouseStringValue v = (ClickHouseStringValue) obj;
-        return Objects.equals(bytes, v.bytes) && Objects.equals(value, v.value);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(bytes, value);
+        return binary == v.binary && Objects.equals(bytes, v.bytes) && Objects.equals(value, v.value);
     }
 
     @Override

diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/data/ClickHouseStringValueTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/data/ClickHouseStringValueTest.java
@@ -57,12 +57,12 @@ public void testTypeConversion() {
 
     @Test(groups = { "unit" })
     public void testBinaryValue() {
-        Assert.assertEquals(ClickHouseStringValue.of((byte[]) null).asBinary(), new byte[0]);
-        Assert.assertEquals(ClickHouseStringValue.of((String) null).asBinary(), new byte[0]);
+        Assert.assertEquals(ClickHouseStringValue.of((byte[]) null).asBinary(), null);
+        Assert.assertEquals(ClickHouseStringValue.of((String) null).asBinary(), null);
         Assert.assertEquals(ClickHouseStringValue.of(new byte[0]).asBinary(), new byte[0]);
         Assert.assertEquals(ClickHouseStringValue.of("").asBinary(), new byte[0]);
-        Assert.assertEquals(ClickHouseStringValue.of((byte[]) null).asBinary(0), new byte[0]);
-        Assert.assertEquals(ClickHouseStringValue.of((String) null).asBinary(0), new byte[0]);
+        Assert.assertEquals(ClickHouseStringValue.of((byte[]) null).asBinary(0), null);
+        Assert.assertEquals(ClickHouseStringValue.of((String) null).asBinary(0), null);
         Assert.assertEquals(ClickHouseStringValue.of(new byte[0]).asBinary(0), new byte[0]);
         Assert.assertEquals(ClickHouseStringValue.of("").asBinary(0), new byte[0]);
 

diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java
@@ -37,6 +37,13 @@ private Object[][] getTypedParameters() {
     @Test(groups = "integration")
     public void testReadWriteBinaryString() throws SQLException {
         Properties props = new Properties();
+        try (ClickHouseConnection conn = newConnection(props);
+                Statement s = conn.createStatement()) {
+            s.execute("drop table if exists test_binary_string; "
+                    + "create table test_binary_string(id Int32, "
+                    + "f0 FixedString(3), f1 Nullable(FixedString(3)), s0 String, s1 Nullable(String))engine=Memory");
+        }
+
         byte[] bytes = new byte[256];
         for (int i = 0; i < 256; i++) {
             bytes[i] = (byte) i;
@@ -51,6 +58,55 @@ public void testReadWriteBinaryString() throws SQLException {
             Assert.assertEquals(rs.getInt(2), bytes.length);
             Assert.assertFalse(rs.next());
         }
+
+        bytes = new byte[] { 0x61, 0x62, 0x63 };
+        try (ClickHouseConnection conn = newConnection(props);
+                PreparedStatement ps = conn.prepareStatement("insert into test_binary_string")) {
+            ps.setInt(1, 1);
+            ps.setBytes(2, bytes);
+            ps.setBytes(3, null);
+            ps.setBytes(4, bytes);
+            ps.setBytes(5, null);
+            ps.addBatch();
+            ps.setInt(1, 2);
+            ps.setString(2, "abc");
+            ps.setString(3, null);
+            ps.setString(4, "abc");
+            ps.setString(5, null);
+            ps.addBatch();
+            ps.setInt(1, 3);
+            ps.setBytes(2, bytes);
+            ps.setBytes(3, bytes);
+            ps.setBytes(4, bytes);
+            ps.setBytes(5, bytes);
+            ps.addBatch();
+            ps.setInt(1, 4);
+            ps.setString(2, "abc");
+            ps.setString(3, "abc");
+            ps.setString(4, "abc");
+            ps.setString(5, "abc");
+            ps.addBatch();
+            ps.executeBatch();
+        }
+
+        try (ClickHouseConnection conn = newConnection(props);
+                PreparedStatement ps = conn
+                        .prepareStatement(
+                                "select distinct * except(id) from test_binary_string where f0 = ? order by id")) {
+            ps.setBytes(1, bytes);
+            ResultSet rs = ps.executeQuery();
+            Assert.assertTrue(rs.next(), "Should have at least one row");
+            Assert.assertEquals(rs.getBytes(1), bytes);
+            Assert.assertNull(rs.getBytes(2), "f1 should be null");
+            Assert.assertEquals(rs.getBytes(3), bytes);
+            Assert.assertNull(rs.getBytes(4), "s1 should be null");
+            Assert.assertTrue(rs.next(), "Should have at least two rows");
+            for (int i = 1; i <= 4; i++) {
+                Assert.assertEquals(rs.getBytes(i), bytes);
+                Assert.assertEquals(rs.getString(i), "abc");
+            }
+            Assert.assertFalse(rs.next(), "Should not have more than two rows");
+        }
     }
 
     @Test(groups = "integration")