[client-v2] Out-of-range numeric values are silently wrapped when inserted into Int32/Int64/UInt64 columns (Int8/Int16 correctly throw)

Đang mở
#3,176 4 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
74/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
java
Lĩnh vực
api, databases

Hướng nghiên cứu

Start in client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java at serializePrimitiveData, then inspect convertToInteger, convertToLong, and NumberConverter.toBigInteger in NumberConverter.java. Use the supplied NarrowingTest reproduction to verify that out-of-range Int32, Int64, UInt32, UInt64, and wider integer values are rejected while in-range values remain unchanged.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Describe the bug

SerializerUtils.serializePrimitiveData narrows the user-supplied value to a Java primitive without a range check for some integer column types. When the value does not fit, the extra bits are dropped silently and a wrong number is stored — no exception is raised and nothing is logged.

The behaviour is inconsistent across widths:

Column type Conversion used Out-of-range behaviour
Int8, Int16, UInt8, UInt16 convertToInteger → BinaryStreamUtils.writeInt8/16/... Correct — ClickHouseChecker.between throws IllegalArgumentException
Int32 convertToInteger(value) → ((Number) value).intValue() Silently wraps
Int64, UInt32 convertToLong(value) → ((Number) value).longValue() Silently wraps (Int64)
UInt64, Int128, UInt128, Int256, UInt256 NumberConverter.toBigInteger(value) → BigInteger.valueOf(((Number) value).longValue()) Silently wraps

The module even has a correct helper already — NumberConverter.toInt / toLong compare intValue() against longValue() and throw ArithmeticException("integer overflow: ...") — but serializePrimitiveData does not call it for these branches.

The server rejects such a narrowing (SELECT toInt32(toDecimal64(4294967296, 0)) → DECIMAL_OVERFLOW), so the client is silently more permissive than the server and corrupts data.

ClickHouse server version

26.9.7.9 (local server at http://localhost:8123). The test below was run against it.

Reproduction

TestNG test in client-v2:

package com.clickhouse.client;

import com.clickhouse.client.api.Client;
import com.clickhouse.client.api.data_formats.RowBinaryFormatWriter;
import com.clickhouse.client.api.insert.InsertResponse;
import com.clickhouse.client.api.insert.InsertSettings;
import com.clickhouse.client.api.metadata.TableSchema;
import com.clickhouse.client.api.query.GenericRecord;
import com.clickhouse.data.ClickHouseFormat;
import org.testng.annotations.Test;

import java.math.BigDecimal;

public class NarrowingTest {

    private static String root(Throwable t) {
        while (t.getCause() != null) t = t.getCause();
        return t.getClass().getSimpleName() + ": " + t.getMessage();
    }

    @Test
    public void narrowing() throws Exception {
        Client client = new Client.Builder()
                .addEndpoint("http://localhost:8123")
                .setUsername("default").setPassword("")
                .setDefaultDatabase("default")
                .compressServerResponse(false).compressClientRequest(false)
                .build();

        client.execute("DROP TABLE IF EXISTS narrow32").get();
        client.execute("CREATE TABLE narrow32 (id UInt8, v Int32) ENGINE = Memory").get();

        Object[][] rows = {
                {(short) 1, new BigDecimal("100000")},      // in range
                {(short) 2, new BigDecimal("4294967296")},  // 2^32, out of Int32 range
                {(short) 3, 4294967296L},                   // same, as a Long
                {(short) 4, new BigDecimal("2147483648")},  // Int32 max + 1
        };

        TableSchema schema = client.getTableSchema("narrow32");
        ClickHouseFormat format = ClickHouseFormat.RowBinary;
        for (Object[] row : rows) {
            try (InsertResponse r = client.insert("narrow32", out -> {
                RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, schema, format);
                for (int i = 0; i < row.length; i++) w.setValue(i + 1, row[i]);
                w.commitRow();
            }, format, new InsertSettings()).get()) {
                System.out.println("accepted id=" + row[0] + " value=" + row[1]);
            } catch (Exception e) {
                System.out.println("rejected id=" + row[0] + " value=" + row[1] + " -> " + root(e));
            }
        }
        for (GenericRecord rec : client.queryAll("SELECT id, v FROM narrow32 ORDER BY id")) {
            System.out.println("STORED id=" + rec.getInteger(1) + " v=" + rec.getLong(2));
        }

        // Contrast: Int16 is range-checked and rejects the same kind of value
        client.execute("DROP TABLE IF EXISTS narrow16").get();
        client.execute("CREATE TABLE narrow16 (id UInt8, v Int16) ENGINE = Memory").get();
        TableSchema s16 = client.getTableSchema("narrow16");
        try (InsertResponse r = client.insert("narrow16", out -> {
            RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, s16, format);
            w.setValue(1, (short) 1);
            w.setValue(2, new BigDecimal("40000"));
            w.commitRow();
        }, format, new InsertSettings()).get()) {
            System.out.println("Int16 40000 accepted");
        } catch (Exception e) {
            System.out.println("Int16 40000 -> " + root(e));
        }

        // 64-bit columns have the same defect
        client.execute("DROP TABLE IF EXISTS narrow64").get();
        client.execute("CREATE TABLE narrow64 (id UInt8, a Int64, b UInt64) ENGINE = Memory").get();
        TableSchema s64 = client.getTableSchema("narrow64");
        try (InsertResponse r = client.insert("narrow64", out -> {
            RowBinaryFormatWriter w = new RowBinaryFormatWriter(out, s64, format);
            w.setValue(1, (short) 1);
            w.setValue(2, new BigDecimal("18446744073709551616")); // 2^64
            w.setValue(3, new BigDecimal("18446744073709551616")); // 2^64
            w.commitRow();
        }, format, new InsertSettings()).get()) {
            System.out.println("Int64/UInt64 2^64 accepted");
        } catch (Exception e) {
            System.out.println("Int64/UInt64 2^64 -> " + root(e));
        }
        for (GenericRecord rec : client.queryAll("SELECT id, toString(a), toString(b) FROM narrow64")) {
            System.out.println("STORED64 id=" + rec.getInteger(1) + " a=" + rec.getString(2) + " b=" + rec.getString(3));
        }
        client.close();
    }
}
Actual output (observed, main at 74b0a62-era checkout, 0.11.0-rc1-SNAPSHOT)
accepted id=1 value=100000
accepted id=2 value=4294967296
accepted id=3 value=4294967296
accepted id=4 value=2147483648
STORED id=1 v=100000
STORED id=2 v=0
STORED id=3 v=0
STORED id=4 v=-2147483648
Int16 40000 -> IllegalArgumentException: int(40000) should be between -32768 and 32767 inclusive of both values
Int64/UInt64 2^64 accepted
STORED64 id=1 a=0 b=0
Expected output

Rows 2, 3 and 4 should be rejected the same way the Int16 row is (an ArithmeticException/IllegalArgumentException about integer overflow), and the Int64/UInt64 row with 2^64 should be rejected too. The in-range row (100000 → 100000) is already correct and must stay correct.

Instead 4294967296 becomes 0, 2147483648 becomes -2147483648, and 2^64 becomes 0 in both the Int64 and the UInt64 column — silently.

Suggested fix

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

  • serializePrimitiveData (around lines 649-685): use the already-existing range-checking helpers NumberConverter.toInt(value) for Int32, NumberConverter.toLong(value) for Int64, and a checked conversion for UInt32/UInt64 instead of convertToInteger / convertToLong.
  • convertToInteger (around line 1050) and convertToLong (around line 1064): the bare ((Number) value).intValue() / longValue() is where the bits are dropped. Either add the overflow check here or stop using these for the fixed-width integer branches.
  • NumberConverter.toBigInteger (around line 117 of NumberConverter.java): BigInteger.valueOf(((Number) value).longValue()) loses the high bits of a BigDecimal/BigInteger argument; it should use ((BigDecimal) value).toBigIntegerExact() for BigDecimal and pass a BigInteger through unchanged, then let the writeUnsignedInt64/writeInt128/... range checks apply.
Link

Found while checking whether https://github.com/ClickHouse/clickhouse-cs/issues/646 (unchecked narrowing in ClickHouseDecimal's IConvertible members) also affects this client. The two C#-specific defects in that report (the (short) typo in ToInt32, and the Convert.ChangeType stack overflow) have no counterpart here — clickhouse-java uses java.math.BigDecimal and has no IConvertible/Convert.ChangeType equivalent. The third defect — out-of-range narrowing wrapping instead of raising an overflow error — does reproduce here, at Int32/Int64/UInt64 rather than at Int8/Int16.

Tracking: ClickHouse/integrations-ai-playground#532

Ngôn ngữ chính
Java
Star
1.6k
Fork
649
Merge trung bình
1 ngày 20 giờ
Pull request đã merge (30 ngày)
39

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của ClickHouse/clickhouse-java

Tất cả issue của ClickHouse/clickhouse-java

Issue tương tự

Thêm issue về Java

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.