Range-check a `UInt64` constant converted to `DateTime` by alexey-milovidov · Pull Request #118367 · ClickHouse/ClickHouse · GitHub
Skip to content

Range-check a UInt64 constant converted to DateTime - #118367

Open
alexey-milovidov wants to merge 1 commit into
masterfrom
datetime-set-constant-range-117244
Open

Range-check a UInt64 constant converted to DateTime#118367
alexey-milovidov wants to merge 1 commit into
masterfrom
datetime-set-constant-range-117244

Conversation

@alexey-milovidov

@alexey-milovidov alexey-milovidov commented Sep 6, 2026 •
edited by clickhouse-gh Bot
Loading

Copy link
Copy Markdown
Member

A DateTime stores a UInt32, so a UInt64 constant that does not fit it cannot equal any value of the column. The conversion took such a constant unchanged - UInt64 is the canonical Field type of DateTime, so it looked like nothing had to be done - and the column insertion downstream truncated it modulo 2^32:

CREATE TABLE t (dt DateTime('UTC')) ENGINE = MergeTree ORDER BY dt;
INSERT INTO t VALUES (0), (1), (2);

SELECT count() FROM t WHERE dt = toUInt64(4294967296);         -- 0
SELECT count() FROM t WHERE dt IN (toUInt64(4294967296));      -- 1, was wrong: matched the epoch row
SELECT count() FROM t WHERE dt NOT IN (toUInt64(4294967296));  -- 2, was wrong: a row lost

-- every equality is individually false, and the OR -> IN rewrite returned all rows:
SELECT count() FROM t
WHERE dt = toUInt64(4294967296) OR dt = toUInt64(4294967297) OR dt = toUInt64(4294967298);  -- 3, was wrong

The Date, Date32 and Time branches next to it already range-check for the same reason; DateTime now does too. A constant outside the range is excluded from a set, as an out-of-range integer for a numeric column already was.

One note on the values path: the issue asks for values('x DateTime', toUInt64(4294967296)) to produce what CAST produces (a wrapped 2106-02-07 06:28:15). It now raises ARGUMENT_OUT_OF_BOUND instead, which is what that path already does for Date, Date32 and even UInt8 - values('x UInt8', toUInt64(256)) has always been an error while CAST wraps. Rejecting it keeps DateTime consistent with every other type there.

Closes: #117244

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fixed a UInt64 constant that does not fit UInt32 matching an unrelated row of a DateTime column in IN (and in an OR chain of equalities, which is rewritten into IN): the constant was truncated modulo 2^32 instead of being excluded from the set.

Workflow [PR]
Sync PR [sync-upstream/pr/118367]

@alexey-milovidov @claude
Range-check a UInt64 constant converted to DateTime
b50a2a4 
A `DateTime` stores a `UInt32`, so a `UInt64` constant that does not fit it
cannot equal any value of the column. The conversion took such a constant
unchanged - `UInt64` is the canonical `Field` type of `DateTime`, so it looked
like nothing had to be done - and the column insertion downstream truncated it
modulo 2^32:

    SELECT count() FROM t WHERE dt = toUInt64(4294967296);        -- 0
    SELECT count() FROM t WHERE dt IN (toUInt64(4294967296));     -- 1: matched the epoch row
    SELECT count() FROM t WHERE dt NOT IN (toUInt64(4294967296)); -- 2: a row lost

At default settings the disjunction rewrite turns an `OR` chain of equalities
into `IN`, so a `WHERE` in which every comparison is individually false returned
every row the constants wrapped onto.

The `Date`, `Date32` and `Time` branches next to it already range-check for the
same reason; `DateTime` now does too. A constant outside the range is excluded
from a set, as an out-of-range integer for a numeric column already was, and the
`values` table function rejects it the way it rejects one for `Date` or `UInt8`.

Closes: #117244

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@clickhouse-gh

clickhouse-gh Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added pr-bugfix Pull request with bugfix, not backported by default comp-query-execution Runtime execution pipeline, processors, vectorized operators, resource management during execution. labels Sep 6, 2026
@zlareb1

zlareb1 commented Sep 6, 2026

Copy link
Copy Markdown
Member

TL;DR: the fix itself looks right and fail-safe on every consumer I traced (set members get excluded exactly, values/INSERT throws loudly, and a null Field in KeyCondition degrades to a full read), but the identical truncation survives a few lines below for DateTime64, and the IPv4 branch mishandles the null Field the fix introduces on other paths.

  1. UInt64 → DateTime64/Time64 (same file, the isDateTime64/isTime64 branches): the conversion funnels through FieldVisitorConvertToNumber<Int64>, whose UInt64 arm is a plain wrap, so a value ≥ 2^63 goes negative instead of being rejected — the same class this PR fixes for DateTime. Verified on a recent build: SELECT toDateTime64(-1, 0, 'UTC') IN (toUInt64(18446744073709551615)) returns 1; on a MergeTree table with DateTime64(0) values (-1, 0, 1), WHERE dt NOT IN (toUInt64(18446744073709551615)) returns 2 of 3 rows; and SELECT x FROM values('x DateTime64(0, ''UTC'')', toUInt64(18446744073709551615)) silently inserts 1969-12-31 23:59:59, while after this PR the identical DateTime shape throws ARGUMENT_OUT_OF_BOUND.

  2. UInt64 → IPv4: the branch calls safeGet<UInt32> on the result of the range-checked conversion, so an out-of-range constant that now yields a null Field throws Bad get: has Null, requested UInt64 (code 170) instead of excluding the member: SELECT toIPv4('0.0.0.4') IN (toUInt64(4294967300)) → BAD_GET. Pre-existing, but the same one-line convertNumericType treatment would cover it.

Happy to file the DateTime64 one separately if it's out of scope here.

@clickhouse-gh

clickhouse-gh Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp-query-execution Runtime execution pipeline, processors, vectorized operators, resource management during execution. pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Milestone

No milestone

2 participants

@alexey-milovidov @zlareb1