From 27d130e704a9ef790174a8e310b0fb19d5d9fc1c Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:51:10 +0000 Subject: [PATCH 1/2] Fix client-v2 Dynamic column BigDecimal type inference valueToColumnForDynamicType inferred the Decimal width from BigDecimal.precision() alone and forced the scale to that width's maximum. Since DecimalN(S) is Decimal(P, S) with P fixed to the width precision (9/18/38/76) and 0 <= S <= P, forcing S = P left no room for an integer part and could not represent a scale larger than P. As a result a value whose scale exceeded the width maximum was silently truncated on write, and a value with an integer part (e.g. 19.99) overflowed and threw on insert. Size the width to hold both the integer digits and the value's scale, and keep the scale as wide as the width allows (maxScale - integerDigits) so the integer part still fits; throw a clear error when the value needs more than Decimal256 precision instead of truncating. Also write the actual column scale into the Dynamic type tag (it previously wrote the width maximum), so the tag matches the serialized value. Sub-1 values that already round-tripped are inferred to the identical type and are unchanged. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/2966 --- CHANGELOG.md | 6 ++ .../internal/SerializerUtils.java | 41 ++++++---- .../internal/SerializerUtilsTest.java | 35 +++++++++ .../client/datatypes/DataTypeTests.java | 76 +++++++++++++++++++ 4 files changed, 143 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca4088730..f4d3d6517 100644 --- a/CHANGELOG.md +++ b/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 scalar `String` query parameters containing a tab (`0x09`), newline (`0x0a`) or backslash being mishandled through the server's `param_` interface. A `{name:String}` parameter value is parsed by the server with `deserializeTextEscaped`, which treated a raw tab or diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java index db476bc08..0131dd1bd 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java @@ -197,23 +197,34 @@ 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()) { + 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()); + } + 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? @@ -370,7 +381,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: diff --git a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java index 41601c2d6..e20d23cf3 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java +++ b/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,40 @@ 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 + }; + } + + @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); + } + + @Test + public void testValueToColumnForDynamicTypeRejectsOversizedDecimal() { + // 10^76 has 77 integer digits, one more than Decimal256 can represent: fail loudly, never truncate. + Assert.assertThrows(ClientException.class, + () -> SerializerUtils.valueToColumnForDynamicType(new BigDecimal(BigInteger.TEN.pow(76)))); + } + @Test(dataProvider = "nonNullableEnumTypes") public void testNullIntoNonNullableEnumThrowsIllegalArgument(String typeName) { ClickHouseColumn column = ClickHouseColumn.of("bs_flag", typeName); diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java index 3fc39ac46..c34ad6d6d 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java @@ -981,6 +981,74 @@ 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")}, + }; + } + + @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 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 +1312,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 dbTypes = client.queryAll("SELECT * FROM system.data_type_families"); From 72cab06643457668e1a25ca5679343183fc2c912 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:44:36 +0000 Subject: [PATCH 2/2] Do not reject a numerically-zero BigDecimal with high scale in Dynamic inference A numerically-zero BigDecimal whose scale/exponent implies more than the maximum 76 digits (e.g. 0E-77, precision 1 / scale 77) was rejected with a ClientException, even though zero rounds to zero at any scale with no data loss and such inserts round-tripped fine before this PR introduced the over-precision check. Restrict the rejection to non-zero values; map an over-wide zero to the widest Decimal256 band, capping the integer digits so the emitted scale stays within [0, maxScale]. Addresses a review comment on PR #2967. --- .../internal/SerializerUtils.java | 15 ++++++++++--- .../internal/SerializerUtilsTest.java | 22 +++++++++++++++---- .../client/datatypes/DataTypeTests.java | 3 +++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java index 578dc7a16..b4b3b2aa0 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java @@ -220,9 +220,18 @@ public static ClickHouseColumn valueToColumnForDynamicType(Object value) { int integerDigits = Math.max(d.precision() - d.scale(), 0); int requiredPrecision = integerDigits + valueScale; if (requiredPrecision > ClickHouseDataType.Decimal256.getMaxPrecision()) { - 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()); + 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()) { diff --git a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java index db90da9a4..1d807884f 100644 --- a/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/api/data_formats/internal/SerializerUtilsTest.java @@ -162,6 +162,11 @@ public Object[][] dynamicDecimalTypeInference() { {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 }; } @@ -172,11 +177,20 @@ public void testValueToColumnForDynamicTypeSizesDecimal(BigDecimal value, ClickH Assert.assertEquals(column.getScale(), expectedScale); } - @Test - public void testValueToColumnForDynamicTypeRejectsOversizedDecimal() { - // 10^76 has 77 integer digits, one more than Decimal256 can represent: fail loudly, never truncate. + @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(new BigDecimal(BigInteger.TEN.pow(76)))); + () -> SerializerUtils.valueToColumnForDynamicType(value)); } @Test(dataProvider = "nonNullableEnumTypes") diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java index c34ad6d6d..27e5891be 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/DataTypeTests.java @@ -998,6 +998,9 @@ public Object[][] dynamicDecimalValues() { {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 }; }