-
Notifications
You must be signed in to change notification settings - Fork 25k
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: Fix the inconsistent behaviour of ISO_WEEK_YEAR() #68758
Changes from 1 commit
835ac7b
8c8b01f
110858c
da98724
dc33982
0d74049
4e8689b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,6 +131,32 @@ SELECT WEEK(birth_date) week, birth_date FROM test_emp ORDER BY WEEK(birth_date) | |
44 |1961-11-02T00:00:00.000Z | ||
; | ||
|
||
isoWeekOfYear | ||
schema::birth_date:ts|iso_week:i|week:i | ||
SELECT birth_date, IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 8 ORDER BY iso_week; | ||
|
||
birth_date | iso_week | week | ||
------------------------+---------------+--------------- | ||
1953-01-07T00:00:00.000Z|2 |2 | ||
1955-01-21T00:00:00.000Z|3 |4 | ||
1953-01-23T00:00:00.000Z|4 |4 | ||
1958-01-21T00:00:00.000Z|4 |4 | ||
1959-01-27T00:00:00.000Z|5 |5 | ||
1956-02-12T00:00:00.000Z|6 |7 | ||
1953-02-08T00:00:00.000Z|6 |7 | ||
1960-02-20T00:00:00.000Z|7 |8 | ||
; | ||
|
||
isoWeekOfYearFilterEquality | ||
SELECT ISO_WEEK_OF_YEAR(CONCAT(CONCAT('2021-01-22T14:26:06.', (salary % 2)::text), 'Z')::datetime) AS iso_week | ||
FROM test_emp WHERE iso_week = 3 LIMIT 2; | ||
|
||
iso_week:i | ||
--------------- | ||
3 | ||
3 | ||
; | ||
|
||
weekOfYearVsIsoWeekOfYearEdgeCases | ||
SELECT ISO_WEEK_OF_YEAR('2005-01-01T00:00:00.000Z'::datetime) AS "isow2005", WEEK('2005-01-01T00:00:00.000Z'::datetime) AS "w2005", | ||
ISO_WEEK_OF_YEAR('2007-12-31T00:00:00.000Z'::datetime) AS "isow2007", WEEK('2007-12-31T00:00:00.000Z'::datetime) AS "w2007"; | ||
|
@@ -1303,24 +1329,25 @@ null |null |10 | |
dateTimeAggByIsoWeekOfYear | ||
SELECT IW(birth_date) iso_week, WEEK(birth_date) week FROM test_emp WHERE IW(birth_date) < 20 GROUP BY iso_week, week ORDER BY iso_week; | ||
|
||
iso_week:i | week:i | ||
iso_week:i | week:i | ||
---------------+--------------- | ||
1 |2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: "manual" checking the csv specs sometimes pays off. |
||
2 |2 | ||
3 |4 | ||
4 |4 | ||
4 |5 | ||
5 |5 | ||
6 |7 | ||
7 |7 | ||
7 |8 | ||
8 |8 | ||
8 |9 | ||
9 |9 | ||
10 |11 | ||
12 |12 | ||
14 |14 | ||
14 |15 | ||
15 |15 | ||
15 |16 | ||
16 |16 | ||
16 |17 | ||
17 |17 | ||
17 |18 | ||
18 |18 | ||
; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,13 +15,13 @@ | |
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateDiffProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DatePartProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFormatProcessor.Formatter; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeParseProcessor.Parser; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTruncProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NamedDateTimeProcessor.NameExtractor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor.NonIsoDateTimeExtractor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.QuarterProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.TimeFunction; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.TimeProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.geo.GeoProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StDistanceProcessor; | ||
import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StWkttosqlProcessor; | ||
|
@@ -221,14 +221,14 @@ public static Number tan(Number value) { | |
// | ||
// Date/Time functions | ||
// | ||
public static Integer dateTimeChrono(Object dateTime, String tzId, String chronoName) { | ||
if (dateTime == null || tzId == null || chronoName == null) { | ||
public static Integer dateTimeExtract(Object dateTime, String tzId, String extractorName) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have a strong preference regarding the method rename however there is an associated cost for it in backwards compatibility. To avoid this, simply keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, good catch. It is not just a simple method rename, also the possible values of the last parameter changed (from |
||
if (dateTime == null || tzId == null || extractorName == null) { | ||
return null; | ||
} | ||
if (dateTime instanceof OffsetTime) { | ||
return TimeFunction.dateTimeChrono((OffsetTime) dateTime, tzId, chronoName); | ||
return TimeProcessor.doProcess((OffsetTime) dateTime, tzId, extractorName); | ||
} | ||
return DateTimeFunction.dateTimeChrono(asDateTime(dateTime), tzId, chronoName); | ||
return DateTimeProcessor.doProcess(asDateTime(dateTime), tzId, extractorName); | ||
} | ||
|
||
public static String dayName(Object dateTime, String tzId) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: maybe you can add a
AND week > 2
so we can check both filters work and one filter uses also the alias`