Merge pull request #2932 from ClickHouse/polyglot/enum-null-non-nullable-npe

fix(client-v2): clear error for null in non-nullable Enum column

Author: chernser

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

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);

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);

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<String, Object> singleEntryMap(String key, Object value) {
+        Map<String, Object> 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('-', '_');

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, "