Skip to content

Commit 18eef20

Browse files
matrivmatriv
committed
SQL: Fix issues with WEEK/ISO_WEEK/DATEDIFF (#49405)
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)
1 parent 0d91bd6 commit 18eef20

File tree

8 files changed

+124
-22
lines changed

8 files changed

+124
-22
lines changed

‎x-pack/plugin/sql/qa/src/main/resources/date.csv-spec

+4-4
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ YEAR(CAST(birth_date AS DATE)) y,
8383
birth_date, last_name l FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
8484

8585
d:i | dm:i | dw:i | dy:i | iso_dw:i | w:i |iso_w:i | q:i | y:i | birth_date:ts | l:s
86-
2 |2 |4 |245 |3 |36 |35 |3 |1953 |1953-09-02T00:00:00Z |Facello
87-
2 |2 |3 |154 |2 |23 |22 |2 |1964 |1964-06-02T00:00:00Z |Simmel
86+
2 |2 |4 |245 |3 |36 |36 |3 |1953 |1953-09-02T00:00:00Z |Facello
87+
2 |2 |3 |154 |2 |23 |23 |2 |1964 |1964-06-02T00:00:00Z |Simmel
8888
3 |3 |5 |337 |4 |49 |49 |4 |1959 |1959-12-03T00:00:00Z |Bamford
89-
1 |1 |7 |121 |6 |18 |18 |2 |1954 |1954-05-01T00:00:00Z |Koblick
89+
1 |1 |7 |121 |6 |18 |17 |2 |1954 |1954-05-01T00:00:00Z |Koblick
9090
21 |21 |6 |21 |5 |4 |3 |1 |1955 |1955-01-21T00:00:00Z |Maliniak
91-
20 |20 |2 |110 |1 |17 |16 |2 |1953 |1953-04-20T00:00:00Z |Preusig
91+
20 |20 |2 |110 |1 |17 |17 |2 |1953 |1953-04-20T00:00:00Z |Preusig
9292
23 |23 |5 |143 |4 |21 |21 |2 |1957 |1957-05-23T00:00:00Z |Zielinski
9393
19 |19 |4 |50 |3 |8 |8 |1 |1958 |1958-02-19T00:00:00Z |Kalloufi
9494
19 |19 |7 |110 |6 |16 |16 |2 |1952 |1952-04-19T00:00:00Z |Peac

‎x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec

+2-3
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ SELECT birth_date, MAX(hire_date) - INTERVAL 1 YEAR AS f FROM test_emp GROUP BY
311311
;
312312

313313
monthOfDatePlusInterval_And_GroupBy
314-
SELECT WEEK_OF_YEAR(birth_date + INTERVAL 25 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 3 ORDER BY c DESC;
314+
SELECT WEEK_OF_YEAR(birth_date + INTERVAL 25 YEAR) x, COUNT(*) c FROM test_emp GROUP BY x HAVING c >= 3 ORDER BY c DESC, x ASC;
315315

316316
x:i | c:l
317317
---------------+---------------
@@ -322,8 +322,7 @@ null |10
322322
30 |4
323323
40 |4
324324
45 |4
325-
1 |3
326-
8 |3
325+
8 |3
327326
21 |3
328327
28 |3
329328
32 |3

‎x-pack/plugin/sql/qa/src/main/resources/datetime.csv-spec

+10-1
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,15 @@ SELECT WEEK(birth_date) week, birth_date FROM test_emp ORDER BY WEEK(birth_date)
108108
44 |1961-11-02T00:00:00.000Z
109109
;
110110

111+
weekOfYearVsIsoWeekOfYearEdgeCases
112+
SELECT ISO_WEEK_OF_YEAR('2005-01-01T00:00:00.000Z'::datetime) AS "isow2005", WEEK('2005-01-01T00:00:00.000Z'::datetime) AS "w2005",
113+
ISO_WEEK_OF_YEAR('2007-12-31T00:00:00.000Z'::datetime) AS "isow2007", WEEK('2007-12-31T00:00:00.000Z'::datetime) AS "w2007";
114+
115+
isow2005 | w2005 | isow2007 | w2007
116+
---------------+---------------+---------------+---------------
117+
53 |1 |1 |53
118+
;
119+
111120
weekOfYearWithFilter
112121
SELECT WEEK(birth_date) week, birth_date FROM test_emp WHERE WEEK(birth_date) > 50 OR WEEK(birth_date) < 4 ORDER BY WEEK(birth_date) DESC, birth_date DESC;
113122

@@ -404,4 +413,4 @@ SELECT CAST (CAST (birth_date AS VARCHAR) AS TIMESTAMP) a FROM test_emp WHERE YE
404413
a:ts
405414
---------------
406415
1965-01-03T00:00:00Z
407-
;
416+
;

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.time.ZoneId;
1414
import java.time.ZonedDateTime;
1515
import java.time.temporal.ChronoField;
16+
import java.time.temporal.WeekFields;
1617
import java.util.Objects;
1718

1819
public class DateTimeProcessor extends BaseDateTimeProcessor {
@@ -36,7 +37,11 @@ public enum DateTimeExtractor {
3637
}
3738

3839
public int extract(ZonedDateTime dt) {
39-
return dt.get(field);
40+
if (field == ChronoField.ALIGNED_WEEK_OF_YEAR) {
41+
return dt.get(WeekFields.ISO.weekOfWeekBasedYear());
42+
} else {
43+
return dt.get(field);
44+
}
4045
}
4146

4247
public int extract(OffsetTime time) {

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NonIsoDateTimeProcessor.java

+2-13
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,11 @@
1010
import org.elasticsearch.common.io.stream.StreamOutput;
1111

1212
import java.io.IOException;
13-
import java.time.LocalDateTime;
1413
import java.time.ZoneId;
1514
import java.time.ZonedDateTime;
1615
import java.time.temporal.ChronoField;
17-
import java.util.Calendar;
18-
import java.util.Locale;
16+
import java.time.temporal.WeekFields;
1917
import java.util.Objects;
20-
import java.util.TimeZone;
2118
import java.util.function.Function;
2219

2320
public class NonIsoDateTimeProcessor extends BaseDateTimeProcessor {
@@ -30,15 +27,7 @@ public enum NonIsoDateTimeExtractor {
3027
return dayOfWeek == 8 ? 1 : dayOfWeek;
3128
}),
3229
WEEK_OF_YEAR(zdt -> {
33-
// by ISO 8601 standard, the first week of a year is the first week with a majority (4 or more) of its days in January.
34-
// Other Locales may have their own standards (see Arabic or Japanese calendars).
35-
LocalDateTime ld = zdt.toLocalDateTime();
36-
Calendar cal = Calendar.getInstance(TimeZone.getTimeZone(zdt.getZone()), Locale.ROOT);
37-
cal.clear();
38-
cal.set(ld.get(ChronoField.YEAR), ld.get(ChronoField.MONTH_OF_YEAR) - 1, ld.get(ChronoField.DAY_OF_MONTH),
39-
ld.get(ChronoField.HOUR_OF_DAY), ld.get(ChronoField.MINUTE_OF_HOUR), ld.get(ChronoField.SECOND_OF_MINUTE));
40-
41-
return cal.get(Calendar.WEEK_OF_YEAR);
30+
return zdt.get(WeekFields.SUNDAY_START.weekOfYear());
4231
});
4332

4433
private final Function<ZonedDateTime, Integer> apply;

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessorTests.java

+46
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,29 @@ public void testApply_withTimezoneUTC() {
5353
assertEquals(1, proc.process(dateTime(0L)));
5454
assertEquals(2, proc.process(dateTime(2017, 01, 02, 10, 10)));
5555
assertEquals(31, proc.process(dateTime(2017, 01, 31, 10, 10)));
56+
57+
// Tested against MS-SQL Server and H2
58+
proc = new DateTimeProcessor(DateTimeExtractor.ISO_WEEK_OF_YEAR, UTC);
59+
assertEquals(1, proc.process(dateTime(1988, 1, 5, 0, 0, 0, 0)));
60+
assertEquals(5, proc.process(dateTime(2001, 2, 4, 0, 0, 0, 0)));
61+
assertEquals(6, proc.process(dateTime(1977, 2, 8, 0, 0, 0, 0)));
62+
assertEquals(11, proc.process(dateTime(1974, 3, 17, 0, 0, 0, 0)));
63+
assertEquals(16, proc.process(dateTime(1977, 4, 20, 0, 0, 0, 0)));
64+
assertEquals(16, proc.process(dateTime(1994, 4, 20, 0, 0, 0, 0)));
65+
assertEquals(17, proc.process(dateTime(2002, 4, 27, 0, 0, 0, 0)));
66+
assertEquals(18, proc.process(dateTime(1974, 5, 3, 0, 0, 0, 0)));
67+
assertEquals(22, proc.process(dateTime(1997, 5, 30, 0, 0, 0, 0)));
68+
assertEquals(22, proc.process(dateTime(1995, 6, 4, 0, 0, 0, 0)));
69+
assertEquals(28, proc.process(dateTime(1972, 7, 12, 0, 0, 0, 0)));
70+
assertEquals(30, proc.process(dateTime(1980, 7, 26, 0, 0, 0, 0)));
71+
assertEquals(33, proc.process(dateTime(1998, 8, 12, 0, 0, 0, 0)));
72+
assertEquals(35, proc.process(dateTime(1995, 9, 3, 0, 0, 0, 0)));
73+
assertEquals(37, proc.process(dateTime(1976, 9, 9, 0, 0, 0, 0)));
74+
assertEquals(38, proc.process(dateTime(1997, 9, 19, 0, 0, 0, 0)));
75+
assertEquals(45, proc.process(dateTime(1980, 11, 7, 0, 0, 0, 0)));
76+
assertEquals(53, proc.process(dateTime(2005, 1, 1, 0, 0, 0, 0)));
77+
assertEquals(1, proc.process(dateTime(2007, 12, 31, 0, 0, 0, 0)));
78+
assertEquals(1, proc.process(dateTime(2019, 12, 31, 20, 22, 33, 987654321)));
5679
}
5780

5881
public void testApply_withTimezoneOtherThanUTC() {
@@ -62,6 +85,29 @@ public void testApply_withTimezoneOtherThanUTC() {
6285

6386
proc = new DateTimeProcessor(DateTimeExtractor.DAY_OF_MONTH, zoneId);
6487
assertEquals(1, proc.process(dateTime(2017, 12, 31, 20, 30)));
88+
89+
// Tested against MS-SQL Server and H2
90+
proc = new DateTimeProcessor(DateTimeExtractor.ISO_WEEK_OF_YEAR, UTC);
91+
assertEquals(1, proc.process(dateTime(1988, 1, 5, 0, 0, 0, 0)));
92+
assertEquals(5, proc.process(dateTime(2001, 2, 4, 0, 0, 0, 0)));
93+
assertEquals(6, proc.process(dateTime(1977, 2, 8, 0, 0, 0, 0)));
94+
assertEquals(11, proc.process(dateTime(1974, 3, 17, 0, 0, 0, 0)));
95+
assertEquals(16, proc.process(dateTime(1977, 4, 20, 0, 0, 0, 0)));
96+
assertEquals(16, proc.process(dateTime(1994, 4, 20, 0, 0, 0, 0)));
97+
assertEquals(17, proc.process(dateTime(2002, 4, 27, 0, 0, 0, 0)));
98+
assertEquals(18, proc.process(dateTime(1974, 5, 3, 0, 0, 0, 0)));
99+
assertEquals(22, proc.process(dateTime(1997, 5, 30, 0, 0, 0, 0)));
100+
assertEquals(22, proc.process(dateTime(1995, 6, 4, 0, 0, 0, 0)));
101+
assertEquals(28, proc.process(dateTime(1972, 7, 12, 0, 0, 0, 0)));
102+
assertEquals(30, proc.process(dateTime(1980, 7, 26, 0, 0, 0, 0)));
103+
assertEquals(33, proc.process(dateTime(1998, 8, 12, 0, 0, 0, 0)));
104+
assertEquals(35, proc.process(dateTime(1995, 9, 3, 0, 0, 0, 0)));
105+
assertEquals(37, proc.process(dateTime(1976, 9, 9, 0, 0, 0, 0)));
106+
assertEquals(38, proc.process(dateTime(1997, 9, 19, 0, 0, 0, 0)));
107+
assertEquals(45, proc.process(dateTime(1980, 11, 7, 0, 0, 0, 0)));
108+
assertEquals(53, proc.process(dateTime(2005, 1, 1, 0, 0, 0, 0)));
109+
assertEquals(1, proc.process(dateTime(2007, 12, 31, 0, 0, 0, 0)));
110+
assertEquals(1, proc.process(dateTime(2019, 12, 31, 20, 22, 33, 987654321)));
65111
}
66112

67113
public void testFailOnTime() {

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java

+10
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.elasticsearch.xpack.sql.util.DateUtils;
1010

1111
import java.time.OffsetTime;
12+
import java.time.ZoneId;
1213
import java.time.ZoneOffset;
1314
import java.time.ZonedDateTime;
1415

@@ -20,6 +21,15 @@ public static ZonedDateTime dateTime(int year, int month, int day, int hour, int
2021
return ZonedDateTime.of(year, month, day, hour, minute, 0, 0, DateUtils.UTC);
2122
}
2223

24+
public static ZonedDateTime dateTime(int year, int month, int day, int hour, int minute, int seconds, int nanos) {
25+
return dateTime(year, month, day, hour, minute, seconds, nanos, DateUtils.UTC);
26+
}
27+
28+
public static ZonedDateTime dateTime(int year, int month, int day, int hour, int minute, int seconds, int nanos,
29+
ZoneId zoneId) {
30+
return ZonedDateTime.of(year, month, day, hour, minute, seconds, nanos, zoneId);
31+
}
32+
2333
public static ZonedDateTime dateTime(long millisSinceEpoch) {
2434
return DateUtils.asDateTime(millisSinceEpoch);
2535
}

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/NonIsoDateTimeProcessorTests.java

+44
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,28 @@ public void testNonISOWeekOfYearInUTC() {
5454
assertEquals(17, proc.process(dateTime(766833730000L))); //1994-04-20T09:22:10Z[UTC]
5555
assertEquals(29, proc.process(dateTime(79780930000L))); //1972-07-12T09:22:10Z[UTC]
5656
assertEquals(33, proc.process(dateTime(902913730000L))); //1998-08-12T09:22:10Z[UTC]
57+
58+
// Tested against MS-SQL Server and H2
59+
assertEquals(2, proc.process(dateTime(1988, 1, 5, 0, 0, 0, 0)));
60+
assertEquals(6, proc.process(dateTime(2001, 2, 4, 0, 0, 0, 0)));
61+
assertEquals(7, proc.process(dateTime(1977, 2, 8, 0, 0, 0, 0)));
62+
assertEquals(12, proc.process(dateTime(1974, 3, 17, 0, 0, 0, 0)));
63+
assertEquals(17, proc.process(dateTime(1977, 4, 20, 0, 0, 0, 0)));
64+
assertEquals(17, proc.process(dateTime(1994, 4, 20, 0, 0, 0, 0)));
65+
assertEquals(17, proc.process(dateTime(2002, 4, 27, 0, 0, 0, 0)));
66+
assertEquals(18, proc.process(dateTime(1974, 5, 3, 0, 0, 0, 0)));
67+
assertEquals(22, proc.process(dateTime(1997, 5, 30, 0, 0, 0, 0)));
68+
assertEquals(23, proc.process(dateTime(1995, 6, 4, 0, 0, 0, 0)));
69+
assertEquals(29, proc.process(dateTime(1972, 7, 12, 0, 0, 0, 0)));
70+
assertEquals(30, proc.process(dateTime(1980, 7, 26, 0, 0, 0, 0)));
71+
assertEquals(33, proc.process(dateTime(1998, 8, 12, 0, 0, 0, 0)));
72+
assertEquals(36, proc.process(dateTime(1995, 9, 3, 0, 0, 0, 0)));
73+
assertEquals(37, proc.process(dateTime(1976, 9, 9, 0, 0, 0, 0)));
74+
assertEquals(38, proc.process(dateTime(1997, 9, 19, 0, 0, 0, 0)));
75+
assertEquals(45, proc.process(dateTime(1980, 11, 7, 0, 0, 0, 0)));
76+
assertEquals(1, proc.process(dateTime(2005, 1, 1, 0, 0, 0, 0)));
77+
assertEquals(53, proc.process(dateTime(2007, 12, 31, 0, 0, 0, 0)));
78+
assertEquals(53, proc.process(dateTime(2019, 12, 31, 20, 22, 33, 987654321)));
5779
}
5880

5981
public void testNonISOWeekOfYearInNonUTCTimeZone() {
@@ -67,6 +89,28 @@ public void testNonISOWeekOfYearInNonUTCTimeZone() {
6789
assertEquals(17, proc.process(dateTime(766833730000L)));
6890
assertEquals(29, proc.process(dateTime(79780930000L)));
6991
assertEquals(33, proc.process(dateTime(902913730000L)));
92+
93+
// Tested against MS-SQL Server and H2
94+
assertEquals(2, proc.process(dateTime(1988, 1, 5, 0, 0, 0, 0)));
95+
assertEquals(5, proc.process(dateTime(2001, 2, 4, 0, 0, 0, 0)));
96+
assertEquals(7, proc.process(dateTime(1977, 2, 8, 0, 0, 0, 0)));
97+
assertEquals(11, proc.process(dateTime(1974, 3, 17, 0, 0, 0, 0)));
98+
assertEquals(17, proc.process(dateTime(1977, 4, 20, 0, 0, 0, 0)));
99+
assertEquals(17, proc.process(dateTime(1994, 4, 20, 0, 0, 0, 0)));
100+
assertEquals(17, proc.process(dateTime(2002, 4, 27, 0, 0, 0, 0)));
101+
assertEquals(18, proc.process(dateTime(1974, 5, 3, 0, 0, 0, 0)));
102+
assertEquals(22, proc.process(dateTime(1997, 5, 30, 0, 0, 0, 0)));
103+
assertEquals(22, proc.process(dateTime(1995, 6, 4, 0, 0, 0, 0)));
104+
assertEquals(29, proc.process(dateTime(1972, 7, 12, 0, 0, 0, 0)));
105+
assertEquals(30, proc.process(dateTime(1980, 7, 26, 0, 0, 0, 0)));
106+
assertEquals(33, proc.process(dateTime(1998, 8, 12, 0, 0, 0, 0)));
107+
assertEquals(35, proc.process(dateTime(1995, 9, 3, 0, 0, 0, 0)));
108+
assertEquals(37, proc.process(dateTime(1976, 9, 9, 0, 0, 0, 0)));
109+
assertEquals(38, proc.process(dateTime(1997, 9, 19, 0, 0, 0, 0)));
110+
assertEquals(45, proc.process(dateTime(1980, 11, 7, 0, 0, 0, 0)));
111+
assertEquals(53, proc.process(dateTime(2005, 1, 1, 0, 0, 0, 0)));
112+
assertEquals(53, proc.process(dateTime(2007, 12, 31, 0, 0, 0, 0)));
113+
assertEquals(53, proc.process(dateTime(2019, 12, 31, 20, 22, 33, 987654321)));
70114
}
71115

72116
public void testNonISODayOfWeekInUTC() {

0 commit comments

Comments
 (0)