ClickHouse / ClickHouse/ClickHouse
Typed DateTime64 JSON leaf sent to a shard as a bare number: rounded via Float64
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
### Describe what's wrong
**A distributed query whose constant is a `JSON` value with a typed `DateTime64` path returns a different instant on the shard than locally. With `DateTime64(9)`, `2023-10-29 01:30:00.123456789` comes back as `...123456700` (89 ns off). With `compatibility = '26.7'` (or `input_format_read_datetime_number_as_raw_value = 1`), a whole-second `DateTime64(3)` value comes back divided by the scale: `1698543000` instead of `1698543000000`, i.e. 1970-01-20 instead of 2023-10-29.**
- **Root cause:** [`src/Analyzer/Utils.cpp:1405`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1405) renders a typed `DateTime64` JSON leaf as a bare number. The added comment asserts this "round-trips losslessly" through `SerializationDateTime64::deserializeTextJSON`, but the `JSON` data type does not use that serialization: typed paths go through `JSONExtractTree`, where a fractional number has already been rounded to `Float64` by the DOM parser (acknowledged at [`src/Formats/JSONExtractTree.cpp:995`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Formats/JSONExtractTree.cpp#L995)) and a bare integer is read as ticks under the legacy setting. The pre-PR quoted-text form parsed exactly in both cases.
Analysis details (evidence, affected locations, impact)
**Why we believe this is a bug:** `ConstantNode::toASTImpl` ([`src/Analyzer/ConstantNode.cpp:230`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/ConstantNode.cpp#L230)) -> `columnConstantToExactLiteralAST` -> the `TypeIndex::Object` arm ([`src/Analyzer/Utils.cpp:1777`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1777)) calls `getFieldFromColumnForASTLiteralImpl` with `datetime64_as_numbers=true` -> the `DateTime64` arm ([`src/Analyzer/Utils.cpp:1401-1405`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1401-L1405)) returns the raw decimal Field, which is written into the JSON text as a bare fractional number -> on the shard the `JSON` type parses that leaf with `DateTime64Node` ([`src/Formats/JSONExtractTree.cpp:990-998`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Formats/JSONExtractTree.cpp#L990-L998)), whose DOUBLE arm re-serialises the already-`Float64`-rounded DOM element, and whose UINT64 arm reads a bare integer as raw ticks when `read_datetime_number_as_raw_value` is on.
**Affected locations:**
- [`src/Analyzer/Utils.cpp:1401`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1401) — `datetime64_as_numbers` arm returns the raw decimal Field for a DateTime64 leaf
- [`src/Analyzer/Utils.cpp:1777`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1777) — Object arm of columnConstantToExactLiteralASTImpl passes datetime64_as_numbers=true
- [`src/Formats/JSONExtractTree.cpp:996`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Formats/JSONExtractTree.cpp#L996) — DOUBLE arm re-serialises the Float64-rounded DOM element
- [`src/Formats/JSONExtractTree.cpp:1006`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Formats/JSONExtractTree.cpp#L1006) — UINT64 arm reads a bare integer as raw ticks under read_datetime_number_as_raw_value
**Impact:** Silent wrong results for distributed queries carrying a `JSON` constant with a typed `DateTime64` path: sub-second precision loss for scales 7-9 (scales 3 and 6 verified unaffected), and an off-by-scale value (wrong date) whenever the shard runs with `compatibility <= '26.7'` / `input_format_read_datetime_number_as_raw_value = 1`. Both are regressions: the pre-PR text form parsed exactly.
### Does it reproduce on most recent release?
Yes — confirmed on current `master` (commit `569436cacf8cec`).
### How to reproduce
```sql
-- Test: a typed DateTime64 leaf of a JSON constant must reach a remote shard as the exact instant.
SET enable_analyzer = 1;
SET prefer_localhost_replica = 0;
SET serialize_query_plan = 0;
SELECT toUnixTimestamp64Nano(materialize(CAST('{"a":"2023-10-29 01:30:00.123456789"}', 'JSON(a DateTime64(9, \'UTC\'))')).a) AS v
ORDER BY v;
SELECT toUnixTimestamp64Nano(json.a) AS v
FROM (SELECT materialize(CAST('{"a":"2023-10-29 01:30:00.123456789"}', 'JSON(a DateTime64(9, \'UTC\'))')) AS json FROM remote('127.0.0.1', system.one))
ORDER BY v;
SELECT toUnixTimestamp64Milli(json.a) AS v
FROM (SELECT materialize(CAST('{"a":"2023-10-29 01:30:00.000"}', 'JSON(a DateTime64(3, \'UTC\'))')) AS json FROM remote('127.0.0.1', system.one))
ORDER BY v
SETTINGS compatibility = '26.7';
```
### Expected behavior
```
1698543000123456789
1698543000123456789
1698543000000
```
### Error message and/or stacktrace
```
1698543000123456789
1698543000123456700
1698543000
```
Suggested fix
Keep the exact text form for typed `DateTime64` JSON leaves and remove the DST ambiguity a different way (e.g. render the leaf in UTC with an explicit offset, or emit the number only when the value is integral in seconds and `input_format_read_datetime_number_as_raw_value` is off). Either way the comment at [`src/Analyzer/Utils.cpp:1366-1376`](https://github.com/ClickHouse/ClickHouse/blob/569436cacf8cec/src/Analyzer/Utils.cpp#L1366-L1376) must stop claiming `SerializationDateTime64::deserializeTextJSON` is the receiving parser — the `JSON` type uses `JSONExtractTree`.
Additional context
**Open risks:**
- `Time64` leaves use the same flag; they round-trip correctly today (verified) because the integer ticks form matches `Time64Node`, but the two leaf kinds now depend on opposite readings of a bare number in the same JSON text.
Found during automated review of [PR #95055](https://github.com/ClickHouse/ClickHouse/pull/95055). Severity P2 · Finding `h_pr95055_001`
cc @yakov-olkhovskiy @evillique @alexey-milovidov (from #95055)
Contributor guide
Assessment
This issue has not been assessed yet.