Merge pull request #666 from zhicwu/negative-decimal

Fix the issue of hanlding negative big decimals
ClickHouse · Jun 7, 2021 · 14384ae · 14384ae
2 parents 5a5fb7d + 4e67c9c
commit 14384ae
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 15 deletions.
diff --git a/clickhouse-jdbc/docs/datetime.md b/clickhouse-jdbc/docs/datetime.md
@@ -51,7 +51,7 @@ The methods which take a [Calendar]((https://docs.oracle.com/javase/8/docs/api/j
 For Date and DateTime fields, the JDBC driver has enough time zone related information available, so these methods would only be relevant for String or other typed fields. There might be valid use cases, but for now we think that adding such an option would make things even more complicated.
 
 Requested Type | Number | Date | DateTime | Other 
----------------| ----------------------------------  
+---------------| -------|------|----------|--------  
 [Date](https://docs.oracle.com/javase/8/docs/api/java/sql/Date.html) | Seconds or milliseconds past epoch truncated to day in relevant time zone | Date in relevant time zone, midnight |  Date time in relevant time zone, rewind to midnight | Try number, date time (with or without offset) truncated to day, date
 [Time](https://docs.oracle.com/javase/8/docs/api/java/sql/Time.html) | Local time at 1970-01-01 (e.g. “1337” is “13:37:00” at TZ) | Midnight on 1970-01-01 in relevant time zone | Local time in relevant time zone | Local time in relevant time zone via ISO format or via number, at 1970-01-01
 [Timestamp](https://docs.oracle.com/javase/8/docs/api/java/sql/Timestamp.html) | Seconds or milliseconds past epoch | Local date at midnight in relevant time zone | Local date and time in relevant time zone | Number, date time with or without offset

diff --git a/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStream.java b/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStream.java
@@ -188,22 +188,24 @@ public void writeFloat64(double value) throws IOException {
         Utils.writeLong(out, Double.doubleToLongBits(value));
     }
 
-    public void writeDecimal128(BigDecimal num, int scale) throws IOException {
-        BigInteger bi = Utils.toBigInteger(num, scale);
-        byte[] r = bi.toByteArray();
-        for (int i = r.length; i > 0; i--) {
-            out.write(r[i - 1]);
+    public void writeBigInteger(BigInteger value, int byteLength) throws IOException {
+        byte empty = value.signum() == -1 ? (byte) 0xFF : 0x00;
+        byte[] bytes = value.toByteArray();
+        for (int i = bytes.length - 1; i >= 0; i--) {
+            out.write(bytes[i]);
+        }
+
+        for (int i = byteLength - bytes.length; i > 0; i--) {
+            out.write(empty);
         }
-        out.write(new byte[16 - r.length]);
+    }
+
+    public void writeDecimal128(BigDecimal num, int scale) throws IOException {
+        writeBigInteger(Utils.toBigInteger(num, scale), 16);
     }
 
     public void writeDecimal256(BigDecimal num, int scale) throws IOException {
-        BigInteger bi = Utils.toBigInteger(num, scale);
-        byte[] r = bi.toByteArray();
-        for (int i = r.length; i > 0; i--) {
-            out.write(r[i - 1]);
-        }
-        out.write(new byte[32 - r.length]);
+        writeBigInteger(Utils.toBigInteger(num, scale), 32);
     }
 
     public void writeDecimal64(BigDecimal num, int scale) throws IOException {

diff --git a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/integration/RowBinaryStreamTest.java b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/integration/RowBinaryStreamTest.java
@@ -2,11 +2,11 @@
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
-import static org.testng.Assert.assertThrows;
 import static org.testng.Assert.assertTrue;
 
 import java.io.EOFException;
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.sql.Date;
 import java.sql.ResultSet;
@@ -276,6 +276,46 @@ public void testBitmap() throws Exception {
         }
     }
 
+    @Test
+    public void testBigDecimals() throws Exception {
+        try (ClickHouseStatement statement = connection.createStatement()) {
+            statement.execute("set allow_experimental_bigint_types=1;"
+                + "create table if not exists test.test_big_decimals(d128 Decimal128(6), d256 Decimal256(12)) engine=Memory");
+        } catch (SQLException e) {
+            return;
+        }
+
+        try (ClickHouseStatement statement = connection.createStatement()) {
+            BigDecimal[] values = new BigDecimal[] {
+                BigDecimal.valueOf(-123.123456789D),
+                BigDecimal.ZERO,
+                BigDecimal.valueOf(123.123456789D)
+            };
+            statement.sendRowBinaryStream("insert into table test.test_big_decimals", new ClickHouseStreamCallback() {
+                @Override
+                public void writeTo(ClickHouseRowBinaryStream stream) throws IOException {
+                    for (int i = 0; i < values.length; i++) {
+                        stream.writeDecimal128(values[i], 6);
+                        stream.writeDecimal256(values[i], 12);
+                    }
+                }
+            });
+
+            try (ResultSet rs = statement.executeQuery("select * from test.test_big_decimals order by d128")) {
+                int rowCounter = 0;
+                while (rs.next()) {
+                    rowCounter++;
+                    assertEquals(rs.getBigDecimal(1, 6), values[rowCounter - 1].setScale(6, BigDecimal.ROUND_DOWN));
+                    assertEquals(rs.getBigDecimal(2, 12), values[rowCounter - 1].setScale(12, BigDecimal.ROUND_DOWN));
+                }
+
+                assertEquals(rowCounter, values.length);
+            }
+
+            statement.execute("drop table if exists test.test_big_decimals");
+        }
+    }
+
     private void testRowBinaryStream(boolean rowBinaryResult) throws Exception {
         createTable("test.raw_binary");
         ClickHouseStatement statement = connection.createStatement();

diff --git a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStreamTest.java b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStreamTest.java
@@ -112,9 +112,32 @@ public void testDecimal128() throws Exception {
                     @Override
                     public void write(ClickHouseRowBinaryStream stream) throws Exception {
                         stream.writeDecimal128(new BigDecimal(10.23), 3);
+                        stream.writeDecimal128(new BigDecimal(-10.23), 3);
                     }
                 },
-                new byte[]{-10, 39, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
+                new byte[] {
+                    -10, 39, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                    10, -40, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
+                }
+        );
+    }
+
+    @Test
+    public void testDecimal256() throws Exception {
+        check(
+                new StreamWriter() {
+                    @Override
+                    public void write(ClickHouseRowBinaryStream stream) throws Exception {
+                        stream.writeDecimal256(new BigDecimal(10.23), 3);
+                        stream.writeDecimal256(new BigDecimal(-10.23), 3);
+                    }
+                },
+                new byte[] {
+                    -10, 39, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                    10, -40, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
+                    -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
+                }
         );
     }