Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query/Sqlite: AddMonths and AddTicks translation may lead to small result discrepancies vs l2o #25851

Closed
maumar opened this issue Sep 2, 2021 · 2 comments · Fixed by #29262
Closed
Assignees
Labels
area-query area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Sep 2, 2021

Tests failing result verification on sql lite:
Select_expression_datetime_add_ticks
Select_expression_datetime_add_month

@maumar
Copy link
Contributor Author

maumar commented Sep 2, 2021

31/10 + 1 month results in 30/11 but on sqlite we translate to 1/12. Moreover, 31/1 + 1 month = 28/2 or 29/2, depending on leap year which might be even more problematic

@roji
Copy link
Member

roji commented Sep 3, 2021

tl;dr Sqlite is indeed the odd one out compared to everything else I'm seeing; this is documented as follows:

Note that "±NNN months" works by rendering the original date into the YYYY-MM-DD format, adding the ±NNN to the MM month value, then normalizing the result. Thus, for example, the data 2001-03-31 modified by '+1 month' initially yields 2001-04-31, but April only has 30 days so the date is normalized to 2001-05-01. A similar effect occurs when the original date is February 29 of a leapyear and the modifier is ±N years where N is not a multiple of four.

This seems to correspond to what Jon Skeet describes as "Work out how far we've overshot, and apply that to the next month", where as everyone else does "Round down to the end of the month", see this article for this fascinating subject. So this should be closed as by design (probably unskip the tests and override to pass).

Results:

PostgreSQL

SELECT '2020-10-31'::date + '1 month'::interval; -- 2020-11-30
SELECT '2020-01-31'::date + '1 month'::interval; -- 2020-02-29

SQL Server

SELECT DATEADD(month, 1, '2020-10-31'); -- 2020-11-30
SELECT DATEADD(month, 1, '2020-01-31'); -- 2020-02-29

Sqlite

SELECT DATE('2020-10-31','+1 months'); -- 2020-12-01
SELECT strftime('%Y-%m-%d', '2020-10-31', '1 months'); -- 2020-12-01

SELECT DATE('2020-01-31','+1 months'); -- 2020-03-02
SELECT strftime('%Y-%m-%d', '2020-01-31', '1 months'); -- 2020-03-02

Nodatime

Console.WriteLine(new LocalDate(2020, 10, 31) + Period.FromMonths(1)); // 2020-11-30
Console.WriteLine(new LocalDate(2020, 1, 31) + Period.FromMonths(1)); // 2020-02-29

.NET DateTime

Console.WriteLine(new DateTime(2020, 10, 31).AddMonths(1)); // 2020-11-30
Console.WriteLine(new DateTime(2020, 1, 31).AddMonths(1)); // 2020-02-29

@ajcvickers ajcvickers added this to the MQ milestone Sep 4, 2021
maumar added a commit that referenced this issue Oct 5, 2022
Sqlite has a peculiar way of adding months, which is by design - we need to compensate in test.

Resolves #25851
maumar added a commit that referenced this issue Oct 5, 2022
Sqlite has a peculiar way of adding months, which is by design - we need to compensate in test.

Resolves #25851
maumar added a commit that referenced this issue Oct 5, 2022
Sqlite has a peculiar way of adding months, which is by design - we need to compensate in test.

Resolves #25851
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 5, 2022
@ghost ghost closed this as completed in #29262 Oct 5, 2022
ghost pushed a commit that referenced this issue Oct 5, 2022
Sqlite has a peculiar way of adding months, which is by design - we need to compensate in test.

Resolves #25851
@ajcvickers ajcvickers modified the milestones: MQ, 8.0.0 Oct 15, 2022
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview1 Jan 29, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants