Implement SQLite TimeSpan translation#37804
Conversation
|
@dotnet-policy-service agree |
- Use constructor parameter sqlExpressionFactory instead of _sqlExpressionFactory in Max/Min TimeSpan branches Fixes dotnet#18844 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR implements SQLite TimeSpan member/method/operator translations by registering two UDFs (ef_days, ef_timespan) on the SQLite connection and adding translators for TimeSpan members, methods, binary operators, and aggregate operations.
Changes:
- Register
ef_daysandef_timespanUDFs on the SQLite connection, and addSqliteTimeSpanMemberTranslatorandSqliteTimeSpanMethodTranslatorfor member/method translations - Add
TryTranslateTimeSpanDateTimeBinaryand unary negate handling inSqliteSqlTranslatingExpressionVisitor, plus Min/Max aggregate support inSqliteQueryableAggregateMethodTranslator - Update test SQL baselines for TimeSpan member tests and add/rename the
Subtract_and_TotalDaystest across SQLite/SQL Server/Cosmos providers
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs |
Registers ef_days (two overloads: TimeSpan? and string?) and ef_timespan UDFs |
src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteTimeSpanMemberTranslator.cs |
New file translating TimeSpan properties (Hours, Minutes, etc.) via ef_days |
src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteTimeSpanMethodTranslator.cs |
New file translating TimeSpan methods (Duration, FromDays, FromHours, etc.) |
src/EFCore.Sqlite.Core/Query/Internal/SqliteSqlTranslatingExpressionVisitor.cs |
Removes TimeSpan from restricted binary expressions and adds new TimeSpan/DateTime binary operator translations |
src/EFCore.Sqlite.Core/Query/Internal/SqliteSqlExpressionFactory.cs |
Adds EfDays and EfTimespan helper factory methods |
src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteQueryableAggregateMethodTranslator.cs |
Enables Min/Max aggregate translations for TimeSpan |
src/EFCore.Sqlite.Core/Query/Internal/SqliteMemberTranslatorProvider.cs |
Registers new SqliteTimeSpanMemberTranslator |
src/EFCore.Sqlite.Core/Query/Internal/SqliteMethodCallTranslatorProvider.cs |
Registers new SqliteTimeSpanMethodTranslator |
test/EFCore.Specification.Tests/Query/Translations/Temporal/DateTimeTranslationsTestBase.cs |
Renames subtract_and_TotalDays → Subtract_and_TotalDays |
test/EFCore.Sqlite.FunctionalTests/Query/Translations/Temporal/TimeSpanTranslationsSqliteTest.cs |
Updates SQL baselines for member translation tests |
test/EFCore.Sqlite.FunctionalTests/Query/Translations/Temporal/DateTimeTranslationsSqliteTest.cs |
Adds SQL baseline for Subtract_and_TotalDays |
test/EFCore.SqlServer.FunctionalTests/Query/Translations/Temporal/DateTimeTranslationsSqlServerTest.cs |
Renames override to match base class rename |
test/EFCore.Cosmos.FunctionalTests/Query/Translations/Temporal/DateTimeTranslationsCosmosTest.cs |
Renames override to match base class rename |
|
|
||
| sqliteConnection.CreateFunction<string?, double?>( | ||
| "ef_days", | ||
| value => value == null ? null : TimeSpan.Parse(value).TotalDays, |
There was a problem hiding this comment.
The TimeSpan.Parse(value) call in the ef_days string overload (line 201) uses the current thread's culture. While the default TimeSpan.ToString() format ("c") is culture-invariant and can be parsed successfully, it is safer to explicitly use TimeSpan.Parse(value, CultureInfo.InvariantCulture) to avoid potential issues with culture-specific format variations, consistent with how decimal.Parse is called with CultureInfo.InvariantCulture in the same class (line 191).
| value => value == null ? null : TimeSpan.Parse(value).TotalDays, | |
| value => value == null ? null : TimeSpan.Parse(value, CultureInfo.InvariantCulture).TotalDays, |
ef_daysandef_timespanUDFs inSqliteRelationalConnectionSqliteTimeSpanMemberTranslatorandSqliteTimeSpanMethodTranslatorTimeSpanbinary ops andDateTime±TimeSpaninSqliteSqlTranslatingExpressionVisitorMax/Min(TimeSpan)inSqliteQueryableAggregateMethodTranslatorTimeSpanandDateTimeSubtract_and_TotalDaysSQLite testsFixes #18844