From 4362f57115740bdb1c482f5abf226b120208ec62 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Fri, 14 Aug 2026 11:17:44 +0200 Subject: [PATCH] Validate date-time query-constant operands --- CHANGELOG.md | 1 + README.md | 2 +- RELEASENOTES.md | 1 + ...ClickHouseDateTimeDbFunctionsExtensions.cs | 10 +++- .../ClickHouseDateTimeMethodTranslator.cs | 28 ++++++++- .../DateTimeFunctionsTranslationTests.cs | 57 +++++++++++++++++++ 6 files changed, 93 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f743cd..a4d48df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ v0.3.1 (Unreleased) * **`toStartOf*` date-time functions** via `EF.Functions`: `ToStartOfYear`, `ToStartOfQuarter`, `ToStartOfMonth`, `ToStartOfWeek` (with optional week `mode`), `ToStartOfDay`, `ToStartOfHour`, `ToStartOfMinute`, `ToStartOfSecond`, the fixed buckets `ToStartOfFiveMinutes` / `ToStartOfTenMinutes` / `ToStartOfFifteenMinutes`, and the general `ToStartOfInterval(source, value, unit)`. Each maps to the matching ClickHouse function and works in `GROUP BY`. Return types follow ClickHouse: the calendar buckets (`Year`/`Quarter`/`Month`/`Week`) return `Date`, the day/hour/minute buckets return `DateTime`, and `ToStartOfSecond` returns `DateTime64`. All accept `DateTime`/`DateTime64` columns, and the plain truncation functions also accept `DateOnly`; `ToStartOfInterval` requires a `DateTime`/`DateTime64` column on older ClickHouse, which rejects a `DateOnly` (Date/Date32) source with `Illegal type Date32 of 1st argument` for every unit; recent versions accept it. `ToStartOfInterval` takes a `ClickHouseInterval` unit (`Second`…`Year`) and emits `toStartOfInterval(source, toInterval(value))`; the unit must be a constant so it can be translated. The default `Date`/`DateTime` result types only span 1970–2149/2106, so ClickHouse narrows out-of-range values — enable `enable_extended_results_for_datetime_functions` (e.g. `set_enable_extended_results_for_datetime_functions=1` in the connection string) for range-preserving `Date32`/`DateTime64` results. ### Bug fixes +* `ToStartOfWeek` now rejects row-dependent week modes during query translation, and `ToStartOfInterval` likewise rejects row-dependent interval sizes. ClickHouse requires these operands to be constant for the query; literals and captured query parameters remain supported. * `Sum`/`SumAsync` over a `double` or `float` column no longer throws `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns `0`, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; the literal generators now convert rather than unbox. The `Float32` read path also converts, since ClickHouse widens `sum(Float32)` to `Float64` (which the driver's `GetFloat()` refuses to downcast). ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) * **SummingMergeTree with multiple sum columns**: `HasSummingMergeTreeEngine("A", "B")` now generates valid DDL (`SummingMergeTree((A, B))`). Previously it emitted a comma-separated argument list (`SummingMergeTree(A, B)`), which ClickHouse rejects with `NUMBER_OF_ARGUMENTS_DOESNT_MATCH`. Single-column and no-column usage are unaffected. diff --git a/README.md b/README.md index e2f60db..6363d7e 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ var buckets = await ctx.Events .ToListAsync(); ``` -`ToStartOfInterval` takes a `ClickHouseInterval` unit (`Second`, `Minute`, `Hour`, `Day`, `Week`, `Month`, `Quarter`, `Year`) — from the `ClickHouse.EntityFrameworkCore.Metadata` namespace — and emits `toStartOfInterval(source, toInterval(value))`. The unit must be a constant. +`ToStartOfInterval` takes a `ClickHouseInterval` unit (`Second`, `Minute`, `Hour`, `Day`, `Week`, `Month`, `Quarter`, `Year`) — from the `ClickHouse.EntityFrameworkCore.Metadata` namespace — and emits `toStartOfInterval(source, toInterval(value))`. The unit must be an inline enum constant. The interval size and optional `ToStartOfWeek` mode may be literals or captured query parameters, but cannot depend on values from the current row. Input and return types follow ClickHouse. The calendar buckets (`ToStartOfYear`/`Quarter`/`Month`/`Week`) return `Date`; `ToStartOfDay` and the hour/minute buckets return `DateTime`; `ToStartOfSecond` returns `DateTime64`. They all accept `DateTime` and `DateTime64` columns, and the plain truncation functions also accept `DateOnly` (Date/Date32). `ToStartOfInterval` is the exception: older ClickHouse rejects a `DateOnly` (Date/Date32) source with `Illegal type Date32 of 1st argument` while recent versions accept it. Prefer a `DateTime`/`DateTime64` column for interval bucketing. diff --git a/RELEASENOTES.md b/RELEASENOTES.md index bb96fc6..885476a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -4,6 +4,7 @@ v0.3.1 (Unreleased) * **`toStartOf*` date-time functions** are now translatable through `EF.Functions`, covering the full family: `ToStartOfYear`, `ToStartOfQuarter`, `ToStartOfMonth`, `ToStartOfWeek` (with an optional ClickHouse week `mode`), `ToStartOfDay`, `ToStartOfHour`, `ToStartOfMinute`, `ToStartOfSecond`, the fixed buckets `ToStartOfFiveMinutes` / `ToStartOfTenMinutes` / `ToStartOfFifteenMinutes`, and the general-purpose `ToStartOfInterval(source, value, unit)`. They compose in `GROUP BY` for time bucketing. Input and return types follow ClickHouse: the calendar buckets (`ToStartOfYear`/`Quarter`/`Month`/`Week`) return `Date`, `ToStartOfDay` and the hour/minute buckets return `DateTime`, and `ToStartOfSecond` returns `DateTime64`. All accept `DateTime`/`DateTime64` columns, and the plain truncation functions also accept `DateOnly` (Date/Date32); `ToStartOfInterval` is the exception — older ClickHouse rejects a `DateOnly` (Date/Date32) source with `Illegal type Date32 of 1st argument`, while recent versions accept it, so prefer a `DateTime`/`DateTime64` column for interval bucketing. `ToStartOfInterval` uses a `ClickHouseInterval` enum for the unit and is emitted as `toStartOfInterval(source, toInterval(value))`. Because the default result types (`Date`/`DateTime`) only span 1970–2149/2106, ClickHouse narrows out-of-range values (pre-1970 clamps to the epoch or wraps around); enable `enable_extended_results_for_datetime_functions` in your session (e.g. `set_enable_extended_results_for_datetime_functions=1` in the connection string) to get range-preserving `Date32`/`DateTime64` results. ### Bug fixes +* `ToStartOfWeek` now rejects row-dependent week modes during query translation, and `ToStartOfInterval` likewise rejects row-dependent interval sizes. ClickHouse requires these operands to be constant for the query; literals and captured query parameters remain supported. * Summing a `double` or `float` column (`.SumAsync(x => x.Value)`) no longer throws `InvalidCastException`. Two ClickHouse-specific mismatches were biting: EF Core hands the float literal generator a boxed `Int32` `0` as the empty-result fallback, and ClickHouse widens `sum(Float32)` to `Float64` so the driver couldn't read it back as a `float`. Both the literal generation and the `Float32` read path now convert instead of hard-casting. ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) (Thanks to @HotTotem!) * **SummingMergeTree with multiple sum columns** now produces valid DDL. Configuring more than one sum column (`HasSummingMergeTreeEngine("A", "B")`) previously emitted `SummingMergeTree(A, B)`, which ClickHouse rejects with `NUMBER_OF_ARGUMENTS_DOESNT_MATCH` — the engine takes a single optional parameter that must be a tuple of columns. Multiple columns are now wrapped in a tuple (`SummingMergeTree((A, B))`); single-column and no-column usage are unchanged. diff --git a/src/EFCore.ClickHouse/Extensions/ClickHouseDateTimeDbFunctionsExtensions.cs b/src/EFCore.ClickHouse/Extensions/ClickHouseDateTimeDbFunctionsExtensions.cs index f02a91e..f1911bd 100644 --- a/src/EFCore.ClickHouse/Extensions/ClickHouseDateTimeDbFunctionsExtensions.cs +++ b/src/EFCore.ClickHouse/Extensions/ClickHouseDateTimeDbFunctionsExtensions.cs @@ -57,7 +57,10 @@ public static T ToStartOfWeek(this DbFunctions _, T source) => /// /// The instance. /// The date or date-time value to truncate. - /// The ClickHouse week mode (0-9) that determines the first day of the week. + /// + /// The ClickHouse week mode (0-9) that determines the first day of the week. Must be a literal or captured + /// query parameter; row-dependent expressions cannot be translated. + /// [DbFunction("toStartOfWeek")] public static T ToStartOfWeek(this DbFunctions _, T source, byte mode) => throw new InvalidOperationException(CoreStrings.FunctionOnClient(nameof(ToStartOfWeek))); @@ -144,7 +147,10 @@ public static T ToStartOfFifteenMinutes(this DbFunctions _, T source) => /// (Date/Date32) source for toStartOfInterval with /// Illegal type Date32 of 1st argument while recent versions accept it. /// - /// The number of interval units in each bucket. + /// + /// The number of interval units in each bucket. Must be a literal or captured query parameter; + /// row-dependent expressions cannot be translated. + /// /// The interval unit. Must be a constant so it can be translated to SQL. [DbFunction("toStartOfInterval")] public static T ToStartOfInterval(this DbFunctions _, T source, int value, ClickHouseInterval unit) => diff --git a/src/EFCore.ClickHouse/Query/ExpressionTranslators/Internal/ClickHouseDateTimeMethodTranslator.cs b/src/EFCore.ClickHouse/Query/ExpressionTranslators/Internal/ClickHouseDateTimeMethodTranslator.cs index 50c11df..fa31725 100644 --- a/src/EFCore.ClickHouse/Query/ExpressionTranslators/Internal/ClickHouseDateTimeMethodTranslator.cs +++ b/src/EFCore.ClickHouse/Query/ExpressionTranslators/Internal/ClickHouseDateTimeMethodTranslator.cs @@ -36,6 +36,7 @@ public class ClickHouseDateTimeMethodTranslator : IMethodCallTranslator [ClickHouseInterval.Year] = "toIntervalYear", }; + private static readonly MethodInfo ToStartOfWeekWithModeMethod; private static readonly MethodInfo ToStartOfIntervalMethod; static ClickHouseDateTimeMethodTranslator() @@ -75,7 +76,7 @@ void RegisterSourceOnly(string methodName, string sqlFunction) RegisterSourceOnly(nameof(DateTimeDbFunctions.ToStartOfFifteenMinutes), "toStartOfFifteenMinutes"); // ToStartOfWeek(source, byte mode) -> toStartOfWeek - var weekWithMode = type.GetMethods().FirstOrDefault(m => + ToStartOfWeekWithModeMethod = type.GetMethods().FirstOrDefault(m => { if (m.Name != nameof(DateTimeDbFunctions.ToStartOfWeek) || !m.IsGenericMethod) { @@ -88,7 +89,7 @@ void RegisterSourceOnly(string methodName, string sqlFunction) && parameters[1].ParameterType.IsGenericParameter && parameters[2].ParameterType == typeof(byte); }) ?? throw new InvalidOperationException("Method ToStartOfWeek(source, mode) with strict signature not found."); - SupportedMethods.Add(weekWithMode, "toStartOfWeek"); + SupportedMethods.Add(ToStartOfWeekWithModeMethod, "toStartOfWeek"); ToStartOfIntervalMethod = type.GetMethods().FirstOrDefault(m => { @@ -121,6 +122,13 @@ public ClickHouseDateTimeMethodTranslator(ISqlExpressionFactory sqlExpressionFac if (SupportedMethods.TryGetValue(genericMethod, out var function)) { + // ClickHouse requires the optional week mode to be constant for the query. SQL literals and + // parameters both satisfy that requirement, but row-dependent expressions (such as columns) do not. + if (genericMethod == ToStartOfWeekWithModeMethod && !IsQueryConstant(arguments[2])) + { + throw QueryConstantRequired(nameof(DateTimeDbFunctions.ToStartOfWeek), "mode"); + } + // arguments[0] is the DbFunctions receiver; the source is arguments[1]. var sqlArguments = arguments.Skip(1).ToList(); var source = sqlArguments[0]; @@ -140,7 +148,13 @@ public ClickHouseDateTimeMethodTranslator(ISqlExpressionFactory sqlExpressionFac var source = arguments[1]; var value = arguments[2]; - // The interval unit must be a compile-time constant so it can be mapped to a toInterval* function. + // ClickHouse requires the interval value to be constant for the query. The unit is stricter: it must + // be a SQL literal because its value selects the toInterval* function emitted into the SQL tree. + if (!IsQueryConstant(value)) + { + throw QueryConstantRequired(nameof(DateTimeDbFunctions.ToStartOfInterval), "value"); + } + if (arguments[3] is not SqlConstantExpression { Value: ClickHouseInterval unit } || !IntervalFunctions.TryGetValue(unit, out var intervalFunction)) { @@ -165,4 +179,12 @@ public ClickHouseDateTimeMethodTranslator(ISqlExpressionFactory sqlExpressionFac return null; } + + private static bool IsQueryConstant(SqlExpression expression) + => expression is SqlConstantExpression or SqlParameterExpression; + + private static InvalidOperationException QueryConstantRequired(string method, string argument) + => new( + $"The '{method}' method's '{argument}' argument must be a SQL literal or query parameter because " + + "ClickHouse requires it to be constant for the query. Row-dependent expressions cannot be translated."); } diff --git a/test/EFCore.ClickHouse.Tests/DateTimeFunctionsTranslationTests.cs b/test/EFCore.ClickHouse.Tests/DateTimeFunctionsTranslationTests.cs index 96122df..8c2666a 100644 --- a/test/EFCore.ClickHouse.Tests/DateTimeFunctionsTranslationTests.cs +++ b/test/EFCore.ClickHouse.Tests/DateTimeFunctionsTranslationTests.cs @@ -141,6 +141,20 @@ public async Task ToStartOfWeek_mode_one_starts_on_monday() Assert.Equal(new DateTime(2026, 8, 10), result); } + [Fact] + public async Task ToStartOfWeek_accepts_parameterized_mode() + { + var mode = (byte)1; + + await using var context = new DateTimeDbContext(_fixture.ConnectionString); + var query = context.Events.AsNoTracking().Where(e => e.Id == 1) + .Select(e => EF.Functions.ToStartOfWeek(e.Timestamp, mode)); + + Assert.Contains("{mode:UInt8}", query.ToQueryString()); + var result = await query.SingleAsync(); + Assert.Equal(new DateTime(2026, 8, 10), result); + } + [Fact] public async Task ToStartOfDay_truncates_to_midnight() { @@ -199,6 +213,20 @@ public async Task ToStartOfInterval_with_minute_unit_truncates_to_bucket() Assert.Equal(new DateTime(2026, 8, 10, 13, 45, 0), result); } + [Fact] + public async Task ToStartOfInterval_accepts_parameterized_size() + { + var size = 15; + + await using var context = new DateTimeDbContext(_fixture.ConnectionString); + var query = context.Events.AsNoTracking().Where(e => e.Id == 1) + .Select(e => EF.Functions.ToStartOfInterval(e.Timestamp, size, ClickHouseInterval.Minute)); + + Assert.Contains("{size:Int32}", query.ToQueryString()); + var result = await query.SingleAsync(); + Assert.Equal(new DateTime(2026, 8, 10, 13, 45, 0), result); + } + [Fact] public async Task ToStartOfInterval_with_hour_unit_truncates_to_bucket() { @@ -395,4 +423,33 @@ public void ToStartOf_with_constant_argument_is_evaluated_server_side() Assert.Contains("toStartOfMonth", sql); } + + [Fact] + public void ToStartOfWeek_with_row_dependent_mode_is_not_translatable() + { + using var context = new OfflineContext(); + + var query = context.Events + .Select(e => EF.Functions.ToStartOfWeek(e.Timestamp, (byte)e.Id)); + + var exception = Assert.Throws(() => query.ToQueryString()); + Assert.Contains("'ToStartOfWeek' method's 'mode' argument", exception.Message); + Assert.Contains("SQL literal or query parameter", exception.Message); + } + + [Fact] + public void ToStartOfInterval_with_row_dependent_size_is_not_translatable() + { + using var context = new OfflineContext(); + + var query = context.Events + .Select(e => EF.Functions.ToStartOfInterval( + e.Timestamp, + (int)e.Id, + ClickHouseInterval.Minute)); + + var exception = Assert.Throws(() => query.ToQueryString()); + Assert.Contains("'ToStartOfInterval' method's 'value' argument", exception.Message); + Assert.Contains("SQL literal or query parameter", exception.Message); + } }