ClickHouse / ClickHouse/clickhouse-java
client-v2: serializePrimitiveData rejects or silently corrupts String values for UInt64, UUID, Date and DateTime
- 主要言語
- Java
- スター
- 1.6k
- フォーク
- 636
- 平均マージ
- 2日 23時間
- マージ済み PR(30日)
- 29
説明
## Summary
`SerializerUtils.serializePrimitiveData` accepts `String` input for most scalar column types via the `convertToInteger` / `convertToLong` / `convertToBigInteger` / `convertToBigDecimal` / `convertToString` helpers, but a subset of types either reject a `String` outright or — for `UInt64` — silently corrupt it. This makes String coercion inconsistent across the type switch.
Line references are against `0.9.5`, file `client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java`.
## Behaviour today
| Column type | String input | Result |
|---|---|---|
| `Int8/16/32`, `UInt8/16`, `Int64`, `UInt32` | `convertToInteger` / `convertToLong` | works |
| `Int128/256`, `UInt128/256` | `convertToBigInteger` | works |
| `Decimal*` | `convertToBigDecimal` | works |
| `String`, `FixedString` | `convertToString` | works |
| **`UInt64`** (`:522`) | `convertToLong` → `Long.parseLong` (`:760`) | **throws above 2^63-1; silently wraps a negative** |
| **`UUID`** (`:575`) | `(UUID) value` | `ClassCastException` |
| **`Date`, `Date32`** (`:553`, `:556`) | `writeDate` / `writeDate32` (`:1105`) | `IllegalArgumentException: Cannot convert … to Long` |
| **`DateTime`, `DateTime64`** (`:559`, `:563`) | `writeDateTime` / `writeDateTime64` (`:1141`) | same |
### The `UInt64` case is a silent-corruption bug
```java
convertToLong("-1") // -> -1L (Long.parseLong, signed)
writeUnsignedInt64(-1L) // -> writeInt64 -> FF FF FF FF FF FF FF FF
```
ClickHouse reads that back as `18446744073709551615`. So a negative String is written as the maximum `UInt64` with no error.
This is already inconsistent within the library: `BinaryStreamUtils.writeUnsignedInt64(OutputStream, BigInteger)` range-checks via `ClickHouseChecker.between(value, TYPE_BIG_INTEGER, BigInteger.ZERO, U_INT64_MAX)`, so a `BigInteger` of `-1` is correctly rejected. Only the String path wraps.
Values in `[2^63, 2^64-1]` are legal `UInt64` values and are encodable on the wire (`writeUnsignedInt64(long)` just emits the raw 8 bytes), but unreachable through the String path.
## Proposed fix
1. **`UInt64`** — use an unsigned-aware conversion at `:522` rather than changing the shared `convertToLong`, which `Int64` (`:504`) and `UInt32` (`:519`) also use and which needs signed parsing:
```java
/** UInt64 runs past Long.MAX_VALUE; the returned long carries the raw bit pattern. */
public static Long convertToUnsignedLong(Object value) {
if (value instanceof String) {
return Long.parseUnsignedLong((String) value);
}
return convertToLong(value);
}
```
2. **`UUID`** — add a `String` branch using `UUID.fromString`.
3. **`Date` / `Date32` / `DateTime` / `DateTime64`** — add a `String` branch in the `writeDate` / `writeDate32` / `writeDateTime` / `writeDateTime64` helpers, which already dispatch on `LocalDate` / `ZonedDateTime` / `OffsetDateTime` / `Timestamp`. Fixing them there also covers their other callers.
### Backward compatibility
Items 2 and 3 are purely additive: those inputs throw today, so nothing that currently succeeds changes behaviour.
Item 1 has one deliberate behaviour change: `"-1"` into a `UInt64` column currently writes `18446744073709551615` and would now throw `NumberFormatException`. That replaces silent corruption with a loud error and aligns the String path with the existing `BigInteger` overload. Suggest keeping it as its own commit so it can be reviewed separately from the additive branches.
The `Long` / `Number` path stays permissive and passes raw bits through unchanged — callers legitimately supply the bit pattern for values above 2^63.
## Why it matters
Reported from the ClickHouse Flink connector. Its checkpoint state format serialises map keys as strings, so for a `Map(K, V)` column the key reaches `serializeMapData` (`:478`) → `serializePrimitiveData` as a `String`. The connector currently has to reject `Map(UInt64, …)`, `Map(UUID, …)`, `Map(Date, …)`, `Map(Date32, …)` and `Map(DateTime(64), …)` at planning time and tell users to change their table DDL, even though all of these are legal ClickHouse map key types.
More generally, any caller passing a `String` value for one of these types — nested or top-level — hits the same behaviour.
コントリビューションガイド
調査の方向性
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java の serializePrimitiveData から始め、UInt64、UUID、日付、日時のヘルパーパスをたどってください。既存の変換動作と、BinaryStreamUtils に関連する unsigned long の範囲チェックを確認してください。これらの型について String 値が一貫して処理され、無効な UInt64 値が暗黙的にラップされずに失敗し、既存の signed または数値のパスが変更されないことが完了の条件です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- java
- 領域
- databases
- issue の種類
- バグ
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 活発さ
- 活発
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 82/100