diff --git a/CHANGELOG.md b/CHANGELOG.md index c21932f08..c4f9aabad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ a `String`/`Boolean`) through `Number#floatValue()`/`Number#doubleValue()`, so the float columns accept the same value types the integer columns already did. (https://github.com/ClickHouse/clickhouse-java/issues/2930) +- **[client-v2]** Fixed a `NullPointerException` when serializing a `null` value into a non-nullable + `Enum8`/`Enum16` column. `SerializerUtils.serializeEnumData` had no `null` guard, so a `null` in a + non-nullable enum column reached `value.getClass()` and failed the RowBinary insert path with a confusing + 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 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/internal/SerializerUtils.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java index ef2da8ea2..e00438f12 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 @@ -709,6 +709,10 @@ private static void serializeTime64(OutputStream stream, Object value) throws IO } public static void serializeEnumData(OutputStream stream, ClickHouseColumn column, Object value) throws IOException { + if (value == null) { + throw new IllegalArgumentException("Cannot write NULL into non-nullable Enum column " + column.getColumnName() + + " of type " + column.getOriginalTypeName()); + } int enumValue = -1; if (value instanceof String) { enumValue = column.getEnumConstants().value((String) value); 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 265cd1cfb..9574bed72 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 @@ -143,6 +143,44 @@ public void testGeometrySerializationRejectsMalformedList() { ClickHouseColumn.of("v", "Geometry"))); } + @Test(dataProvider = "nonNullableEnumTypes") + public void testNullIntoNonNullableEnumThrowsIllegalArgument(String typeName) { + ClickHouseColumn column = ClickHouseColumn.of("bs_flag", typeName); + + IllegalArgumentException ex = Assert.expectThrows(IllegalArgumentException.class, + () -> SerializerUtils.serializeData(new ByteArrayOutputStream(), null, column)); + String message = ex.getMessage(); + Assert.assertTrue(message.contains("Cannot write NULL into non-nullable Enum column"), + "Unexpected message: " + message); + Assert.assertTrue(message.contains("bs_flag"), + "Message should name the offending column: " + message); + Assert.assertTrue(message.contains(typeName), + "Message should include the enum type: " + message); + } + + @DataProvider(name = "nonNullableEnumTypes") + private Object[][] nonNullableEnumTypes() { + return new Object[][] { + {"Enum8('B' = 1, 'S' = 2)"}, + {"Enum16('B' = 1, 'S' = 2)"}, + }; + } + + @Test + public void testEnumSerializationUnaffectedByNullGuard() throws Exception { + // A Nullable(Enum) with null still takes the early null-marker path and never reaches + // enum serialization, so a single null-marker byte is written. + ByteArrayOutputStream nullableOut = new ByteArrayOutputStream(); + SerializerUtils.serializeData(nullableOut, null, + ClickHouseColumn.of("v", "Nullable(Enum8('B' = 1, 'S' = 2))")); + Assert.assertEquals(nullableOut.toByteArray(), new byte[] {1}); + + // A present value in a non-nullable Enum column still serializes to its mapped numeric value. + ByteArrayOutputStream valueOut = new ByteArrayOutputStream(); + SerializerUtils.serializeData(valueOut, "S", ClickHouseColumn.of("v", "Enum8('B' = 1, 'S' = 2)")); + Assert.assertEquals(valueOut.toByteArray(), new byte[] {2}); + } + @Test(dataProvider = "nestedNullableData") public void testNestedNullableRoundTrip(String typeName, Object value) throws Exception { ClickHouseColumn column = ClickHouseColumn.of("v", typeName); 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 1a0ee2287..333feaa5f 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 @@ -17,7 +17,9 @@ import com.clickhouse.data.ClickHouseFormat; import com.clickhouse.data.ClickHouseVersion; import org.apache.commons.lang3.RandomStringUtils; +import org.testng.Assert; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.IOException; @@ -254,6 +256,86 @@ public void writeMissingFieldsTest() throws Exception { } + @Test (groups = { "integration" }) + public void writeEnumZeroLikeValuesTest() throws Exception { + String tableName = "rowBinaryFormatWriterTest_enumZeroLike_" + UUID.randomUUID().toString().replace('-', '_'); + String tableCreate = "CREATE TABLE \"" + tableName + "\" " + + " (id Int32, " + + " e8 Enum8('' = 0, 'a' = 1, 'neg' = -5), " + + " e16 Enum16('zero' = 0, 'big' = 30000, 'nb' = -20000), " + + " tail Float64" + + " ) Engine = MergeTree ORDER BY id"; + + Field[][] rows = new Field[][] { + // Zero-like written by enum name: an empty-string name and a named zero both map to 0. + {new Field("id", 1), new Field("e8", ""), new Field("e16", "zero"), new Field("tail", 1.5)}, + // The same zero-like members written by their underlying int 0. + {new Field("id", 2), new Field("e8", 0).set(""), new Field("e16", 0).set("zero"), new Field("tail", 2.5)}, + // Negative members (Enum8/Enum16 are signed) written by name. + {new Field("id", 3), new Field("e8", "neg"), new Field("e16", "nb"), new Field("tail", 3.5)}, + // The same negative members written by their underlying int. + {new Field("id", 4), new Field("e8", -5).set("neg"), new Field("e16", -20000).set("nb"), new Field("tail", 4.5)}, + // Ordinary positive members. + {new Field("id", 5), new Field("e8", "a"), new Field("e16", "big"), new Field("tail", 5.5)}, + }; + + writeTest(tableName, tableCreate, rows); + } + + + // A null element in a non-nullable Enum sub-column reaches serializeEnumData directly (no + // per-element null preamble is written for non-nullable nested columns), which is the path + // that used to throw an opaque NullPointerException. Covers every container seam that routes + // an element through serializeEnumData: Array (level 1), Tuple, Map value, and a nested Array. + @DataProvider(name = "nullEnumContainers") + private Object[][] nullEnumContainers() { + return new Object[][] { + {"Array(Enum8('a' = 1, 'b' = 2))", Arrays.asList("a", null)}, + {"Tuple(Enum8('a' = 1, 'b' = 2), Int32)", Arrays.asList(null, 7)}, + {"Map(String, Enum16('a' = 1, 'b' = 2))", singleEntryMap("k", null)}, + {"Array(Array(Enum8('a' = 1, 'b' = 2)))", Arrays.asList(Arrays.asList("a", null))}, + }; + } + + @Test (groups = { "integration" }, dataProvider = "nullEnumContainers") + public void writeNullEnumInContainerThrowsTest(String columnType, Object valueWithNullEnum) throws Exception { + String tableName = "rowBinaryFormatWriterTest_enumContainerNull_" + UUID.randomUUID().toString().replace('-', '_'); + initTable(tableName, + "CREATE TABLE \"" + tableName + "\" (id Int32, c " + columnType + ") Engine = MergeTree ORDER BY id", + new CommandSettings()); + TableSchema schema = client.getTableSchema(tableName); + ClickHouseFormat format = ClickHouseFormat.RowBinaryWithDefaults; + + 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("c"), valueWithNullEnum); + w.commitRow(); + }, format, settings).get(EXECUTE_CMD_TIMEOUT, TimeUnit.SECONDS)) { + // The insert must not succeed: a null Enum element in a non-nullable container is invalid. + } catch (Exception e) { + thrown = e; + } + + Assert.assertNotNull(thrown, "Expected the insert to fail for a null Enum element in " + columnType); + boolean clearMessage = false; + for (Throwable t = thrown; t != null; t = t.getCause()) { + if (t.getMessage() != null && t.getMessage().contains("Cannot write NULL into non-nullable Enum column")) { + clearMessage = true; + break; + } + } + Assert.assertTrue(clearMessage, "Expected a clear non-nullable Enum error for " + columnType + ", but got: " + thrown); + } + + private static Map singleEntryMap(String key, Object value) { + Map map = new HashMap<>(); + map.put(key, value); + return map; + } + + @Test (groups = { "integration" }) public void writeNumbersTest() throws Exception { String tableName = "rowBinaryFormatWriterTest_writeNumbersTest_" + UUID.randomUUID().toString().replace('-', '_'); diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java index a82485210..a3b46496b 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/JdbcDataTypeTests.java @@ -1349,6 +1349,68 @@ public void testStringTypes() throws SQLException { } } + @Test(groups = { "integration" }) + public void testEnumZeroLikeValues() throws SQLException { + runQuery("DROP TABLE IF EXISTS test_enum_zero_like"); + runQuery("CREATE TABLE test_enum_zero_like (order Int8, " + + "e8 Enum8('' = 0, 'a' = 1, 'neg' = -5), e16 Enum16('zero' = 0, 'big' = 30000, 'nb' = -20000)" + + ") ENGINE = MergeTree ORDER BY ()"); + + try (Connection conn = getJdbcConnection()) { + try (PreparedStatement stmt = conn.prepareStatement("INSERT INTO test_enum_zero_like VALUES ( ?, ?, ? )")) { + // Zero-like written by name: an empty-string name and a named zero both map to 0. + stmt.setInt(1, 1); + stmt.setString(2, ""); + stmt.setString(3, "zero"); + stmt.addBatch(); + // The same zero-like members written by their underlying int 0. + stmt.setInt(1, 2); + stmt.setInt(2, 0); + stmt.setInt(3, 0); + stmt.addBatch(); + // Negative members (Enum8/Enum16 are signed) written by name. + stmt.setInt(1, 3); + stmt.setString(2, "neg"); + stmt.setString(3, "nb"); + stmt.addBatch(); + // The same negative members written by their underlying int. + stmt.setInt(1, 4); + stmt.setInt(2, -5); + stmt.setInt(3, -20000); + stmt.addBatch(); + stmt.executeBatch(); + } + } + + try (Connection conn = getJdbcConnection()) { + try (Statement stmt = conn.createStatement()) { + try (ResultSet rs = stmt.executeQuery("SELECT * FROM test_enum_zero_like ORDER BY order")) { + assertTrue(rs.next()); + assertEquals(rs.getString("e8"), ""); + assertEquals(rs.getInt("e8"), 0); + assertEquals(rs.getString("e16"), "zero"); + assertEquals(rs.getInt("e16"), 0); + assertTrue(rs.next()); + assertEquals(rs.getString("e8"), ""); + assertEquals(rs.getInt("e8"), 0); + assertEquals(rs.getString("e16"), "zero"); + assertEquals(rs.getInt("e16"), 0); + assertTrue(rs.next()); + assertEquals(rs.getString("e8"), "neg"); + assertEquals(rs.getInt("e8"), -5); + assertEquals(rs.getString("e16"), "nb"); + assertEquals(rs.getInt("e16"), -20000); + assertTrue(rs.next()); + assertEquals(rs.getString("e8"), "neg"); + assertEquals(rs.getInt("e8"), -5); + assertEquals(rs.getString("e16"), "nb"); + assertEquals(rs.getInt("e16"), -20000); + assertFalse(rs.next()); + } + } + } + } + @Test(groups = { "integration" }) public void testIpAddressTypes() throws SQLException, UnknownHostException { runQuery("CREATE TABLE test_ips (order Int8, "