search

ClickHouse / clickhouse-java

ClickHouse Java Clients & JDBC Driver
https://clickhouse.com
Apache License 2.0
1.43k stars 529 forks source link

Negative bigDecimal (128/256) doesn't work correctly. #665

Closed UnamedRus closed 3 years ago

UnamedRus commented 3 years ago

Currently zero bytes is being used for filling, which isn't correct for negative numbers. Decimal128 https://github.com/ClickHouse/clickhouse-jdbc/blob/094ed0b9d2dd8a18ae0c7b3f8f22c35e595822a6/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStream.java#L197

Decimal256 https://github.com/ClickHouse/clickhouse-jdbc/blob/094ed0b9d2dd8a18ae0c7b3f8f22c35e595822a6/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/util/ClickHouseRowBinaryStream.java#L206

SELECT
    hex(reinterpretAsString(-860 * 10000000)) AS init,
    hex(reinterpretAsString(CAST('109091.1627776', 'Decimal128(7)'))) AS dec1,
    hex(reinterpretAsString(CAST('-860', 'Decimal128(7)'))) AS dec

Query id: 2e2fdb71-34c1-47ff-8cd3-6b745247c97c

┌─init─────────────┬─dec1───────┬─dec──────────────────────────────┐
│ 006A66FFFDFFFFFF │ 006A66FFFD │ 006A66FFFDFFFFFFFFFFFFFFFFFFFFFF │
└──────────────────┴────────────┴──────────────────────────────────┘

SELECT
    hex(reinterpretAsString(860 * 10000000)) AS init,
    hex(reinterpretAsString(CAST('109091.1627776', 'Decimal128(7)'))) AS dec1,
    hex(reinterpretAsString(CAST('860', 'Decimal128(7)'))) AS dec

Query id: 6f48970c-20a2-4e17-a70f-7d15c85c728e

┌─init───────┬─dec1───────┬─dec────────┐
│ 0096990002 │ 006A66FFFD │ 0096990002 │
└────────────┴────────────┴────────────┘

It does looks like that correct handling of negative numbers is done in clickhouse-jdbc-bridge

https://github.com/ClickHouse/clickhouse-jdbc-bridge/blob/a462e07f22101fac3c1de388d3e80dc59e6e9d85/src/main/java/ru/yandex/clickhouse/jdbcbridge/core/ByteBuffer.java#L398

zhicwu commented 3 years ago

Thanks @UnamedRus. Will fix this in the coming weekend.