Translate standard DateTime members and methods (#55) - #64
Draft
alex-clickhouse wants to merge 2 commits into
Draft
Translate standard DateTime members and methods (#55)#64alex-clickhouse wants to merge 2 commits into
alex-clickhouse wants to merge 2 commits into
Conversation
The provider registered no date/time member translator, so only a direct comparison worked: every member and method threw "The LINQ expression ... could not be translated". Add one shared translator, which serves DateTime and DateOnly because the ClickHouse function is the same for each. Components map to the to* extraction functions. Those return UInt8/UInt16, which the provider's integer mappings already widen on read. .DayOfWeek maps to toDayOfWeek(x, 2). Week mode 2 agrees with System.DayOfWeek exactly, so no arithmetic correction is applied. The result carries a number-backed enum mapping, because this provider maps a C# enum to a ClickHouse string and that mapping would otherwise render x.DayOfWeek == DayOfWeek.Sunday as a comparison against 'Sunday'. .TimeOfDay maps to toTime64(x, 7); precision 7 is one .NET tick, so the fraction survives, which toTime would drop. AddYears and AddMonths take an int, so they map straight onto addYears and addMonths, which clamp the day of month the way .NET does. The other Add* methods take a double, which .NET scales to whole ticks. The matching ClickHouse function takes a whole number of its own unit and discards the rest, so addDays(x, 1.5) would add only one day. A constant is therefore folded to ticks and expressed in the coarsest unit that holds it exactly: the natural function when possible, otherwise addMilliseconds. Preferring the natural function keeps the source's store type and keeps Date/Date32 columns working, which addMilliseconds rejects. A sub-millisecond offset, a non-constant offset, and a value outside the DateTime range are left untranslated rather than rounded to fit. addNanoseconds would express a tick exactly, but promotes the result to DateTime64(9), whose Int64 nanosecond count cannot span the DateTime64 range — that would trade a rounding error for a silently wrong date. Server-side rounding was rejected too: ClickHouse round() is banker's rounding, so it disagrees with .NET. An untranslated call still gives the correct value through client evaluation in a projection, and reports a reason in a predicate. Also report a clear reason for arithmetic on two date/time values. ClickHouse has no operator that matches the .NET result: one date minus another gives a TimeSpan while dateDiff counts whole units, Time64 subtraction gives a Decimal of seconds, and a date plus a TimeSpan is rejected outright. These used to fail with an internal cast or coercion error naming CLR types the user never wrote. Reporting the reason also restores client evaluation in a projection, where the .NET result is correct. DateTimeOffset follows in the next commit, once the store mapping this branch is stacked on is in place. The Northwind GroupJoin_aggregate_anonymous_key_selectors2 query now translates, so its "not translatable" override is removed. Co-Authored-By: Claude <noreply@anthropic.com>
The shared translator already takes the CLR type as a parameter, so serving DateTimeOffset is a registration. It was held back only because the type had no store mapping: such a property resolved to String, where the extraction functions fail on the server and addDays silently drops both the offset and the sub-second part. The mapping from #53, which this branch is stacked on, removes that obstacle. The result is in the timezone the column declares, which the store type pins to UTC. That agrees with .NET, because a value read back from such a column carries the +00:00 offset, so .Hour and .Date describe the same instant on both sides. DateTimeOffset.Now translates to the same UTC-pinned now64 as UtcNow, since a DateTimeOffset is an instant. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds standard .NET date/time translation support to the ClickHouse EF Core query pipeline.
Changes:
- Translates date/time components, clocks, and
Add*methods. - Improves unsupported date/time arithmetic handling.
- Adds integration coverage and user documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
ClickHouseDateTimeMemberTranslator.cs |
Translates date/time members and clocks. |
ClickHouseDateTimeMethodTranslator.cs |
Translates Add* methods. |
ClickHouseMemberTranslatorProvider.cs |
Registers the member translator. |
ClickHouseSqlTranslatingExpressionVisitor.cs |
Handles unsupported date/time arithmetic. |
DateTimeMemberTranslationTests.cs |
Adds translation and integration tests. |
NorthwindJoinQueryClickHouseTest.cs |
Re-enables a formerly unsupported query. |
README.md |
Documents supported translations and limits. |
CHANGELOG.md |
Records the feature and behavior changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| }) ?? throw new InvalidOperationException("Method ToStartOfInterval with strict signature not found."); | ||
|
|
||
| RegisterAddMethods(typeof(DateTime), hasTimeComponents: true); | ||
| RegisterAddMethods(typeof(DateTimeOffset), hasTimeComponents: true); |
Comment on lines
+328
to
+330
| var scaled = value * ticksPerUnit + (value >= 0 ? 0.5 : -0.5); | ||
|
|
||
| return double.Abs(scaled) > DateTime.MaxValue.Ticks ? null : (long)scaled; |
Comment on lines
+86
to
+89
| // A DateTimeOffset is an instant, and its store type is UTC-pinned, so both of its clock | ||
| // members read the same UTC value. | ||
| ServerClockMembers.Add(Property(typeof(DateTimeOffset), nameof(DateTimeOffset.UtcNow)), ServerClock.UtcNow); | ||
| ServerClockMembers.Add(Property(typeof(DateTimeOffset), nameof(DateTimeOffset.Now)), ServerClock.UtcNow); |
Comment on lines
+284
to
+286
| if (value is not SqlConstantExpression { Value: double constantValue }) | ||
| { | ||
| return null; |
| * **`.TimeOfDay`** → `toTime64(x, 7)`; precision 7 is one .NET tick, so no part of the value is lost (`toTime` would drop the fraction). | ||
| * **`DateTime.UtcNow`** → `now64(7, 'UTC')`, **`DateTime.Now`** → `now64(7)` and **`DateTime.Today`** → `toStartOfDay(now())`. `today()` is not used for `.Today` because it returns a `Date`, whereas the member's type is `DateTime`. | ||
| * **`.AddYears(n)`** → `addYears` and **`.AddMonths(n)`** → `addMonths`. Both take an `int` in .NET, and ClickHouse clamps the day of month the same way .NET does, so `2026-01-31` plus one month gives `2026-02-28` in both. | ||
| * **`.AddDays`/`.AddHours`/`.AddMinutes`/`.AddSeconds`/`.AddMilliseconds`** take a `double` in .NET, which .NET scales to whole **ticks** (100 ns), rounding half away from zero — so `AddSeconds(0.1234567)` adds exactly 1 234 567 ticks. The matching ClickHouse function takes a whole number of its own unit and discards the rest, so `addDays(x, 1.5)` would add only one day. A constant argument is therefore folded to ticks during translation and then expressed in the coarsest unit that holds it exactly: a whole number of the unit emits the natural function (`addDays(x, 1)`), and otherwise `addMilliseconds` carries the exact count (`AddDays(1.5)` → `addMilliseconds(x, 129600000)`). Preferring the natural function keeps the store type of the source and keeps `Date`/`Date32` columns working, since `addMilliseconds` rejects those outright. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #55.
Problem
The provider registered no date/time member translator. EF Core's base
RelationalMemberTranslatorProvideradds none of its own, so only a direct comparison worked andevery member and method threw
The LINQ expression ... could not be translated. Subtracting one datefrom another was worse: it failed with an internal cast or coercion error naming CLR types the user
never wrote.
Solution
One shared translator serves
DateTime,DateTimeOffsetandDateOnly, because the ClickHousefunction is the same for each.
RegisterInstanceMembersregisters only the members a given typedeclares, so
DateOnlygets the date components alone..Year.Month.DaytoYeartoMonthtoDayOfMonth.Hour.Minute.Second.MillisecondtoHourtoMinutetoSecondtoMillisecond.DayOfYeartoDayOfYear.DayOfWeektoDayOfWeek(x, 2).DatetoStartOfDay.TimeOfDaytoTime64(x, 7).AddYears(n).AddMonths(n)addYearsaddMonths.AddDays(n)….AddMilliseconds(n)addDays… (see below)DateTime.UtcNow/.Now/.Todaynow64(7, 'UTC')/now64(7)/toStartOfDay(now())Every design decision below was measured against a real ClickHouse 26.7.1 server, and .NET behaviour
was measured on .NET 10 rather than taken from the documentation.
.DayOfWeekneeds no arithmetic. Week mode 2 agrees withSystem.DayOfWeekexactly (Sunday 0through Saturday 6). The mode argument is always sent, because the default mode 0 starts the week on
Monday.
.DayOfWeekcarries a number-backed enum mapping. This provider maps a C#enumto a ClickHousestring, so the default mapping would render
x.DayOfWeek == DayOfWeek.Sundayas a comparison against'Sunday'while the function returns a number. AnEnumToNumberConverterover the Int32 mappingfixes both sides. Verified against a server for projection,
WHERE, parameters,GROUP BY,HAVING,ORDER BY,DISTINCTandIN..TimeOfDayusestoTime64(x, 7), nottoTime. One .NET tick is 100 ns, which isTime64precision 7, so the fraction survives;
toTimedrops it.Add*is exact or is not translated. This is the subtle part.AddDaysand the other time-basedmethods take a
double, which .NET scales to whole ticks —AddSeconds(0.1234567)adds exactly1 234 567 ticks. The matching ClickHouse function takes a whole number of its own unit and discards
the rest, so
addDays(x, 1.5)would add only one day. A constant is therefore folded to ticks andexpressed in the coarsest unit that holds it exactly:
Preferring the natural function keeps the store type of the source, and it is the only form that
works on a
Date/Date32column — ClickHouse rejectsaddMillisecondson those withILLEGAL_TYPE_OF_ARGUMENT.Anything that cannot be expressed exactly is left untranslated rather than rounded to fit. Three
cases: a sub-millisecond offset, a non-constant offset, and a value outside the
DateTimerange. Anuntranslated call still gives the correct .NET value through client evaluation in a projection, and
reports a clear reason in a predicate.
Milliseconds are as fine as this goes deliberately.
addNanosecondswould express a tick exactly,but promotes the result to
DateTime64(9), whose Int64 nanosecond count cannot span theDateTime64range — that would trade a rounding error for a silently wrong date. Server-siderounding was also rejected: ClickHouse
round()is banker's rounding (round(2.5)is2), so itdisagrees with .NET.
Date/time arithmetic now reports a reason. ClickHouse has no operator matching the .NET result
for any of these shapes, and each failed differently before:
TimeSpan, whereasdateDiffcounts whole units;TimeSpan, whereasTime64subtraction gives aDecimalof seconds;
TimeSpankeeps the date type in .NET, whereas ClickHouse rejects the mixedoperands.
VisitBinarynow reports these through EF Core's translation-error channel instead of throwing, soa projection falls back to the client and returns the correct value, and a predicate explains why.
Tests
DateTimeMemberTranslationTests— 60 tests. Integration tests run against a real ClickHouse throughTestcontainers, as
AGENTS.mdprefers, with a small offline class for SQL-shape assertions.Coverage worth calling out: sub-millisecond
Add*values, a parameterised offset, an out-of-rangeoffset,
AddMonthsday clamping,.DayOfWeekcompared against a .NET constant,.TimeOfDayto onetick,
Date32columns,TimeSpanarithmetic falling back to the client, and theDateTimeOffsetequivalents.
Also in this PR
A previously-unsupported Northwind query,
GroupJoin_aggregate_anonymous_key_selectors2, nowtranslates. Its provider-specific override asserted
InvalidOperationException, so the override isremoved and the base test runs. The functional suite goes from 321 passed + 2 failed to 323 passed.
Behaviour change
DateTime.NowandDateTime.Todayin a projection used to be evaluated on the client; they nowread the server clock. The value therefore follows the server's timezone rather than the
client's, and comes back with
DateTimeKind.Unspecifiedinstead ofLocal. UseDateTime.UtcNowfor an instant that does not depend on server configuration. In a predicate all three were
untranslatable before, so nothing changes there.
This matches how other EF Core providers translate these members (
GETDATE(),now()), but it is asemantic shift and worth a second opinion.
Known limits
.DateusestoStartOfDay, which returns aDateTimespanning 1970–2106. ClickHouse wraps avalue outside that window instead of reporting it, so
.Dateon aDateTime64column holding apre-1970 date reads back wrong. Enabling
enable_extended_results_for_datetime_functionsgives a range-preservingDateTime64result —measured. This is the same caveat that already applies to
EF.Functions.ToStartOfDay, and it isnow documented for both.
.Ticks,.AddTicks,.Microsecond/.Nanosecond, andDateTimeOffset's.UtcDateTime/.LocalDateTime/.Offset.dateDiff,dateTrunc) stay in Add EF.Functions translations for the remaining ClickHouse date/time functions #58.🤖 Generated with Claude Code