Skip to content

Implement SQLite TimeSpan translation#37804

Open
buhaiov-vs wants to merge 2 commits intodotnet:mainfrom
buhaiov-vs:fix-sqlite-timespan-translation
Open

Implement SQLite TimeSpan translation#37804
buhaiov-vs wants to merge 2 commits intodotnet:mainfrom
buhaiov-vs:fix-sqlite-timespan-translation

Conversation

@buhaiov-vs
Copy link

  • Add ef_days and ef_timespan UDFs in SqliteRelationalConnection
  • Add SqliteTimeSpanMemberTranslator and SqliteTimeSpanMethodTranslator
  • Translate TimeSpan binary ops and DateTime ± TimeSpan in SqliteSqlTranslatingExpressionVisitor
  • Translate Max/Min(TimeSpan) in SqliteQueryableAggregateMethodTranslator
  • Add TimeSpan and DateTime Subtract_and_TotalDays SQLite tests

Fixes #18844

@buhaiov-vs
Copy link
Author

@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>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_days and ef_timespan UDFs on the SQLite connection, and add SqliteTimeSpanMemberTranslator and SqliteTimeSpanMethodTranslator for member/method translations
  • Add TryTranslateTimeSpanDateTimeBinary and unary negate handling in SqliteSqlTranslatingExpressionVisitor, plus Min/Max aggregate support in SqliteQueryableAggregateMethodTranslator
  • Update test SQL baselines for TimeSpan member tests and add/rename the Subtract_and_TotalDays test 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_TotalDaysSubtract_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,
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
value => value == null ? null : TimeSpan.Parse(value).TotalDays,
value => value == null ? null : TimeSpan.Parse(value, CultureInfo.InvariantCulture).TotalDays,

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQLite: Translate TimeSpan members

3 participants