Fix client-v2 Dynamic column BigDecimal type inference (truncation + overflow)

CHANGELOG.md

@@ -19,6 +19,12 @@
 
 ### Bug Fixes 
 
+- **[client-v2]** Fixed `BigDecimal` values written into a `Dynamic` column being silently truncated when the
+  value's scale exceeded the inferred width's maximum scale, and throwing an overflow error when the value
+  carried an integer part (e.g. `19.99`). The `Dynamic` type inference now sizes the `Decimal` width to hold
+  both the integer digits and the value's scale, keeps the scale as wide as the width allows without stealing
+  room from the integer part, and writes the actual column scale into the `Dynamic` type tag. Values that
+  already round-tripped losslessly are unchanged. (https://github.com/ClickHouse/clickhouse-java/issues/2966)
 - **[client-v2]** Fixed the `RowBinary` writer throwing
   `UnsupportedOperationException: Unsupported data type: SimpleAggregateFunction` when inserting into a
   `SimpleAggregateFunction(func, T)` column (the reader already supported these columns). The value is now

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

@@ -209,23 +209,43 @@ public static ClickHouseColumn valueToColumnForDynamicType(Object value) {
             column = ClickHouseColumn.of("v", "DateTime64(9, " + ZoneId.systemDefault().getId() + ")");
         } else if (value instanceof BigDecimal) {
             BigDecimal d = (BigDecimal) value;
-            String decType;
-            int scale;
-            if (d.precision() > ClickHouseDataType.Decimal128.getMaxScale()) {
-                decType = "Decimal256";
-                scale = ClickHouseDataType.Decimal256.getMaxScale();
-            } else if (d.precision() > ClickHouseDataType.Decimal64.getMaxScale()) {
-                decType = "Decimal128";
-                scale = ClickHouseDataType.Decimal128.getMaxScale();
-            } else if (d.precision() > ClickHouseDataType.Decimal32.getMaxScale()) {
-                decType = "Decimal64";
-                scale = ClickHouseDataType.Decimal64.getMaxScale();
+            // A DecimalN(S) column is Decimal(P, S) with P fixed to the width's precision
+            // (Decimal32=9, Decimal64=18, Decimal128=38, Decimal256=76) and 0 <= S <= P, so it can
+            // hold at most P - S integer digits. Size the width to fit both the integer digits and
+            // the value's own scale, then keep S as wide as the width allows without stealing room
+            // from the integer part. Keying the width off precision() alone and forcing S to the
+            // width maximum truncated values whose scale exceeded that maximum, and overflowed
+            // values that carried an integer part.
+            int valueScale = Math.max(d.scale(), 0);
+            int integerDigits = Math.max(d.precision() - d.scale(), 0);
+            int requiredPrecision = integerDigits + valueScale;
+            if (requiredPrecision > ClickHouseDataType.Decimal256.getMaxPrecision()) {
+                if (d.signum() != 0) {
+                    throw new ClientException("Unable to serialize BigDecimal into a Dynamic column: it needs "
+                            + requiredPrecision + " digits of precision, exceeding the maximum supported Decimal256 precision of "
+                            + ClickHouseDataType.Decimal256.getMaxPrecision());
+                }
+                // A numerically-zero value rounds to zero at any scale with no data loss, so it fits
+                // any Decimal width regardless of the scale/exponent implied by precision()/scale()
+                // (e.g. 0E-77 implies scale 77, 0E+77 implies 78 integer digits). Store it in the
+                // widest band rather than rejecting a value ClickHouse can represent exactly; cap the
+                // integer digits so the emitted scale (maxScale - integerDigits) stays non-negative.
+                integerDigits = Math.min(integerDigits, ClickHouseDataType.Decimal256.getMaxScale());
+                requiredPrecision = ClickHouseDataType.Decimal256.getMaxPrecision();
+            }
+            ClickHouseDataType decType;
+            if (requiredPrecision > ClickHouseDataType.Decimal128.getMaxPrecision()) {
+                decType = ClickHouseDataType.Decimal256;
+            } else if (requiredPrecision > ClickHouseDataType.Decimal64.getMaxPrecision()) {
+                decType = ClickHouseDataType.Decimal128;
+            } else if (requiredPrecision > ClickHouseDataType.Decimal32.getMaxPrecision()) {
+                decType = ClickHouseDataType.Decimal64;
             } else {
-                decType = "Decimal32";
-                scale = ClickHouseDataType.Decimal32.getMaxScale();
+                decType = ClickHouseDataType.Decimal32;
             }
+            int scale = decType.getMaxScale() - integerDigits;
 
-            column = ClickHouseColumn.of("v", decType + "(" + scale + ")");
+            column = ClickHouseColumn.of("v", decType.name() + "(" + scale + ")");
         } else if (value instanceof Map<?,?>) {
             Map<?, ?> map = (Map<?, ?>) value;
             // TODO: handle empty map?
@@ -382,7 +402,7 @@ public static void writeDynamicTypeTag(OutputStream stream, ClickHouseColumn typ
             case Decimal256:
                 stream.write(binTag);
                 BinaryStreamUtils.writeUnsignedInt8(stream, dt.getMaxPrecision());
-                BinaryStreamUtils.writeUnsignedInt8(stream, dt.getMaxScale());
+                BinaryStreamUtils.writeUnsignedInt8(stream, typeColumn.getScale());
                 break;
             case IntervalNanosecond:
             case IntervalMillisecond:

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

@@ -1,5 +1,6 @@
 package com.clickhouse.client.api.data_formats.internal;
 
+import com.clickhouse.client.api.ClientException;
 import com.clickhouse.data.ClickHouseColumn;
 import com.clickhouse.data.ClickHouseDataType;
 import com.clickhouse.data.value.ClickHouseGeoPolygonValue;
@@ -144,6 +145,54 @@ public void testGeometrySerializationRejectsMalformedList() {
                         ClickHouseColumn.of("v", "Geometry")));
     }
 
+    @DataProvider(name = "dynamicDecimalTypeInference")
+    public Object[][] dynamicDecimalTypeInference() {
+        return new Object[][]{
+                // value, inferred width, inferred scale. The width must hold integerDigits + scale,
+                // and the scale is kept as wide as the width allows (maxScale - integerDigits).
+                {new BigDecimal("0.5"), ClickHouseDataType.Decimal32, 9},                 // sub-1, unchanged
+                {new BigDecimal("19.99"), ClickHouseDataType.Decimal32, 7},               // integer part 2
+                {new BigDecimal("-19.99"), ClickHouseDataType.Decimal32, 7},              // sign is irrelevant
+                {new BigDecimal("1000"), ClickHouseDataType.Decimal32, 5},                // integer, scale 0
+                {new BigDecimal("1E3"), ClickHouseDataType.Decimal32, 5},                 // negative scale (-3)
+                {new BigDecimal("0"), ClickHouseDataType.Decimal32, 8},
+                {new BigDecimal("1.23456789"), ClickHouseDataType.Decimal32, 8},          // required precision == 9 boundary
+                {new BigDecimal("0.0123456789"), ClickHouseDataType.Decimal64, 18},       // scale 10 > Decimal32 max
+                {new BigDecimal("123456789.123456789"), ClickHouseDataType.Decimal64, 9},
+                {new BigDecimal("0.00012345678901234567"), ClickHouseDataType.Decimal128, 38}, // scale 20 > Decimal64 max
+                {new BigDecimal("12345678901234567890.12345678901234567890"), ClickHouseDataType.Decimal256, 56}, // 20 int + 20 frac
+                {new BigDecimal("0.12345678901234567890123456789012345678901"), ClickHouseDataType.Decimal256, 76}, // scale 41
+                // Numerically-zero values whose implied width exceeds Decimal256 still fit: zero rounds
+                // to zero at any scale with no loss, so they map to the widest band, not a rejection.
+                {new BigDecimal("0E-77"), ClickHouseDataType.Decimal256, 76},             // zero, scale 77 > max scale
+                {new BigDecimal("0E+100"), ClickHouseDataType.Decimal256, 0},             // zero, 101 integer digits > max
+                {new BigDecimal(BigInteger.ZERO, Integer.MAX_VALUE), ClickHouseDataType.Decimal256, 76}, // zero, maximal scale
+        };
+    }
+
+    @Test(dataProvider = "dynamicDecimalTypeInference")
+    public void testValueToColumnForDynamicTypeSizesDecimal(BigDecimal value, ClickHouseDataType expectedType, int expectedScale) {
+        ClickHouseColumn column = SerializerUtils.valueToColumnForDynamicType(value);
+        Assert.assertEquals(column.getDataType(), expectedType);
+        Assert.assertEquals(column.getScale(), expectedScale);
+    }
+
+    @DataProvider(name = "oversizedDecimalValues")
+    public Object[][] oversizedDecimalValues() {
+        return new Object[][]{
+                // Non-zero values that genuinely exceed Decimal256: fail loudly, never truncate. The
+                // zero-fits-any-width relaxation must NOT let these through — they carry real digits.
+                {new BigDecimal(BigInteger.TEN.pow(76))},  // 10^76: 77 integer digits, one past the width
+                {new BigDecimal(BigInteger.ONE, 77)},      // 1E-77: scale 77, one past the max scale
+        };
+    }
+
+    @Test(dataProvider = "oversizedDecimalValues")
+    public void testValueToColumnForDynamicTypeRejectsOversizedDecimal(BigDecimal value) {
+        Assert.assertThrows(ClientException.class,
+                () -> SerializerUtils.valueToColumnForDynamicType(value));
+    }
+
     @Test(dataProvider = "nonNullableEnumTypes")
     public void testNullIntoNonNullableEnumThrowsIllegalArgument(String typeName) {
         ClickHouseColumn column = ClickHouseColumn.of("bs_flag", typeName);

client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java

@@ -981,6 +981,77 @@ public void testDynamicWithPrimitives() throws Exception {
         }
     }
 
+    @DataProvider(name = "dynamicDecimalValues")
+    public Object[][] dynamicDecimalValues() {
+        return new Object[][]{
+                // Low-precision values whose scale exceeds the inferred width's maximum scale:
+                // previously silently truncated on write.
+                {new BigDecimal("0.0123456789")},            // scale 10 > Decimal32 max scale 9
+                {new BigDecimal("0.00012345678901234567")},  // scale 20 > Decimal64 max scale 18
+                {new BigDecimal("0.12345678901234567890123456789012345678901")}, // scale 41 -> Decimal256
+                // Values carrying an integer part: previously overflowed on insert because the
+                // scale was forced to the full width precision, leaving no room for integer digits.
+                {new BigDecimal("19.99")},
+                {new BigDecimal("-19.99")},
+                {new BigDecimal("1000")},
+                {new BigDecimal("1E3")},                     // negative scale (-3)
+                {new BigDecimal("123456789.123456789")},
+                {new BigDecimal("12345678901234567890.12345678901234567890")}, // 20 int + 20 frac -> Decimal256
+                {new BigDecimal("0")},
+                // A numerically-zero value whose scale exceeds any width: it fits (zero rounds to zero
+                // with no loss) and must round-trip rather than being rejected on insert.
+                {new BigDecimal("0E-77")},                   // zero, scale 77 > Decimal256 max scale
+        };
+    }
+
+    @Test(groups = {"integration"}, dataProvider = "dynamicDecimalValues")
+    public void testDynamicColumnPreservesBigDecimalValue(BigDecimal value) throws Exception {
+        if (isVersionMatch("(,24.8]")) {
+            return;
+        }
+        final String table = "test_dynamic_decimal_roundtrip";
+        final int tail = 987654321;
+        client.execute("DROP TABLE IF EXISTS " + table).get();
+        client.execute(tableDefinition(table, "rowId Int32", "field Dynamic", "tail Int32"),
+                (CommandSettings) new CommandSettings().serverSetting("allow_experimental_dynamic_type", "1")).get();
+        client.register(DTOForDynamicDecimalTests.class, client.getTableSchema(table));
+
+        client.insert(table, Collections.singletonList(new DTOForDynamicDecimalTests(1, value, tail))).get().close();
+
+        List<GenericRecord> rows = client.queryAll("SELECT field, tail FROM " + table);
+        Assert.assertEquals(rows.size(), 1);
+        GenericRecord row = rows.get(0);
+        Assert.assertEquals(row.getBigDecimal("field").compareTo(value), 0,
+                "Dynamic Decimal round-trip must not lose data for " + value.toPlainString());
+        // The trailing fixed-width column stays intact only if the Decimal width written matches the
+        // type tag; a wrong width would shift the following bytes and corrupt it.
+        Assert.assertEquals(row.getInteger("tail"), tail);
+    }
+
+    @Test(groups = {"integration"})
+    public void testDynamicColumnFractionalDecimalRepresentationUnchanged() throws Exception {
+        if (isVersionMatch("(,24.8]")) {
+            return;
+        }
+        final String table = "test_dynamic_decimal_unchanged";
+        final int tail = 123456789;
+        client.execute("DROP TABLE IF EXISTS " + table).get();
+        client.execute(tableDefinition(table, "rowId Int32", "field Dynamic", "tail Int32"),
+                (CommandSettings) new CommandSettings().serverSetting("allow_experimental_dynamic_type", "1")).get();
+        client.register(DTOForDynamicDecimalTests.class, client.getTableSchema(table));
+
+        // A sub-integer value (no integer part) whose scale fits the smallest width was already
+        // stored correctly, as Decimal32 with the scale padded to the width maximum. The fix must
+        // leave this representation byte-for-byte unchanged.
+        client.insert(table, Collections.singletonList(new DTOForDynamicDecimalTests(1, new BigDecimal("0.5"), tail))).get().close();
+
+        GenericRecord row = client.queryAll("SELECT field, tail FROM " + table).get(0);
+        BigDecimal readBack = row.getBigDecimal("field");
+        Assert.assertEquals(readBack, new BigDecimal("0.500000000"));
+        Assert.assertEquals(readBack.scale(), 9);
+        Assert.assertEquals(row.getInteger("tail"), tail);
+    }
+
     @Test(groups = {"integration"})
     public void testDynamicWithArrays() throws Exception {
         testDynamicWith("arrays",
@@ -1244,6 +1315,14 @@ public static class DTOForDynamicDefaultsTests {
         private String extra;
     }
 
+    @Data
+    @AllArgsConstructor
+    public static class DTOForDynamicDecimalTests {
+        private int rowId;
+        private Object field;
+        private int tail;
+    }
+
     @Test(groups = {"integration"})
     public void testAllDataTypesKnown() {
         List<GenericRecord> dbTypes = client.queryAll("SELECT * FROM system.data_type_families");