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

Aberta
#3,176 4 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 1 dia

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
3/5
Tempo estimado
1-2 dias
Facilidade para iniciantes
74/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
java
Domínio
api, databases

Direção de pesquisa

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.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

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

Linguagem predominante
Java
Estrelas
1.6k
Forks
649
Merge médio
1d 20h
PRs com merge (30d)
39

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de ClickHouse/clickhouse-java

Todas as issues de ClickHouse/clickhouse-java

Issues semelhantes

Mais issues de Java

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.