Accept fractional unix timestamp for DateTime64 in containers and JSON - #109978
Accept fractional unix timestamp for DateTime64 in containers and JSON#109978groeneai wants to merge 19 commits into
Conversation
Container (Array/Tuple/Map) and JSON DateTime64 elements went through deserializeTextQuoted/deserializeTextJSON, whose unquoted branch used readIntText and stopped at the decimal point, so an element written as an unquoted fractional unix timestamp (e.g. 1783585473.954) failed with CANNOT_READ_ARRAY_FROM_TEXT while a scalar DateTime64 column accepted it. Read the fractional part when a decimal point follows the integer, mirroring the scalar readDateTimeTextImpl logic. A bare integer stays a scaled tick count, so previously-accepted inputs are unchanged. Closes ClickHouse#109884 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pre-PR validation gate (click to expand)
Session id: cron:clickhouse-worker-slot-1:20260710-100400 |
|
cc @Avogar @yariks5s — could you review this? It makes |
| SELECT 'csv_array_frac', * FROM format(CSV, 'x Array(DateTime64(3))', '"[1783585473.954,1783585473.954]"'); | ||
|
|
||
| -- Fractional form parses to the same value as the equivalent integer-ticks form. | ||
| SELECT 'csv_array_frac_eq_ticks', |
|
Workflow [PR], commit [d96914e] Summary: ❌
AI ReviewSummaryThis PR extends unquoted fractional unix-timestamp parsing for nested and JSON Final Verdict
|
readIntText normalises -0 to 0, so the sign was lost for pre-epoch sub-second values with a zero whole part parsed inside containers (arrays/tuples/maps) and JSON: -0.123 was stored as 1970-01-01 00:00:00.123 instead of 1969-12-31 23:59:59.877. Restore the sign explicitly when the whole part is zero and a negative sign was present. Non-zero whole parts already carry the sign through decimalFromComponents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Re: DateTime. Checked scalar vs container for plain
So plain |
|
Fixed the negative sub-second edge case (0bb3a36).
New handling: restore the sign explicitly only when a Verified against ticks (scale 3): |
The scalar readDateTime64Text path normalised the whole part of -0.xxx to 0 (readIntText drops the sign) and never restored it, so a scalar DateTime64 parsed '-0.123' as 1970-01-01 00:00:00.123 while the container/JSON helper (fixed earlier in this PR) parsed it as 1969-12-31 23:59:59.877. Factor the fractional sign finalisation into a shared adjustFractionalDateTimeSign() used by the scalar DateTime64, Time64, and container paths so all agree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in 237f5a9. Moved the sign restoration into a shared `adjustFractionalDateTimeSign()` helper (src/IO/ReadHelpers.h) now used by the scalar `readDateTimeTextImpl(DateTime64&)`, the `Time64` reader (which already had the same logic inline), and the container/JSON helper. Both cases live in one place:
Scalar and container now agree (Basic input format, verified locally):
Added scalar regressions to |
CI randomizes date_time_input_format. The fractional unix-timestamp form this fix adds is a feature of the basic DateTime64 reader (readDateTime64Text / readNumericText). The best_effort reader is a separate parser that rejects the '.'-containing form even for scalar columns, so under a randomized date_time_input_format=best_effort the test failed with CANNOT_PARSE_DATETIME / UNEXPECTED_DATA_AFTER_PARSED_VALUE. Pin basic so the test exercises the reader the bug and fix concern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The scalar readDateTime64Text path already accepts the shorthand -.123 (sign directly followed by the decimal point, implied zero whole part), but the container/JSON numeric helper went straight through readIntText, which rejects a lone sign with no digits. That left a scalar/nested asymmetry for the negative shorthand form. Special-case -.<digits> in the container helper: consume the sign, leave whole == 0, and let the shared adjustFractionalDateTimeSign restore the sign. Now scalar and container agree for -.123. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in e0d333f. The container/JSON numeric helper now special-cases the bare
The Regression coverage added in 04401: |
The container/JSON helper detected the bare -.123 shorthand with a lookahead into the current buffer chunk (istr.available()/position()[1]). When - was the last byte of a refill and . started the next, the branch was skipped, readIntText consumed the sign and threw CANNOT_PARSE_NUMBER. Consume the sign first, then inspect the next byte after an istr.eof() refill so the decision is independent of chunk boundaries. A lone - with no fraction or magnitude is still rejected. Adds a streamed regression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test declared the array elements as bare Array(DateTime64(3)), whose rendering via toString() falls back to the host timezone. On a non-UTC CI runner the pre-epoch values rendered one hour off (e.g. 1970-01-01 00:59:59.877 instead of 1969-12-31 23:59:59.877), failing the reference diff. Pin the element timezone to 'UTC' in the --structure so rendering is host-TZ-independent. Parse-correctness assertions are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed a test-determinism issue in the new |
Bake 'UTC' into the Array(DateTime64(3, 'UTC')) column type so toString() rendering is independent of the CI runner's process timezone. The parsed instants were already correct; only the rendered strings differed under a non-UTC runner because session_timezone does not propagate into the arrayMap(toString) element type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The container/JSON numeric helper read the whole part through readIntText, which silently accepts a leading '+'. Scalar DateTime64 basic parsing rejects it (readDateTimeTextFallback only special-cases '-'), and the pre-PR container path went through readDateTime64Text and also rejected it. Filter '+' up front so the container/JSON path stays in parity with scalar and pre-PR behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nal test CI randomizes date_time_input_format. A per-query SETTINGS clause on format() inside a scalar subquery does not reach the parallel parsing reader, so under best_effort the scalar -0.xxx rows hit parseDateTimeBestEffort (which rejects the '.') and fail with CANNOT_PARSE_DATETIME. Use a top-level SET, which reliably overrides the runner randomization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The unquoted container/JSON DateTime64 helper hard-coded the basic-parser dotted rules regardless of date_time_input_format. Under best_effort the scalar and quoted-nested paths route through parseDateTime64BestEffort and reject the dotted fractional/sign/shorthand forms, but the unquoted nested path still accepted them, so the effective parser depended on quoting/nesting instead of the setting. Thread date_time_input_format into the helper and fall back to the pre-PR bare-integer tick-count path under best_effort/best_effort_us, so the dotted forms are rejected there exactly as the scalar and quoted paths reject them. The bare integer tick count still parses under every setting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ners The container/JSON DateTime64 helper consumed the leading '-' up front (for chunk-boundary safety) then read the magnitude into a signed time_t and did `whole = -whole`. For the minimum tick value -9223372036854775808 the magnitude 2^63 does not fit a signed Int64, so this was signed-overflow UB, and it also diverged between the throw path (readIntText, no overflow check) and the try path (tryReadIntText, CHECK_OVERFLOW rejects 2^63). Read the magnitude as UInt64 and negate via well-defined two's-complement arithmetic, rejecting magnitudes past 2^63. This preserves INT64_MIN/INT64_MAX exactly, keeps both paths in agreement, and matches the pre-PR readIntText(x) container behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The unquoted container/JSON helper readNumericTextImpl read the positive whole part via bare readIntText (DO_NOT_CHECK_OVERFLOW), so the throw path could consume an out-of-range whole seconds value and wrap a signed time_t before the fractional logic ran, while the try path (tryReadIntText, CHECK_OVERFLOW) and the scalar readDateTime64Text path both reject it. Use ReadIntTextCheckOverflow::CHECK_OVERFLOW for the positive whole part in both the throw and try helpers, and for the best_effort bare-integer fallback, so an out-of-range whole is rejected consistently and container parsing matches scalar. Add regressions for the positive out-of-range whole (container throw/try + scalar) and an in-range scalar==container check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ime64 containers The unquoted container/JSON DateTime64 helper rejected a leading '+' only for the basic dotted path; under best_effort/best_effort_us it took the bare-integer readIntText fallback first, which accepts '+', so [+1783585473954] parsed while scalar DateTime64 (parseDateTime64BestEffort) rejects the same token. Move the '+' guard before the input_format branch so it applies to every date_time_input_format, restoring scalar/container parity. Also chunk-boundary safe: the '+' byte is inspected before any refill decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A token with no digit on either side of the decimal point ('.' or '-.')
was silently coerced to the epoch: readIntText leaves whole == 0, then the
fractional branch consumed '.' and padded an empty mantissa with zeroes.
Both forms were rejected before this PR. Add a digit-presence check in the
shared scalar/container path (readFractionalDateTimePart reports the number
of fractional digits; the whole-part advance is measured via buf.count(),
which is chunk-boundary safe) so scalar DateTime64, Time64, and the
container/JSON helper all reject the empty-mantissa forms and agree.
'.5', '5.', '-.5', '0.' all carry a digit and remain valid.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rence The non-throwing (try) DateTime64/Time64 text readers recovered on a trailing '.' whenever the whole reader failed, even after it had consumed date/time bytes (e.g. `5981 10:01` before `.000`). That fed a bogus whole part into the fractional branch, which overflowed Decimal64 at large scale during schema inference, so `5981 10:01.000` inferred as DateTime64(9) and then threw DECIMAL_OVERFLOW instead of falling back to String. Restrict recovery to the leading-dot shorthand (`.5`, `-.5`), where the reader consumed nothing but the optional sign. The throwing path is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed the `03720_datetime64_bad_inference` Fast test regression in bf77586. The round-10 helper change relaxed the non-throwing (try) DateTime64/Time64 readers to recover on a trailing `.` whenever the whole reader failed. That was too broad: for a date-like token such as `5981 10:01.000` the whole reader consumes the date/time bytes, fails (year out of range), and left position at `.`; recovery then fed a bogus whole part into the fractional branch, so schema inference picked `DateTime64(9)` and reading overflowed Decimal64 (`DECIMAL_OVERFLOW`) instead of falling back to `String`. The fix restricts try-path recovery to the leading-dot shorthand (`.5`, `-.5`), where the reader consumed nothing but the optional sign (`buf.count() == count_before_whole + sign`, chunk-boundary safe). The throwing path is unchanged, so scalar `5.`/`5.5`/`.5` still parse. `03720` now infers `Nullable(String)` again; `03720`, `04401`, and `04409` all pass (x3 with randomized settings). |
At scale 3 a 13-digit best_effort millisecond timestamp and a scale-3 raw tick count render identically, so the existing be_unquoted_bare_int case cannot distinguish the scalar/quoted (parseDateTime64BestEffort) path from the unquoted-container raw-tick path. Add a scale-6 case where they differ (2026-07-09 08:24:33.954000 vs 1970-01-21 15:26:25.473954) and pin all three paths. The unquoted-vs-scalar/quoted difference is intentional and pre-existing: before this PR the unquoted numeric container path read a bare integer with readIntText (raw ticks) under every date_time_input_format, and preserving bare-integer = raw ticks is a backward-compatibility requirement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The out-of-range-year saturation of a date-like token such as 5981 10:01.000 comes from readDateTimeTextFallback, which this PR does not touch and which fires identically without a fraction. Add a direct scalar throwing-path case and assert scalar == toDateTime64OrNull (no throw-vs-try divergence). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The old analyzer drops subquery-level SETTINGS clauses, so the two best_effort comparison lines had their per-subquery settings ignored and both sides fell back to the session-level basic reader. With CI randomizing date_time_input_format the quoted side then read raw ticks and the equality returned 0 (deterministic FAIL under the old-analyzer coverage config). Hoist the best_effort settings to the outer query, which both analyzers honor and which overrides the session SET and CI randomization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed the deterministic `04401` failure under `Stateless tests (amd_llvm_coverage, old analyzer, ...)` (head f51fa36). Root cause: the two `be_*_bare_s6` best_effort comparison lines put `SETTINGS ... = 'best_effort'` inside the subqueries. The old analyzer drops subquery-level SETTINGS, so both sides fell back to the session-level `basic` reader; with CI randomizing `date_time_input_format` the quoted side read raw ticks and `be_scalar_eq_quoted_bare_s6` returned 0 instead of 1. The standalone `be_scalar_bare_s6`/`be_quoted_bare_s6` lines passed because their SETTINGS live on the top-level SELECT, which the old analyzer honors. Fix (d96914e, test-only): hoist the best_effort settings to the outer query, which both analyzers honor and which overrides the session SET and CI randomization. Verified all 42 output lines match the reference under both analyzers (0 mismatches), including with a hostile top-level `date_time_input_format=best_effort`; serverError-expecting lines still reject. |
CI finish ledger — d96914eCI is fully finished (Finish Workflow + Mergeable Check complete, no checks pending). The PR-caused failures on earlier heads (Fast test
Session id: cron:our-pr-ci-monitor:20260712-023000 |
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered: 108/130 (83.08%) · Uncovered code |
|
Dear @yariks5s, you haven't been active on this PR for 30 days. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself. |
Closes: #109884
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fixed parsing of
DateTime64values written as an unquoted fractional unix timestamp (e.g.1783585473.954) when they appear as elements ofArray,Tuple,Map, or in JSON. Previously only scalarDateTime64columns accepted this form; inside containers the value failed withCANNOT_READ_ARRAY_FROM_TEXT. A bare integer is still interpreted as a scaled tick count, unchanged.Description
Closes #109884.
Parsing CSV/TSV/JSON/Values into a container of
DateTime64(N)(e.g.Array(DateTime64(3))) rejected an element written as an unquoted fractional unix timestamp such as1783585473.954, failing withCannot read array from text, expected comma or end of array, found '.'(CANNOT_READ_ARRAY_FROM_TEXT). The same instant written as integer ticks (1783585473954) worked, and a scalarDateTime64column accepted both forms.Root cause: container/JSON elements go through
SerializationDateTime64::deserializeTextQuoted/deserializeTextJSON, whose unquoted branch usedreadIntText, which stops at the.. The leftover.then broke the array reader. Scalar CSV/Escaped columns usereadText(readDateTime64Text), which already handles the fractional form.Fix: the unquoted numeric branch now reads the integer and, if a
.follows, treats the integer part as whole seconds and the fraction as subseconds (mirroring the scalarreadDateTimeTextImpllogic, including negative-timestamp handling and scale truncation/padding). A bare integer with no.keeps its existing raw-tick interpretation, so behavior is unchanged for every previously-accepted input (no backward-incompatible change).