From 577e765395222e69b6c544274315aa3ae95c28cd Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Mon, 20 Jul 2026 09:07:53 +0000 Subject: [PATCH 1/3] Fix client-v2: reject null into non-nullable Array in RowBinary writer Writing a Java null into a non-nullable Array(...) column via RowBinaryFormatWriter emitted two bytes (00 00) instead of one: writeValuePreamble special-cased Array and wrote a stray writeNonNull marker (0x00) on top of the array length that serializeArrayData(null) already writes (var-uint 0 = 0x00). The server read the extra byte as a phantom extra row (single-column inserts) or a column shift that failed the whole insert with CANNOT_READ_ALL_DATA (multi-column inserts). A non-nullable Array cannot represent a null, so it now throws IllegalArgumentException naming the column - like every other non-nullable type and the merged Enum fix (#2932) - in both the RowBinary and RowBinaryWithDefaults branches. Empty arrays still serialize as a single length byte, columns with a DDL default still use the default under RowBinaryWithDefaults, and Dynamic columns (which can hold a null as the implicit Nothing type) are left unchanged. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/2938 --- CHANGELOG.md | 9 ++ .../RowBinaryFormatSerializer.java | 11 +- .../datatypes/RowBinaryFormatWriterTest.java | 117 ++++++++++++++++++ .../client/internal/SerializerUtilsTests.java | 14 +++ 4 files changed, 144 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4f9aabad..0c6c8b10c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,15 @@ NPE instead of a clear error. It now throws `IllegalArgumentException` naming the column, consistent with the existing `IllegalArgumentException` for other unsupported enum values. Nullable enum columns are unaffected. (https://github.com/ClickHouse/clickhouse-java/issues/2931) +- **[client-v2]** Fixed silent data corruption when serializing a Java `null` into a non-nullable + `Array(...)` column via `RowBinaryFormatWriter`. `RowBinaryFormatSerializer.writeValuePreamble` + special-cased `Array`, emitting a stray marker byte on top of the array length; the server read the + extra byte as a phantom extra row (single-column inserts) or as a column shift that failed the insert + with `CANNOT_READ_ALL_DATA` (multi-column inserts). A non-nullable `Array` cannot represent a `null`, + so it now throws `IllegalArgumentException` naming the column — consistent with every other non-nullable + type — in both the `RowBinary` and `RowBinaryWithDefaults` paths. Empty arrays (`[]`) still serialize + correctly, and `Dynamic` columns, which can hold a `null` as the implicit `Nothing` type, are + unaffected. (https://github.com/ClickHouse/clickhouse-java/issues/2938) - **[client-v2]** Fixed POJO insert error classification so transport write failures such as java.net.SocketException: Broken pipe (Write failed) are now surfaced as transfer/network errors instead of being wrapped as DataSerializationException. This only changes the exception type reported for request-body transport failures during diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java index ad8ee680a..edc602bb4 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java @@ -204,10 +204,9 @@ public static boolean writeValuePreamble(OutputStream out, boolean defaultsSuppo SerializerUtils.writeNonNull(out); SerializerUtils.writeNull(out);//Then we send null, write 1 return false;//And we're done - } else if (dataType == ClickHouseDataType.Array) {//If the column is an array - SerializerUtils.writeNonNull(out);//Then we send nonNull } else if (dataType == ClickHouseDataType.Dynamic) { - SerializerUtils.writeNonNull(out); + // A Dynamic column can hold a null: it is serialized below as the implicit `Nothing` type. + SerializerUtils.writeNonNull(out);//Write 0 for no default } else { throw new IllegalArgumentException(String.format("An attempt to write null into not nullable column '%s'", column)); } @@ -220,10 +219,8 @@ public static boolean writeValuePreamble(OutputStream out, boolean defaultsSuppo } SerializerUtils.writeNonNull(out); } else if (value == null) { - if (dataType == ClickHouseDataType.Array) { - SerializerUtils.writeNonNull(out); - } else if (dataType == ClickHouseDataType.Dynamic) { - // do nothing + if (dataType == ClickHouseDataType.Dynamic) { + // A Dynamic column can hold a null: it is serialized below as the implicit `Nothing` type. } else { throw new IllegalArgumentException(String.format("An attempt to write null into not nullable column '%s'", column)); } diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java index 333feaa5f..a5776eb0c 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java @@ -335,6 +335,123 @@ private static Map singleEntryMap(String key, Object value) { return map; } + @DataProvider(name = "rowBinaryWriterFormats") + private Object[][] rowBinaryWriterFormats() { + return new Object[][] { + {ClickHouseFormat.RowBinary}, + {ClickHouseFormat.RowBinaryWithDefaults}, + }; + } + + // A non-nullable Array column cannot represent a null, so writing a Java null into it must fail + // loudly like every other non-nullable type instead of emitting a stray marker byte on top of the + // array length. That stray byte was read by the server as a phantom extra row (single column) or + // shifted every following column (multi column). 'tail' sits after the array so a stray byte corrupts it. + @Test (groups = { "integration" }, dataProvider = "rowBinaryWriterFormats") + public void writeNullIntoNonNullableArrayThrowsTest(ClickHouseFormat format) throws Exception { + String tableName = "rowBinaryFormatWriterTest_nonNullableArrayNull_" + UUID.randomUUID().toString().replace('-', '_'); + initTable(tableName, + "CREATE TABLE \"" + tableName + "\" (id Int32, arr Array(Int32), tail Int32) Engine = MergeTree ORDER BY id", + new CommandSettings()); + TableSchema schema = client.getTableSchema(tableName); + + Exception thrown = null; + try (InsertResponse response = client.insert(tableName, out -> { + RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, schema, format); + w.setValue(schema.nameToColumnIndex("id"), 1); + w.setValue(schema.nameToColumnIndex("arr"), null); + w.setValue(schema.nameToColumnIndex("tail"), 7); + w.commitRow(); + }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { + // The insert must not succeed: a null in a non-nullable Array column is invalid. + } catch (Exception e) { + thrown = e; + } + + Assert.assertNotNull(thrown, "Expected the insert to fail for a null in a non-nullable Array column using " + format); + boolean clearMessage = false; + for (Throwable t = thrown; t != null; t = t.getCause()) { + if (t.getMessage() != null && t.getMessage().contains("An attempt to write null into not nullable column")) { + clearMessage = true; + break; + } + } + Assert.assertTrue(clearMessage, "Expected a clear non-nullable column error using " + format + ", but got: " + thrown); + } + + // Contrast: an empty (and a populated) non-nullable Array must still round-trip, and the fixed-width + // 'tail' column after it keeps its value - proving the array length is still written as a single + // leading byte and that rejecting null did not perturb valid array writes. + @Test (groups = { "integration" }, dataProvider = "rowBinaryWriterFormats") + public void writeNonNullableArrayRoundTripsTest(ClickHouseFormat format) throws Exception { + String tableName = "rowBinaryFormatWriterTest_nonNullableArrayRoundTrip_" + UUID.randomUUID().toString().replace('-', '_'); + initTable(tableName, + "CREATE TABLE \"" + tableName + "\" (id Int32, arr Array(Int32), tail Int32) Engine = MergeTree ORDER BY id", + new CommandSettings()); + TableSchema schema = client.getTableSchema(tableName); + + try (InsertResponse response = client.insert(tableName, out -> { + RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, schema, format); + w.setValue(schema.nameToColumnIndex("id"), 1); + w.setValue(schema.nameToColumnIndex("arr"), new ArrayList()); + w.setValue(schema.nameToColumnIndex("tail"), 7); + w.commitRow(); + w.setValue(schema.nameToColumnIndex("id"), 2); + w.setValue(schema.nameToColumnIndex("arr"), Arrays.asList(1, 2, 3)); + w.setValue(schema.nameToColumnIndex("tail"), 8); + w.commitRow(); + }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { + // inserted + } + + List records = client.queryAll( + "SELECT id, toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM \"" + + tableName + "\" ORDER BY id"); + Assert.assertEquals(records.size(), 2); + + GenericRecord row1 = records.get(0); + Assert.assertEquals(row1.getInteger("alen"), 0); + Assert.assertEquals(row1.getString("acat"), ""); + Assert.assertEquals(row1.getInteger("tail"), 7); + + GenericRecord row2 = records.get(1); + Assert.assertEquals(row2.getInteger("alen"), 3); + Assert.assertEquals(row2.getString("acat"), "1,2,3"); + Assert.assertEquals(row2.getInteger("tail"), 8); + } + + // Contrast: with RowBinaryWithDefaults, a null into a non-nullable Array column that HAS a DDL + // default must still fall back to that default (the null-to-default coercion is decided before the + // type dispatch), not throw - proving the fix only rejects null for non-nullable arrays with no default. + @Test (groups = { "integration" }) + public void writeNullIntoDefaultedArrayUsesDefaultTest() throws Exception { + String tableName = "rowBinaryFormatWriterTest_defaultedArrayNull_" + UUID.randomUUID().toString().replace('-', '_'); + initTable(tableName, + "CREATE TABLE \"" + tableName + "\" (id Int32, arr Array(Int32) DEFAULT [1, 2], tail Int32) Engine = MergeTree ORDER BY id", + new CommandSettings()); + TableSchema schema = client.getTableSchema(tableName); + ClickHouseFormat format = ClickHouseFormat.RowBinaryWithDefaults; + + try (InsertResponse response = client.insert(tableName, out -> { + RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, schema, format); + w.setValue(schema.nameToColumnIndex("id"), 1); + w.setValue(schema.nameToColumnIndex("arr"), null); + w.setValue(schema.nameToColumnIndex("tail"), 7); + w.commitRow(); + }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { + // inserted; the null arr must fall back to the DDL default + } + + List records = client.queryAll( + "SELECT toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM \"" + + tableName + "\" ORDER BY id"); + Assert.assertEquals(records.size(), 1); + GenericRecord row = records.get(0); + Assert.assertEquals(row.getInteger("alen"), 2); + Assert.assertEquals(row.getString("acat"), "1,2"); + Assert.assertEquals(row.getInteger("tail"), 7); + } + @Test (groups = { "integration" }) public void writeNumbersTest() throws Exception { diff --git a/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java b/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java index 8c76828ef..5d2baaefa 100644 --- a/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java @@ -38,6 +38,20 @@ public void testDynamicNullWithDefaultsWritesPreambleAndNothingTag() throws Exce Assert.assertEquals(out.toByteArray(), new byte[]{0, 0}); } + @Test + public void testDynamicNullWithoutDefaultsWritesNothingTag() throws Exception { + // A non-nullable Dynamic column can still hold a null (serialized as the implicit `Nothing` + // type), unlike a plain non-nullable Array which must reject null. This pins that behavior in + // the plain RowBinary path: no null-marker, just the single Nothing type tag. + ClickHouseColumn column = ClickHouseColumn.of("dyn", "Dynamic"); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + Assert.assertTrue(RowBinaryFormatSerializer.writeValuePreamble(out, false, column, null)); + SerializerUtils.serializeData(out, null, column); + + Assert.assertEquals(out.toByteArray(), new byte[]{0}); + } + @Test public void testSerializeNumbersRoundTrip() throws IOException { String[] names = new String[]{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", From 816f99ccf427f31e408e8e5a51b4153c46690f9f Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:28:28 +0000 Subject: [PATCH 2/3] Add DataTypeTests integration coverage for null into non-nullable Array Addresses review feedback on #2938: add integration tests in DataTypeTests for the null-into-non-nullable-Array RowBinary writer fix. These exercise the fix through the POJO insert path (client.insert -> POJOSerDe -> RowBinaryFormatSerializer.writeValuePreamble), a different entry point than the existing RowBinaryFormatWriterTest coverage: - null into a non-nullable Array now throws IllegalArgumentException, pinned in both the plain RowBinary branch (no defaults) and the RowBinaryWithDefaults branch (a sibling column carries a default); - valid empty/populated arrays still round-trip with the trailing column intact (no misframing); - a null into a non-nullable Array that HAS a DDL default still coerces to the default instead of throwing. --- .../client/datatypes/DataTypeTests.java | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) 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 b24883ca9..3d051ddbe 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 @@ -155,6 +155,110 @@ public static String tblCreateSQL(String table) { } } + // A non-nullable Array column cannot represent a null. Writing a Java null into one through the POJO + // insert path (POJOSerDe -> RowBinaryFormatSerializer.writeValuePreamble) must fail loudly like every + // other non-nullable type instead of emitting a stray marker byte, which the server read as a phantom + // extra row or a column shift. The fixed-width 'tail' column after the array would be corrupted by any + // stray byte, so it doubles as a misframing sentinel. + @Test(groups = {"integration"}, dataProvider = "nonNullableArrayNullColumns") + public void testInsertNullIntoNonNullableArrayThrows(String columns) throws Exception { + final String table = "test_non_nullable_array_null"; + client.execute("DROP TABLE IF EXISTS " + table).get(); + client.execute(tableDefinition(table, columns)).get(); + + client.register(DTOForNonNullableArrayTests.class, client.getTableSchema(table)); + + Exception thrown = null; + try { + client.insert(table, Collections.singletonList(new DTOForNonNullableArrayTests(1, null, 7))) + .get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS); + } catch (Exception e) { + thrown = e; + } + + Assert.assertNotNull(thrown, "Expected the insert to fail for a null in a non-nullable Array column using " + columns); + boolean clearMessage = false; + for (Throwable t = thrown; t != null; t = t.getCause()) { + if (t.getMessage() != null && t.getMessage().contains("An attempt to write null into not nullable column")) { + clearMessage = true; + break; + } + } + Assert.assertTrue(clearMessage, "Expected a clear non-nullable column error using " + columns + ", but got: " + thrown); + } + + @DataProvider(name = "nonNullableArrayNullColumns") + public static Object[][] nonNullableArrayNullColumns() { + return new Object[][] { + // No defaults anywhere: the POJO insert uses plain RowBinary (non-defaults preamble branch). + {"rowId Int32, arr Array(Int32), tail Int32"}, + // A sibling column has a default, so the POJO insert uses RowBinaryWithDefaults; the + // non-nullable arr itself has no default and so must still reject a null (defaults branch). + {"rowId Int32, arr Array(Int32), tail Int32 DEFAULT 99"}, + }; + } + + // Contrast: valid non-nullable arrays (empty and populated) still round-trip through the POJO insert + // path, and the trailing 'tail' column keeps its value - proving the array length is still written as a + // single leading byte and that rejecting null did not perturb valid array writes. + @Test(groups = {"integration"}, dataProvider = "nonNullableArrayRoundTrip") + public void testInsertNonNullableArrayRoundTrips(List arr, int expectedLength, String expectedConcat) throws Exception { + final String table = "test_non_nullable_array_round_trip"; + client.execute("DROP TABLE IF EXISTS " + table).get(); + client.execute(tableDefinition(table, "rowId Int32", "arr Array(Int32)", "tail Int32")).get(); + + client.register(DTOForNonNullableArrayTests.class, client.getTableSchema(table)); + client.insert(table, Collections.singletonList(new DTOForNonNullableArrayTests(1, arr, 7))) + .get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS); + + List records = client.queryAll( + "SELECT toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM " + table + " ORDER BY rowId"); + Assert.assertEquals(records.size(), 1); + GenericRecord row = records.get(0); + Assert.assertEquals(row.getInteger("alen"), expectedLength); + Assert.assertEquals(row.getString("acat"), expectedConcat); + Assert.assertEquals(row.getInteger("tail"), 7); + } + + @DataProvider(name = "nonNullableArrayRoundTrip") + public static Object[][] nonNullableArrayRoundTrip() { + return new Object[][] { + {new ArrayList(), 0, ""}, + {Arrays.asList(1, 2, 3), 3, "1,2,3"}, + }; + } + + // Contrast: with a DDL DEFAULT the write uses RowBinaryWithDefaults, where a null into the non-nullable + // Array column falls back to the default instead of throwing - proving the fix only rejects null for a + // non-nullable array that has no default. + @Test(groups = {"integration"}) + public void testInsertNullIntoDefaultedNonNullableArrayUsesDefault() throws Exception { + final String table = "test_non_nullable_array_null_default"; + client.execute("DROP TABLE IF EXISTS " + table).get(); + client.execute(tableDefinition(table, "rowId Int32", "arr Array(Int32) DEFAULT [1, 2]", "tail Int32")).get(); + + client.register(DTOForNonNullableArrayTests.class, client.getTableSchema(table)); + client.insert(table, Collections.singletonList(new DTOForNonNullableArrayTests(1, null, 7))) + .get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS); + + List records = client.queryAll( + "SELECT toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM " + table + " ORDER BY rowId"); + Assert.assertEquals(records.size(), 1); + GenericRecord row = records.get(0); + Assert.assertEquals(row.getInteger("alen"), 2); + Assert.assertEquals(row.getString("acat"), "1,2"); + Assert.assertEquals(row.getInteger("tail"), 7); + } + + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class DTOForNonNullableArrayTests { + private int rowId; + private List arr; + private int tail; + } + @Data @AllArgsConstructor @NoArgsConstructor From 63fe4ba719a22322bc2d89d521f7c674d8231865 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:06:37 +0000 Subject: [PATCH 3/3] Add jdbc-v2 array writer tests; drop byte-level unit test Addresses @chernser review on #2940 (issue #2938): - add jdbc-v2 integration tests in WriterStatementImplTest for null into a non-nullable Array via the beta RowBinary writer path (WriterStatementImpl -> RowBinaryFormatWriter -> writeValuePreamble), covering both the RowBinary and RowBinaryWithDefaults serializer branches, plus round-trip and defaulted-array contrasts - remove the SerializerUtilsTests byte-level unit test - trim narrative comments from the client-v2 array tests per AGENTS.md --- .../client/datatypes/DataTypeTests.java | 14 --- .../datatypes/RowBinaryFormatWriterTest.java | 12 -- .../client/internal/SerializerUtilsTests.java | 14 --- .../jdbc/WriterStatementImplTest.java | 109 ++++++++++++++++++ 4 files changed, 109 insertions(+), 40 deletions(-) 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 3d051ddbe..4ae1e2af4 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 @@ -155,11 +155,6 @@ public static String tblCreateSQL(String table) { } } - // A non-nullable Array column cannot represent a null. Writing a Java null into one through the POJO - // insert path (POJOSerDe -> RowBinaryFormatSerializer.writeValuePreamble) must fail loudly like every - // other non-nullable type instead of emitting a stray marker byte, which the server read as a phantom - // extra row or a column shift. The fixed-width 'tail' column after the array would be corrupted by any - // stray byte, so it doubles as a misframing sentinel. @Test(groups = {"integration"}, dataProvider = "nonNullableArrayNullColumns") public void testInsertNullIntoNonNullableArrayThrows(String columns) throws Exception { final String table = "test_non_nullable_array_null"; @@ -190,17 +185,11 @@ public void testInsertNullIntoNonNullableArrayThrows(String columns) throws Exce @DataProvider(name = "nonNullableArrayNullColumns") public static Object[][] nonNullableArrayNullColumns() { return new Object[][] { - // No defaults anywhere: the POJO insert uses plain RowBinary (non-defaults preamble branch). {"rowId Int32, arr Array(Int32), tail Int32"}, - // A sibling column has a default, so the POJO insert uses RowBinaryWithDefaults; the - // non-nullable arr itself has no default and so must still reject a null (defaults branch). {"rowId Int32, arr Array(Int32), tail Int32 DEFAULT 99"}, }; } - // Contrast: valid non-nullable arrays (empty and populated) still round-trip through the POJO insert - // path, and the trailing 'tail' column keeps its value - proving the array length is still written as a - // single leading byte and that rejecting null did not perturb valid array writes. @Test(groups = {"integration"}, dataProvider = "nonNullableArrayRoundTrip") public void testInsertNonNullableArrayRoundTrips(List arr, int expectedLength, String expectedConcat) throws Exception { final String table = "test_non_nullable_array_round_trip"; @@ -228,9 +217,6 @@ public static Object[][] nonNullableArrayRoundTrip() { }; } - // Contrast: with a DDL DEFAULT the write uses RowBinaryWithDefaults, where a null into the non-nullable - // Array column falls back to the default instead of throwing - proving the fix only rejects null for a - // non-nullable array that has no default. @Test(groups = {"integration"}) public void testInsertNullIntoDefaultedNonNullableArrayUsesDefault() throws Exception { final String table = "test_non_nullable_array_null_default"; diff --git a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java index a5776eb0c..5e2ddd2af 100644 --- a/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java +++ b/client-v2/src/test/java/com/clickhouse/client/datatypes/RowBinaryFormatWriterTest.java @@ -343,10 +343,6 @@ private Object[][] rowBinaryWriterFormats() { }; } - // A non-nullable Array column cannot represent a null, so writing a Java null into it must fail - // loudly like every other non-nullable type instead of emitting a stray marker byte on top of the - // array length. That stray byte was read by the server as a phantom extra row (single column) or - // shifted every following column (multi column). 'tail' sits after the array so a stray byte corrupts it. @Test (groups = { "integration" }, dataProvider = "rowBinaryWriterFormats") public void writeNullIntoNonNullableArrayThrowsTest(ClickHouseFormat format) throws Exception { String tableName = "rowBinaryFormatWriterTest_nonNullableArrayNull_" + UUID.randomUUID().toString().replace('-', '_'); @@ -363,7 +359,6 @@ public void writeNullIntoNonNullableArrayThrowsTest(ClickHouseFormat format) thr w.setValue(schema.nameToColumnIndex("tail"), 7); w.commitRow(); }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { - // The insert must not succeed: a null in a non-nullable Array column is invalid. } catch (Exception e) { thrown = e; } @@ -379,9 +374,6 @@ public void writeNullIntoNonNullableArrayThrowsTest(ClickHouseFormat format) thr Assert.assertTrue(clearMessage, "Expected a clear non-nullable column error using " + format + ", but got: " + thrown); } - // Contrast: an empty (and a populated) non-nullable Array must still round-trip, and the fixed-width - // 'tail' column after it keeps its value - proving the array length is still written as a single - // leading byte and that rejecting null did not perturb valid array writes. @Test (groups = { "integration" }, dataProvider = "rowBinaryWriterFormats") public void writeNonNullableArrayRoundTripsTest(ClickHouseFormat format) throws Exception { String tableName = "rowBinaryFormatWriterTest_nonNullableArrayRoundTrip_" + UUID.randomUUID().toString().replace('-', '_'); @@ -420,9 +412,6 @@ public void writeNonNullableArrayRoundTripsTest(ClickHouseFormat format) throws Assert.assertEquals(row2.getInteger("tail"), 8); } - // Contrast: with RowBinaryWithDefaults, a null into a non-nullable Array column that HAS a DDL - // default must still fall back to that default (the null-to-default coercion is decided before the - // type dispatch), not throw - proving the fix only rejects null for non-nullable arrays with no default. @Test (groups = { "integration" }) public void writeNullIntoDefaultedArrayUsesDefaultTest() throws Exception { String tableName = "rowBinaryFormatWriterTest_defaultedArrayNull_" + UUID.randomUUID().toString().replace('-', '_'); @@ -439,7 +428,6 @@ public void writeNullIntoDefaultedArrayUsesDefaultTest() throws Exception { w.setValue(schema.nameToColumnIndex("tail"), 7); w.commitRow(); }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { - // inserted; the null arr must fall back to the DDL default } List records = client.queryAll( diff --git a/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java b/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java index 5d2baaefa..8c76828ef 100644 --- a/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/internal/SerializerUtilsTests.java @@ -38,20 +38,6 @@ public void testDynamicNullWithDefaultsWritesPreambleAndNothingTag() throws Exce Assert.assertEquals(out.toByteArray(), new byte[]{0, 0}); } - @Test - public void testDynamicNullWithoutDefaultsWritesNothingTag() throws Exception { - // A non-nullable Dynamic column can still hold a null (serialized as the implicit `Nothing` - // type), unlike a plain non-nullable Array which must reject null. This pins that behavior in - // the plain RowBinary path: no null-marker, just the single Nothing type tag. - ClickHouseColumn column = ClickHouseColumn.of("dyn", "Dynamic"); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - - Assert.assertTrue(RowBinaryFormatSerializer.writeValuePreamble(out, false, column, null)); - SerializerUtils.serializeData(out, null, column); - - Assert.assertEquals(out.toByteArray(), new byte[]{0}); - } - @Test public void testSerializeNumbersRoundTrip() throws IOException { String[] names = new String[]{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/WriterStatementImplTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/WriterStatementImplTest.java index 2762c76c6..1bf2734be 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/WriterStatementImplTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/WriterStatementImplTest.java @@ -1,13 +1,21 @@ package com.clickhouse.jdbc; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.sql.Connection; import java.sql.JDBCType; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Statement; +import java.sql.Types; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Properties; +import java.util.UUID; @Test(groups = {"integration"}) public class WriterStatementImplTest extends JdbcIntegrationTest { @@ -28,4 +36,105 @@ public void testTargetTypeMethodThrowException() throws SQLException { Assert.expectThrows(SQLException.class, () -> stmt.setObject(1, "", JDBCType.DECIMAL, 3)); } } + + @DataProvider(name = "nonNullableArrayNullTables") + public static Object[][] nonNullableArrayNullTables() { + return new Object[][] { + {"id Int32, arr Array(Int32), tail Int32"}, + {"id Int32, arr Array(Int32), tail Int32 DEFAULT 99"}, + }; + } + + @Test(groups = {"integration"}, dataProvider = "nonNullableArrayNullTables") + public void testWriterNullIntoNonNullableArrayThrows(String columns) throws Exception { + String table = "writer_stmt_arr_null_" + UUID.randomUUID().toString().replace('-', '_'); + runQuery("DROP TABLE IF EXISTS " + table); + runQuery("CREATE TABLE " + table + " (" + columns + ") Engine = MergeTree ORDER BY id"); + + Properties properties = new Properties(); + properties.setProperty(DriverProperties.BETA_ROW_BINARY_WRITER.getKey(), "true"); + try (Connection connection = getJdbcConnection(properties); + PreparedStatement stmt = connection.prepareStatement("INSERT INTO " + table + " VALUES (?, ?, ?)")) { + Assert.assertTrue(stmt instanceof WriterStatementImpl); + stmt.setInt(1, 1); + stmt.setNull(2, Types.ARRAY); + stmt.setInt(3, 7); + + SQLException thrown = Assert.expectThrows(SQLException.class, stmt::execute); + Assert.assertTrue(hasNonNullableColumnMessage(thrown), + "Expected a clear non-nullable column error using " + columns + ", but got: " + thrown); + } + } + + @DataProvider(name = "nonNullableArrayRoundTrip") + public static Object[][] nonNullableArrayRoundTrip() { + return new Object[][] { + {new ArrayList(), 0, ""}, + {Arrays.asList(1, 2, 3), 3, "1,2,3"}, + }; + } + + @Test(groups = {"integration"}, dataProvider = "nonNullableArrayRoundTrip") + public void testWriterNonNullableArrayRoundTrips(List arr, int expectedLength, String expectedConcat) throws Exception { + String table = "writer_stmt_arr_round_trip_" + UUID.randomUUID().toString().replace('-', '_'); + runQuery("DROP TABLE IF EXISTS " + table); + runQuery("CREATE TABLE " + table + " (id Int32, arr Array(Int32), tail Int32) Engine = MergeTree ORDER BY id"); + + Properties properties = new Properties(); + properties.setProperty(DriverProperties.BETA_ROW_BINARY_WRITER.getKey(), "true"); + try (Connection connection = getJdbcConnection(properties); + PreparedStatement stmt = connection.prepareStatement("INSERT INTO " + table + " VALUES (?, ?, ?)")) { + stmt.setInt(1, 1); + stmt.setObject(2, arr); + stmt.setInt(3, 7); + stmt.execute(); + } + + try (Connection connection = getJdbcConnection(); + Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM " + + table + " ORDER BY id")) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getInt("alen"), expectedLength); + Assert.assertEquals(rs.getString("acat"), expectedConcat); + Assert.assertEquals(rs.getInt("tail"), 7); + Assert.assertFalse(rs.next()); + } + } + + @Test(groups = {"integration"}) + public void testWriterNullIntoDefaultedArrayUsesDefault() throws Exception { + String table = "writer_stmt_arr_null_default_" + UUID.randomUUID().toString().replace('-', '_'); + runQuery("DROP TABLE IF EXISTS " + table); + runQuery("CREATE TABLE " + table + " (id Int32, arr Array(Int32) DEFAULT [1, 2], tail Int32) Engine = MergeTree ORDER BY id"); + + Properties properties = new Properties(); + properties.setProperty(DriverProperties.BETA_ROW_BINARY_WRITER.getKey(), "true"); + try (Connection connection = getJdbcConnection(properties); + PreparedStatement stmt = connection.prepareStatement("INSERT INTO " + table + " VALUES (?, ?, ?)")) { + stmt.setInt(1, 1); + stmt.setNull(2, Types.ARRAY); + stmt.setInt(3, 7); + stmt.execute(); + } + + try (Connection connection = getJdbcConnection(); + Statement stmt = connection.createStatement(); + ResultSet rs = stmt.executeQuery("SELECT toInt32(length(arr)) AS alen, arrayStringConcat(arr, ',') AS acat, tail FROM " + + table + " ORDER BY id")) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getInt("alen"), 2); + Assert.assertEquals(rs.getString("acat"), "1,2"); + Assert.assertEquals(rs.getInt("tail"), 7); + } + } + + private static boolean hasNonNullableColumnMessage(Throwable thrown) { + for (Throwable t = thrown; t != null; t = t.getCause()) { + if (t.getMessage() != null && t.getMessage().contains("An attempt to write null into not nullable column")) { + return true; + } + } + return false; + } }