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

SQL: incorrect ISO_WEEK_OF_YEAR and WEEK_OF_YEAR #49376

Closed
astefan opened this issue Nov 20, 2019 · 3 comments · Fixed by #49405
Closed

SQL: incorrect ISO_WEEK_OF_YEAR and WEEK_OF_YEAR #49376

astefan opened this issue Nov 20, 2019 · 3 comments · Fixed by #49405
Assignees
Labels

Comments

@astefan
Copy link
Contributor

astefan commented Nov 20, 2019

According to ISO 8601 standard, the week of an year is calculated following few rules:

  • Weeks start with Monday
  • Each week's year is the Gregorian year in which the Thursday falls
  • The definition for week 01 is the week with the Gregorian year's first Thursday in it. The following statements are true, as well:
    • It is the first week with a majority (4 or more) of its days in January.
    • Its first day is the Monday nearest to 1 January.
    • It has 4 January in it. Hence the earliest possible first week extends from Monday 29 December (previous Gregorian year) to Sunday 4 January, the latest possible first week extends from Monday 4 January to Sunday 10 January.
    • It has the year's first working day in it, if Saturdays, Sundays and 1 January are not working days.

Examples:

  • January 1st, 2005 was on a Saturday. According to Gregorian calendar, it's week 1 of year 2005. According to ISO standard, it's week 53 of year 2004 because the first week of 2005 starts on Monday, January 3rd.
  • December 31st, 2007 was on a Monday. According to Gregorian calendar, it's the last week of year 2007. According to ISO standard, it's week 1 or year 2008 because it starts on Monday and because most of its days are in January.

In case of ES SQL, we have:

SELECT ISO_WEEK_OF_YEAR(CAST('2005-01-01T00:00:00Z' AS DATETIME)) isow2005, WEEK(CAST('2005-01-01T00:00:00Z' AS DATETIME)) AS w2005, ISO_WEEK_OF_YEAR(CAST('2007-12-31T00:00:00Z' AS DATETIME)) isow2007, WEEK(CAST('2007-12-31T00:00:00Z' AS DATETIME)) AS w2007

Which results in

   isow2005    |     w2005     |   isow2007    |     w2007     
---------------+---------------+---------------+---------------
1              |1              |53             |1              

And it should have resulted in 53 |1 |1 |53

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/SQL)

@astefan
Copy link
Contributor Author

astefan commented Nov 25, 2019

Sql Server 2017: SELECT DATEPART(WEEK, '1/1/2005') AS w2005, DATEPART(ISO_WEEK, '1/1/2005') AS isow2005, DATEPART(WEEK, '12/31/2007') AS w2007, DATEPART(ISO_WEEK, '12/31/2007') AS isow2007 gives

w2005 isow2005 w2007 isow2007
1 53 53 1

PostgreSQL 12: EXTRACT(WEEK FROM TIMESTAMP '2005-01-01 00:00:00') gives

isow2005 isow2007
53 1

matriv added a commit to matriv/elasticsearch that referenced this issue Nov 27, 2019
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since elastic#48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: elastic#49376
matriv added a commit that referenced this issue Nov 29, 2019
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since #48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: #49376
matriv added a commit that referenced this issue Nov 29, 2019
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since #48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: #49376

(cherry picked from commit 54fe7f5)
matriv added a commit that referenced this issue Nov 29, 2019
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since #48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: #49376

(cherry picked from commit 54fe7f5)
matriv added a commit that referenced this issue Nov 29, 2019
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since #48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: #49376

(cherry picked from commit 54fe7f5)
@matriv
Copy link
Contributor

matriv commented Nov 29, 2019

master : 54fe7f5
7.x : 901a8d1
7.5 : 57d5503

For the following branch DATE_DIFF wasn't yet implemented, so only WEEK/ISO_WEEK
are fixed:
7.4 : 18eef20

SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
Some extended testing with MS-SQL server and H2 (which agree on
results) revealed bugs in the implementation of WEEK related extraction
and diff functions.

Non-iso WEEK seems to be broken since elastic#48209 because
of the replacement of Calendar and the change in the ISO rules.

ISO_WEEK failed for some edge cases around the January 1st.

DATE_DIFF was previously based on non-iso WEEK extraction which seems
not to be the case.

Fixes: elastic#49376
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants