Skip to content

Commit cf695df

Browse files
committed
SQL: Update unit tests with proper number of weeks
Signed-off-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
1 parent 67b81f1 commit cf695df

File tree

5 files changed

+62
-39
lines changed

5 files changed

+62
-39
lines changed

core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package org.opensearch.sql.expression.datetime;
77

8+
import static java.time.DayOfWeek.SUNDAY;
9+
import static java.time.temporal.TemporalAdjusters.nextOrSame;
810
import static org.junit.jupiter.api.Assertions.assertAll;
911
import static org.junit.jupiter.api.Assertions.assertEquals;
1012
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -23,6 +25,7 @@
2325
import java.time.LocalDate;
2426
import java.time.LocalDateTime;
2527
import java.time.format.DateTimeFormatter;
28+
import java.time.temporal.ChronoUnit;
2629
import java.util.List;
2730
import java.util.stream.Stream;
2831
import lombok.AllArgsConstructor;
@@ -1230,7 +1233,7 @@ public void testWeekFormats(
12301233
@Test
12311234
public void testWeekOfYearWithTimeType() {
12321235
LocalDate today = LocalDate.now(functionProperties.getQueryStartClock());
1233-
int week = DateTimeTestBase.getYearWeek(today);
1236+
int week = getWeekOfYearBeforeSunday(today);
12341237

12351238
assertAll(
12361239
() ->
@@ -1251,6 +1254,15 @@ public void testWeekOfYearWithTimeType() {
12511254
week));
12521255
}
12531256

1257+
private int getWeekOfYearBeforeSunday(LocalDate date) {
1258+
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
1259+
if (date.isBefore(firstSundayOfYear)) {
1260+
return 0;
1261+
}
1262+
1263+
return (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
1264+
}
1265+
12541266
@Test
12551267
public void modeInUnsupportedFormat() {
12561268
FunctionExpression expression1 =

core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@
55

66
package org.opensearch.sql.expression.datetime;
77

8-
import static java.time.DayOfWeek.SUNDAY;
9-
import static java.time.temporal.TemporalAdjusters.nextOrSame;
108
import static org.opensearch.sql.data.model.ExprValueUtils.fromObjectValue;
119

1210
import java.time.Instant;
1311
import java.time.LocalDate;
1412
import java.time.LocalDateTime;
1513
import java.time.LocalTime;
1614
import java.time.ZoneOffset;
17-
import java.time.temporal.ChronoUnit;
1815
import java.time.temporal.Temporal;
1916
import java.util.List;
2017
import org.opensearch.sql.data.model.ExprDateValue;
@@ -234,19 +231,4 @@ protected Double unixTimeStampOf(LocalDateTime value) {
234231
protected Double unixTimeStampOf(Instant value) {
235232
return unixTimeStampOf(DSL.literal(new ExprTimestampValue(value))).valueOf().doubleValue();
236233
}
237-
238-
// The following calculation is needed to correct the discrepancy in how ISO 860 and our
239-
// implementation of YEARWEEK calculates. ISO 8601 calculates weeks using the following criteria:
240-
// - Weeks start on Monday
241-
// - The first week of a year is any week containing 4 or more days in the new year.
242-
// Whereas YEARWEEK counts only full weeks, where weeks start on Sunday. To fix the discrepancy
243-
// we find the first Sunday of the year and start counting weeks from that date.
244-
protected static int getYearWeek(LocalDate date) {
245-
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
246-
int week =
247-
date.isBefore(firstSundayOfYear)
248-
? 52
249-
: (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
250-
return week;
251-
}
252234
}

core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@
55

66
package org.opensearch.sql.expression.datetime;
77

8+
import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR;
89
import static org.junit.jupiter.api.Assertions.assertEquals;
910
import static org.opensearch.sql.data.type.ExprCoreType.LONG;
1011

1112
import java.time.LocalDate;
12-
import java.time.temporal.WeekFields;
13-
import java.util.Locale;
1413
import java.util.stream.Stream;
1514
import org.junit.jupiter.api.Test;
1615
import org.junit.jupiter.params.ParameterizedTest;
@@ -93,15 +92,20 @@ private void datePartWithTimeArgQuery(String part, String time, long expected) {
9392

9493
@Test
9594
public void testExtractDatePartWithTimeType() {
95+
// run this for 4 years worth to get at least one leap year:
9696
LocalDate now = LocalDate.now(functionProperties.getQueryStartClock());
9797

9898
datePartWithTimeArgQuery("DAY", timeInput, now.getDayOfMonth());
99-
100-
datePartWithTimeArgQuery(
101-
"WEEK", timeInput, now.get(WeekFields.of(Locale.ENGLISH).weekOfYear()));
102-
99+
// To avoid flaky test, skip the testing in December and January because the WEEK is ISO 8601
100+
// week-of-week-based-year which is considered to start on a Monday and week 1 is the first week
101+
// with >3 days. it is possible for early-January dates to be part of the 52nd or 53rd week of
102+
// the previous year, and for late-December dates to be part of the first week of the next year.
103+
// For example, 2005-01-02 is part of the 53rd week of year 2004, while 2012-12-31 is part of
104+
// the first week of 2013
105+
if (now.getMonthValue() != 1 && now.getMonthValue() != 12) {
106+
datePartWithTimeArgQuery("WEEK", datetimeInput, now.get(ALIGNED_WEEK_OF_YEAR));
107+
}
103108
datePartWithTimeArgQuery("MONTH", timeInput, now.getMonthValue());
104-
105109
datePartWithTimeArgQuery("YEAR", timeInput, now.getYear());
106110
}
107111

core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@
55

66
package org.opensearch.sql.expression.datetime;
77

8+
import static java.time.DayOfWeek.SUNDAY;
9+
import static java.time.temporal.TemporalAdjusters.nextOrSame;
810
import static org.junit.jupiter.api.Assertions.assertAll;
911
import static org.junit.jupiter.api.Assertions.assertEquals;
1012
import static org.junit.jupiter.api.Assertions.assertThrows;
1113
import static org.opensearch.sql.data.model.ExprValueUtils.integerValue;
1214
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;
1315

1416
import java.time.LocalDate;
17+
import java.time.temporal.ChronoUnit;
1518
import java.util.stream.Stream;
1619
import org.junit.jupiter.api.Test;
1720
import org.junit.jupiter.params.ParameterizedTest;
@@ -98,10 +101,7 @@ public void testYearweekWithoutMode() {
98101

99102
@Test
100103
public void testYearweekWithTimeType() {
101-
LocalDate today = LocalDate.now(functionProperties.getQueryStartClock());
102-
int week = DateTimeTestBase.getYearWeek(today);
103-
int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear();
104-
int expected = Integer.parseInt(String.format("%d%02d", year, week));
104+
int expected = getYearWeekBeforeSunday(LocalDate.now(functionProperties.getQueryStartClock()));
105105

106106
FunctionExpression expression =
107107
DSL.yearweek(
@@ -110,9 +110,27 @@ public void testYearweekWithTimeType() {
110110
FunctionExpression expressionWithoutMode =
111111
DSL.yearweek(functionProperties, DSL.literal(new ExprTimeValue("10:11:12")));
112112

113-
assertAll(
114-
() -> assertEquals(expected, eval(expression).integerValue()),
115-
() -> assertEquals(expected, eval(expressionWithoutMode).integerValue()));
113+
assertEquals(
114+
expected,
115+
eval(expression).integerValue(),
116+
String.format(
117+
"Expected year week: %d, got %s (test with mode)", expected, eval(expression)));
118+
assertEquals(
119+
expected,
120+
eval(expressionWithoutMode).integerValue(),
121+
String.format(
122+
"Expected year week: %d, got %s (test without mode)", expected, eval(expression)));
123+
}
124+
125+
private int getYearWeekBeforeSunday(LocalDate date) {
126+
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
127+
if (date.isBefore(firstSundayOfYear)) {
128+
return getYearWeekBeforeSunday(date.minusDays(1));
129+
}
130+
131+
int week = (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
132+
int year = date.getYear();
133+
return Integer.parseInt(String.format("%d%02d", year, week));
116134
}
117135

118136
@Test

docs/user/ppl/functions/datetime.rst

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2169,7 +2169,7 @@ YEARWEEK
21692169
Description
21702170
>>>>>>>>>>>
21712171

2172-
Usage: yearweek(date) returns the year and week for date as an integer. It accepts and optional mode arguments aligned with those available for the `WEEK`_ function.
2172+
Usage: yearweek(date[, mode]) returns the year and week for date as an integer. It accepts and optional mode arguments aligned with those available for the `WEEK`_ function.
21732173

21742174
Argument type: STRING/DATE/TIME/TIMESTAMP
21752175

@@ -2179,10 +2179,17 @@ Example::
21792179

21802180
os> source=people | eval `YEARWEEK('2020-08-26')` = YEARWEEK('2020-08-26') | eval `YEARWEEK('2019-01-05', 1)` = YEARWEEK('2019-01-05', 1) | fields `YEARWEEK('2020-08-26')`, `YEARWEEK('2019-01-05', 1)`
21812181
fetched rows / total rows = 1/1
2182-
+------------------------+---------------------------+
2183-
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) |
2184-
|------------------------+---------------------------|
2185-
| 202034 | 201901 |
2186-
+------------------------+---------------------------+
2182+
+------------------------+---------------------------+---------------------------+
2183+
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) | YEARWEEK('2025-01-04', 1) |
2184+
|------------------------+---------------------------+---------------------------|
2185+
| 202034 | 201901 | 202452 |
2186+
+------------------------+---------------------------+---------------------------+
21872187

2188+
os> source=people | eval `YEARWEEK('2025-01-04')` = YEARWEEK('2025-01-04') | eval `YEARWEEK('2019-01-05', 1)` = YEARWEEK('2019-01-05', 1) | fields `YEARWEEK('2020-08-26')`, `YEARWEEK('2019-01-05', 1)`
2189+
fetched rows / total rows = 1/1
2190+
+------------------------+---------------------------+---------------------------+
2191+
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) | YEARWEEK('2025-01-04', 1) |
2192+
|------------------------+---------------------------+---------------------------|
2193+
| 202034 | 201901 | 202452 |
2194+
+------------------------+---------------------------+---------------------------+
21882195

0 commit comments

Comments
 (0)