From e998e35e7fa38c5e8275f7c0b8165402cedaba7a Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 01:29:50 -0800 Subject: [PATCH 1/7] Fixed DateTimeFunctionTest.testWeekOfYearWithTimeType and YearWeekTest.testYearWeekWithTimeType test failures Signed-off-by: Kenrick Yap --- .../datetime/DateTimeFunctionTest.java | 27 ++++++++++++++----- .../sql/expression/datetime/YearweekTest.java | 21 +++++++++++++-- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index c820c97196..add5abdd03 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -5,7 +5,8 @@ package org.opensearch.sql.expression.datetime; -import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR; +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; @@ -24,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; @@ -1232,26 +1234,37 @@ public void testWeekFormats( // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test public void testWeekOfYearWithTimeType() { + // The following calculation is needed to correct the discrepancy in how ISO 8601 + // and our implementation 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 we only count 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. + LocalDate today = LocalDate.now(functionProperties.getQueryStartClock()); + LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY)); + int week = + today.isBefore(firstSundayOfYear) + ? 52 + : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1; + assertAll( () -> validateStringFormat( DSL.week( functionProperties, DSL.literal(new ExprTimeValue("12:23:34")), DSL.literal(0)), "week(TIME '12:23:34', 0)", - LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - - 1), + week), () -> validateStringFormat( DSL.week_of_year(functionProperties, DSL.literal(new ExprTimeValue("12:23:34"))), "week_of_year(TIME '12:23:34')", - LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - - 1), + week), () -> validateStringFormat( DSL.weekofyear(functionProperties, DSL.literal(new ExprTimeValue("12:23:34"))), "weekofyear(TIME '12:23:34')", - LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - - 1)); + week)); } @Test diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index 47225ac601..63230def26 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -5,7 +5,8 @@ package org.opensearch.sql.expression.datetime; -import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR; +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; @@ -13,6 +14,7 @@ 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; @@ -101,7 +103,22 @@ public void testYearweekWithoutMode() { // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test public void testYearweekWithTimeType() { - int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - 1; + LocalDate today = LocalDate.now(functionProperties.getQueryStartClock()); + + // The following calculation is needed to correct the discrepancy in how ISO 8601 + // 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. + LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY)); + int week = + today.isBefore(firstSundayOfYear) + ? 52 + : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1; + int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); int expected = Integer.parseInt(String.format("%d%02d", year, week)); From b127352357f8297a005a6ab4fbbb1003ad727b7f Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 10:53:23 -0800 Subject: [PATCH 2/7] addressed PR comments Signed-off-by: Kenrick Yap --- .../datetime/DateTimeFunctionTest.java | 18 +--------------- .../expression/datetime/DateTimeTestBase.java | 18 ++++++++++++++++ .../sql/expression/datetime/YearweekTest.java | 21 +------------------ 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index add5abdd03..d4866d291e 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -5,8 +5,6 @@ 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; @@ -25,7 +23,6 @@ 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; @@ -1230,23 +1227,10 @@ public void testWeekFormats( expectedInteger); } - // subtracting 1 as a temporary fix for year 2024. - // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test public void testWeekOfYearWithTimeType() { - // The following calculation is needed to correct the discrepancy in how ISO 8601 - // and our implementation 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 we only count 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. LocalDate today = LocalDate.now(functionProperties.getQueryStartClock()); - LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY)); - int week = - today.isBefore(firstSundayOfYear) - ? 52 - : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1; + int week = DateTimeTestBase.getYearWeek(today); assertAll( () -> diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java index 865c162f76..62b53ec9ec 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java @@ -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.opensearch.sql.data.model.ExprValueUtils.fromObjectValue; import java.time.Instant; @@ -12,6 +14,7 @@ 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; @@ -231,4 +234,19 @@ 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; + } } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index 63230def26..ec075cd0f3 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -5,8 +5,6 @@ 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; @@ -14,7 +12,6 @@ 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; @@ -99,26 +96,10 @@ public void testYearweekWithoutMode() { assertEquals(eval(expression), eval(expressionWithoutMode)); } - // subtracting 1 as a temporary fix for year 2024. - // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test public void testYearweekWithTimeType() { LocalDate today = LocalDate.now(functionProperties.getQueryStartClock()); - - // The following calculation is needed to correct the discrepancy in how ISO 8601 - // 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. - LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY)); - int week = - today.isBefore(firstSundayOfYear) - ? 52 - : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1; - + int week = DateTimeTestBase.getYearWeek(today); int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); int expected = Integer.parseInt(String.format("%d%02d", year, week)); From 67b81f1e8f4cd684ecaa82a71ee9c0c9be53c8aa Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 14 Jan 2025 08:15:18 -0800 Subject: [PATCH 3/7] fixed testExtractDatePartWithTimeType Signed-off-by: Kenrick Yap --- .../sql/expression/datetime/ExtractTest.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index d7635de610..deb18421f3 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -5,11 +5,12 @@ 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; @@ -96,15 +97,8 @@ public void testExtractDatePartWithTimeType() { datePartWithTimeArgQuery("DAY", timeInput, now.getDayOfMonth()); - // 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( + "WEEK", timeInput, now.get(WeekFields.of(Locale.ENGLISH).weekOfYear())); datePartWithTimeArgQuery("MONTH", timeInput, now.getMonthValue()); From cf695df7de94d48072a402787df8c540df441dd0 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 14 Jan 2025 13:02:19 -0800 Subject: [PATCH 4/7] SQL: Update unit tests with proper number of weeks Signed-off-by: Andrew Carbonetto --- .../datetime/DateTimeFunctionTest.java | 14 +++++++- .../expression/datetime/DateTimeTestBase.java | 18 ----------- .../sql/expression/datetime/ExtractTest.java | 18 +++++++---- .../sql/expression/datetime/YearweekTest.java | 32 +++++++++++++++---- docs/user/ppl/functions/datetime.rst | 19 +++++++---- 5 files changed, 62 insertions(+), 39 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index d4866d291e..e39cc08c5f 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -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; @@ -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; @@ -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( () -> @@ -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 = diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java index 62b53ec9ec..865c162f76 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeTestBase.java @@ -5,8 +5,6 @@ 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; @@ -14,7 +12,6 @@ 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; @@ -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; - } } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index deb18421f3..9c481b4d78 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -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; @@ -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()); } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index ec075cd0f3..d944f7c85c 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -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; @@ -12,6 +14,7 @@ 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; @@ -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( @@ -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 diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst index c0d42297ac..c33ecd4680 100644 --- a/docs/user/ppl/functions/datetime.rst +++ b/docs/user/ppl/functions/datetime.rst @@ -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 @@ -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 | + +------------------------+---------------------------+---------------------------+ From 1565c9bc6a054b55560ed8dd26e6fdf5560db621 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 14 Jan 2025 13:23:37 -0800 Subject: [PATCH 5/7] SQL: update datetime test for self-review Signed-off-by: Andrew Carbonetto --- .../datetime/DateTimeFunctionTest.java | 17 ++++++------ .../sql/expression/datetime/ExtractTest.java | 4 ++- docs/user/ppl/functions/datetime.rst | 26 +++++++++---------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index e39cc08c5f..a2c2520d1a 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -1233,7 +1233,13 @@ public void testWeekFormats( @Test public void testWeekOfYearWithTimeType() { LocalDate today = LocalDate.now(functionProperties.getQueryStartClock()); - int week = getWeekOfYearBeforeSunday(today); + + // week is based on the first sunday of the year + LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY)); + int week = + today.isBefore(firstSundayOfYear) + ? 0 + : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1; assertAll( () -> @@ -1254,14 +1260,7 @@ 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; - } + private int getWeekOfYearBeforeSunday(LocalDate date) {} @Test public void modeInUnsupportedFormat() { diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index 9c481b4d78..d7635de610 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -92,10 +92,10 @@ 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()); + // 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 @@ -105,7 +105,9 @@ public void testExtractDatePartWithTimeType() { 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()); } diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst index c33ecd4680..e2a2ef7bf5 100644 --- a/docs/user/ppl/functions/datetime.rst +++ b/docs/user/ppl/functions/datetime.rst @@ -2179,17 +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) | 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 | - +------------------------+---------------------------+---------------------------+ + +------------------------+---------------------------+ + | YEARWEEK('2020-08-26') | YEARWEEK('2019-01-05', 1) | + |------------------------+---------------------------| + | 202034 | 201901 | + +------------------------+---------------------------+ + + os> source=people | eval `YEARWEEK('2025-01-04')` = YEARWEEK('2025-01-04') | fields `YEARWEEK('2025-01-04', 1)` + fetched rows / total rows = 1/1 + +---------------------------+ + | YEARWEEK('2025-01-04', 1) | + |---------------------------| + | 202452 | + +---------------------------+ From f09fa395d54e521c97f26661701996014431eae7 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 14 Jan 2025 14:13:46 -0800 Subject: [PATCH 6/7] SQL: update datetime test for self-review Signed-off-by: Andrew Carbonetto --- .../sql/expression/datetime/DateTimeFunctionTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index a2c2520d1a..ad15dadfb7 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -1260,8 +1260,6 @@ public void testWeekOfYearWithTimeType() { week)); } - private int getWeekOfYearBeforeSunday(LocalDate date) {} - @Test public void modeInUnsupportedFormat() { FunctionExpression expression1 = From e31ae65086eb73364e54b07a1ff0735c4d50d61f Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 14 Jan 2025 15:56:59 -0800 Subject: [PATCH 7/7] SQL: remove doctest Signed-off-by: Andrew Carbonetto --- docs/user/ppl/functions/datetime.rst | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst index e2a2ef7bf5..9af02f3bde 100644 --- a/docs/user/ppl/functions/datetime.rst +++ b/docs/user/ppl/functions/datetime.rst @@ -2185,11 +2185,3 @@ Example:: | 202034 | 201901 | +------------------------+---------------------------+ - os> source=people | eval `YEARWEEK('2025-01-04')` = YEARWEEK('2025-01-04') | fields `YEARWEEK('2025-01-04', 1)` - fetched rows / total rows = 1/1 - +---------------------------+ - | YEARWEEK('2025-01-04', 1) | - |---------------------------| - | 202452 | - +---------------------------+ -