Repository navigation
Use the last day of the month for AddMonths and AddYears on SQLite - #39548
Open
officialasishkumar wants to merge 2 commits into
Open
officialasishkumar wants to merge 2 commits into
officialasishkumar wants to merge 2 commits into
Conversation
SQLite resolves a day that doesn't exist in the resulting month by overflowing into the next month, so 2023-01-31 + 1 month was 2023-03-03 and 2024-02-29 + 1 year was 2025-03-01. .NET and the other providers use the last day of the resulting month, so the query silently returned a different date than the same expression evaluated on the client. - Add the 'floor' modifier after the months and years modifiers for DateTime and DateOnly. SQLite added it in 3.46 for this case; dotnet#25851 was closed as by design before it existed. - Remove the workaround in Select_expression_datetime_add_month that allowed up to 3 days of difference, so it now uses the base test. - Add tests for adding months past the end of a month, in both directions, and for adding years to a leap day. Fixes dotnet#39545
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The unconditional SQLite floor modifier returns NULL on native versions before 3.46, silently dropping matching rows from existing date-addition filters.
2 open issues
What changed in this PR
This PR addresses #39545 by aligning SQLite’s DateTime and DateOnly month/year additions with .NET’s end-of-month clamping behavior.
Changes:
- Adds SQLite’s
floormodifier after each month or year adjustment. - Adds month-end and leap-day regression tests and updates SQL baselines.
- Restores exact date comparison in the Northwind month-addition test.
| File | Description |
|---|---|
test/EFCore.Sqlite.FunctionalTests/Query/Translations/Temporal/DateTimeTranslationsSqliteTest.cs |
Adds DateTime boundary tests and updates the year-addition baseline. |
test/EFCore.Sqlite.FunctionalTests/Query/Translations/Temporal/DateOnlyTranslationsSqliteTest.cs |
Adds DateOnly boundary tests and updates standalone and chained SQL baselines. |
test/EFCore.Sqlite.FunctionalTests/Query/NorthwindSelectQuerySqliteTest.cs |
Updates the year-addition SQL baseline. |
test/EFCore.Sqlite.FunctionalTests/Query/NorthwindMiscellaneousQuerySqliteTest.cs |
Removes the date-tolerance workaround and updates SQL baselines. |
src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteDateTimeMethodTranslator.cs |
Emits floor after month and year modifiers for both date types. |
Review effort: Balanced
| // By default SQLite overflows a day past the end of the resulting month into the next month (e.g. January 31 + 1 month gives | ||
| // March 3); 'floor' makes it the last day of the resulting month instead, which is what DateTime and DateOnly do. | ||
| private SqlExpression[] MakeClampedModifiers(SqlExpression argument, string unitSuffix) | ||
| => [MakeModifier(argument, unitSuffix), sqlExpressionFactory.Constant("floor")]; |
Comment on lines
+247
to
+251
| public virtual async Task AddMonths_past_end_of_month_gives_last_day_of_month() | ||
| { | ||
| // 1998-05-31 + 1 month | ||
| await AssertQuery( | ||
| ss => ss.Set<BasicTypesEntity>().Where(o => o.DateTime.AddDays(27).AddMonths(1) == new DateTime(1998, 6, 30, 15, 30, 10))); |
SQLite returns NULL for an unknown date modifier, so on native versions older than 3.46 every AddMonths and AddYears translation would return NULL. Check the version as the other SQLite feature checks do and keep the previous overflow behavior when 'floor' isn't available.
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 #39545
On SQLite,
AddMonthsandAddYearsonDateTimeandDateOnlyreturned a different date from .NET when the day doesn't exist in the resulting month. SQLite overflows into the next month, so2023-01-31+ 1 month was2023-03-03and2024-02-29+ 1 year was2025-03-01. .NET, SQL Server and PostgreSQL all use the last day of the resulting month. There's no error: the query just returns a wrong date, and a filter on the result silently misses rows.#25851 closed this as by design in 2022, because SQLite had no way to get the other behavior. SQLite 3.46 has since added the
floormodifier for this case.Fix
SqliteDateTimeMethodTranslatornow adds'floor'straight after the months or years modifier:Each
AddMonthsorAddYearscall gets its own'floor', so chained calls clamp at each step as they do in .NET, including whenSqliteSqlExpressionFactorymerges nestedstrftimeanddatecalls into one. Days, hours, minutes, seconds, milliseconds and ticks aren't affected.Tests
DateOnlyTranslationsSqliteTest: new tests for adding months past the end of a month, in both directions, and for adding years to a leap day.DateTimeTranslationsSqliteTest: new tests for adding months past the end of a month and for adding years to a leap day.Select_expression_datetime_add_month: removed the workaround that allowed up to 3 days of difference, so the SQLite test now runs the base test with exact comparison.'floor'modifier.Without the change, the new tests and
Select_expression_datetime_add_monthfail on the data, not only on the SQL (for exampleExpected: 1996-11-30,Actual: 1996-12-01on Northwind). With it, all ofEFCore.Sqlite.FunctionalTestspasses locally (38714 passed, 284 skipped).Behavior notes
This changes the results of existing queries when the day doesn't exist in the resulting month. They now match client evaluation and the other providers.
'floor'needs SQLite 3.46 or later. TheSQLitePCLRaw.bundle_e_sqlite3version that EF Core depends on supports it. With an older native SQLite, the translator checks the version and leaves out'floor', so those applications keep the previous overflow behavior instead of gettingNULL.I've read the guidelines for contributing and seen the walkthrough
I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
The code builds and tests pass locally (also verified by our automated build checks)
Commit messages follow this format: