Skip to content

Commit

Permalink
SQL: Update unit tests with proper number of weeks
Browse files Browse the repository at this point in the history
Signed-off-by: Andrew Carbonetto <[email protected]>
  • Loading branch information
acarbonetto committed Jan 14, 2025
1 parent 67b81f1 commit cf695df
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package org.opensearch.sql.expression.datetime;

import static java.time.DayOfWeek.SUNDAY;
import static java.time.temporal.TemporalAdjusters.nextOrSame;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand All @@ -23,6 +25,7 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.stream.Stream;
import lombok.AllArgsConstructor;
Expand Down Expand Up @@ -1230,7 +1233,7 @@ public void testWeekFormats(
@Test
public void testWeekOfYearWithTimeType() {
LocalDate today = LocalDate.now(functionProperties.getQueryStartClock());
int week = DateTimeTestBase.getYearWeek(today);
int week = getWeekOfYearBeforeSunday(today);

assertAll(
() ->
Expand All @@ -1251,6 +1254,15 @@ public void testWeekOfYearWithTimeType() {
week));
}

private int getWeekOfYearBeforeSunday(LocalDate date) {
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
if (date.isBefore(firstSundayOfYear)) {
return 0;
}

return (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
}

@Test
public void modeInUnsupportedFormat() {
FunctionExpression expression1 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@

package org.opensearch.sql.expression.datetime;

import static java.time.DayOfWeek.SUNDAY;
import static java.time.temporal.TemporalAdjusters.nextOrSame;
import static org.opensearch.sql.data.model.ExprValueUtils.fromObjectValue;

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneOffset;
import java.time.temporal.ChronoUnit;
import java.time.temporal.Temporal;
import java.util.List;
import org.opensearch.sql.data.model.ExprDateValue;
Expand Down Expand Up @@ -234,19 +231,4 @@ protected Double unixTimeStampOf(LocalDateTime value) {
protected Double unixTimeStampOf(Instant value) {
return unixTimeStampOf(DSL.literal(new ExprTimestampValue(value))).valueOf().doubleValue();
}

// The following calculation is needed to correct the discrepancy in how ISO 860 and our
// implementation of YEARWEEK calculates. ISO 8601 calculates weeks using the following criteria:
// - Weeks start on Monday
// - The first week of a year is any week containing 4 or more days in the new year.
// Whereas YEARWEEK counts only full weeks, where weeks start on Sunday. To fix the discrepancy
// we find the first Sunday of the year and start counting weeks from that date.
protected static int getYearWeek(LocalDate date) {
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
int week =
date.isBefore(firstSundayOfYear)
? 52
: (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
return week;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@

package org.opensearch.sql.expression.datetime;

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

import java.time.LocalDate;
import java.time.temporal.WeekFields;
import java.util.Locale;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -93,15 +92,20 @@ private void datePartWithTimeArgQuery(String part, String time, long expected) {

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

datePartWithTimeArgQuery("DAY", timeInput, now.getDayOfMonth());

datePartWithTimeArgQuery(
"WEEK", timeInput, now.get(WeekFields.of(Locale.ENGLISH).weekOfYear()));

// To avoid flaky test, skip the testing in December and January because the WEEK is ISO 8601
// week-of-week-based-year which is considered to start on a Monday and week 1 is the first week
// with >3 days. it is possible for early-January dates to be part of the 52nd or 53rd week of
// the previous year, and for late-December dates to be part of the first week of the next year.
// For example, 2005-01-02 is part of the 53rd week of year 2004, while 2012-12-31 is part of
// the first week of 2013
if (now.getMonthValue() != 1 && now.getMonthValue() != 12) {
datePartWithTimeArgQuery("WEEK", datetimeInput, now.get(ALIGNED_WEEK_OF_YEAR));
}
datePartWithTimeArgQuery("MONTH", timeInput, now.getMonthValue());

datePartWithTimeArgQuery("YEAR", timeInput, now.getYear());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@

package org.opensearch.sql.expression.datetime;

import static java.time.DayOfWeek.SUNDAY;
import static java.time.temporal.TemporalAdjusters.nextOrSame;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.opensearch.sql.data.model.ExprValueUtils.integerValue;
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;

import java.time.LocalDate;
import java.time.temporal.ChronoUnit;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -98,10 +101,7 @@ public void testYearweekWithoutMode() {

@Test
public void testYearweekWithTimeType() {
LocalDate today = LocalDate.now(functionProperties.getQueryStartClock());
int week = DateTimeTestBase.getYearWeek(today);
int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear();
int expected = Integer.parseInt(String.format("%d%02d", year, week));
int expected = getYearWeekBeforeSunday(LocalDate.now(functionProperties.getQueryStartClock()));

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

assertAll(
() -> assertEquals(expected, eval(expression).integerValue()),
() -> assertEquals(expected, eval(expressionWithoutMode).integerValue()));
assertEquals(
expected,
eval(expression).integerValue(),
String.format(
"Expected year week: %d, got %s (test with mode)", expected, eval(expression)));
assertEquals(
expected,
eval(expressionWithoutMode).integerValue(),
String.format(
"Expected year week: %d, got %s (test without mode)", expected, eval(expression)));
}

private int getYearWeekBeforeSunday(LocalDate date) {
LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
if (date.isBefore(firstSundayOfYear)) {
return getYearWeekBeforeSunday(date.minusDays(1));
}

int week = (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
int year = date.getYear();
return Integer.parseInt(String.format("%d%02d", year, week));
}

@Test
Expand Down
19 changes: 13 additions & 6 deletions docs/user/ppl/functions/datetime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2169,7 +2169,7 @@ YEARWEEK
Description
>>>>>>>>>>>

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.
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.

Argument type: STRING/DATE/TIME/TIMESTAMP

Expand All @@ -2179,10 +2179,17 @@ Example::

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)`
fetched rows / total rows = 1/1
+------------------------+---------------------------+
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) |
|------------------------+---------------------------|
| 202034 | 201901 |
+------------------------+---------------------------+
+------------------------+---------------------------+---------------------------+
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) | YEARWEEK('2025-01-04', 1) |
|------------------------+---------------------------+---------------------------|
| 202034 | 201901 | 202452 |
+------------------------+---------------------------+---------------------------+

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)`
fetched rows / total rows = 1/1
+------------------------+---------------------------+---------------------------+
| YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) | YEARWEEK('2025-01-04', 1) |
|------------------------+---------------------------+---------------------------|
| 202034 | 201901 | 202452 |
+------------------------+---------------------------+---------------------------+

0 comments on commit cf695df

Please sign in to comment.