Fixed DateTimeFunctionTest.testWeekOfYearWithTimeType and YearWeekTes…

…t.testYearWeekWithTimeType test failures Signed-off-by: Kenrick Yap <[email protected]>
opensearch-project · Jan 9, 2025 · e998e35 · e998e35
1 parent 0e4acd3
commit e998e35
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 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
@@ -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
@@ -5,14 +5,16 @@
 
 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;
 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;
@@ -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));