Address review comments

Signed-off-by: currantw <taylor.curran@improving.com>

docs/ppl-lang/functions/ppl-datetime.md

@@ -738,19 +738,25 @@ Example:
 **Description:**
 
 
-**Usage:** relative_timestamp(str) returns a relative timestamp from given relative string and the current
-timestamp at the time of query execution.
+**Usage:** relative_timestamp(str) returns a relative timestamp corresponding to the given relative string and the
+current timestamp at the time of query execution.
 
-The relative timestamp string has syntax `[+|-]<offset_time_integer><offset_time_unit>@<snap_time_unit>`, and is made up
-of two optional components:
-* An offset from the current timestamp, which is composed of a sign (`+` or `-`), an optional time integer, and a time
-  unit. If the time integer is not specified, it defaults to one. For example, `+2hr` is two hours after the current
-  timestamp, while `-mon` is one month ago. 
-* A snap-to time using the `@` symbol followed by a time unit. The snap-to time is applied after the offset (if 
-  specified), and rounds the time <i>down</i> to the start of the specified time unit (i.e. backwards in time). For 
-  example, `@wk` is the start of the current week (Sunday is considered to be the first day of the week).
+The relative timestamp string has syntax `[+|-]<offset_time_integer><offset_time_unit>@<snap_time_unit>`, and is
+made up of two optional components.
+* An offset from the current timestamp, which is composed of a sign (`+` or `-`), optional `offset_time_integer`, and 
+  `offset_time_unit`. If the offset time integer is not specified, it defaults to `1`. For example, `+2hr` is two
+  hours after the current timestamp, while `-mon` is one month ago. 
+* A snap-to time using the `@` symbol followed by `snap_time_unit`. The snap-to time is applied after the offset (if 
+  specified), and rounds the time <i>down</i> to the start of the specified time unit. For example, `@wk` is the start
+  of the current week (Sunday is considered to be the first day of the week).
 
-The following offset time units are supported:
+The special relative timestamp string `now`, corresponding to the current timestamp, is also supported. The current
+timestamp is determined once at the start of query execution, and is used for all relative timestamp calculations for
+that query.
+
+The relative timestamp string is case-insensitive.
+
+The following values are supported for `offset_time_unit`:
 
 | Time Unit | Supported Keywords                        |
 |-----------|-------------------------------------------|
@@ -762,7 +768,7 @@ The following offset time units are supported:
 | Quarters  | `q`, `qtr`, `qtrs`, `quarter`, `quarters` |
 | Years     | `y`, `yr`, `yrs`, `year`, `years`         |
 
-The snap-to time supports all the time units above, as well as the following day of the week time units:
+All the time units above are supported for `snap_time_unit`, as well as the following day-of-the-week time units:
 
 | Time Unit | Supported Keywords |
 |-----------|--------------------|
@@ -774,16 +780,14 @@ The snap-to time supports all the time units above, as well as the following day
 | Friday    | `w5`               |
 | Saturday  | `w6`               |
 
-The special relative timestamp string `now`, corresponding to the current timestamp, is also supported.
-
 For example, if the current timestamp is Monday, January 03, 2000 at 01:01:01 am:
 
 | Relative String | Description                                                  | Resulting Relative Time                     |
 |-----------------|--------------------------------------------------------------|---------------------------------------------|
 | `-60m`          | Sixty minutes ago                                            | Monday, January 03, 2000 at 00:01:01 am     |
-| `-1h`           | One hour ago                                                 | Monday, January 03, 2000 at 00:01:01 am     |
+| `-1H`           | One hour ago                                                 | Monday, January 03, 2000 at 00:01:01 am     |
 | `+2wk`          | Two weeks from now                                           | Monday, January 17, 2000 at 00:01:01 am     |
-| `-1h@w3`        | One hour ago, rounded to the start of the previous Wednesday | Wednesday, December 29, 1999 at 00:00:00 am |
+| `-1h@W3`        | One hour ago, rounded to the start of the previous Wednesday | Wednesday, December 29, 1999 at 00:00:00 am |
 | `@d`            | Start of the current day                                     | Monday, January 03, 2000 at 00:00:00 am     |
 | `now`           | Now                                                          | Monday, January 03, 2000 at 01:01:01 am     | 
 

integ-test/src/integration/scala/org/opensearch/flint/spark/ppl/FlintSparkPPLBuiltInDateTimeFunctionITSuite.scala

@@ -370,34 +370,46 @@ class FlintSparkPPLBuiltInDateTimeFunctionITSuite
 
   test("test RELATIVE_TIMESTAMP") {
     var frame = sql(s"""
-                       | source = $testTable
+                       | source=$testTable
                        | | eval seconds_diff = timestampdiff(SECOND, now(), relative_timestamp("now"))
                        | | fields seconds_diff
                        | | head 1
                        | """.stripMargin)
     assertSameRows(Seq(Row(0)), frame)
 
     frame = sql(s"""
-                       | source = $testTable
-                       | | eval hours_diff = timestampdiff(HOUR, now(), relative_timestamp("+1h"))
+                       | source=$testTable
+                       | | eval hours_diff = timestampdiff(HOUR, relative_timestamp("+1h"), relative_timestamp("+1d"))
                        | | fields hours_diff
                        | | head 1
                        | """.stripMargin)
-    assertSameRows(Seq(Row(1)), frame)
+    assertSameRows(Seq(Row(23)), frame)
 
     frame = sql(s"""
-                   | source = $testTable
+                   | source =$testTable
                    | | eval day = day_of_week(relative_timestamp("@w0"))
                    | | fields day
                    | | head 1
                    | """.stripMargin)
     assertSameRows(Seq(Row(1)), frame)
+
+    frame = sql(s"""
+                   | source=$testTable
+                   | | eval last_wednesday = relative_timestamp("-1d@w3")
+                   | | eval actual_days_ago = timestampdiff(DAY, last_wednesday, now() )
+                   | | eval day_of_week = day_of_week(now())
+                   | | eval expected_days_ago = case(day_of_week >= 4, day_of_week - 4 else day_of_week + 3)
+                   | | eval test_result = (expected_weeks_ago = actual_weeks_ago)
+                   | | fields test_result
+                   | | head 1
+                   | """.stripMargin)
+    assertSameRows(Seq(Row(1)), frame)
   }
 
   // TODO #957: Support earliest
   ignore("test EARLIEST") {
     var frame = sql(s"""
-                       | source = $testTable
+                       | source=$testTable
                        | | eval earliest_hour_before = earliest(now(), "-1h")
                        | | eval earliest_now = earliest(now(), "now")
                        | | eval earliest_hour_after = earliest(now(), "+1h")
@@ -410,7 +422,7 @@ class FlintSparkPPLBuiltInDateTimeFunctionITSuite
   // TODO #957: Support latest
   ignore("test LATEST") {
     var frame = sql(s"""
-                       | source = $testTable
+                       | source=$testTable
                        | | eval latest_hour_before = latest(now(), "-1h")
                        | | eval latest_now = latest(now(), "now")
                        | | eval latest_hour_after = latest(now(), "+1h")

ppl-spark-integration/src/main/java/org/opensearch/sql/expression/function/TimeUtils.java

@@ -8,7 +8,11 @@
 import com.google.common.collect.ImmutableMap;
 import lombok.experimental.UtilityClass;
 
-import java.time.*;
+import java.time.DayOfWeek;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.Month;
+import java.time.Period;
 import java.time.temporal.ChronoUnit;
 import java.util.HashMap;
 import java.util.Map;
@@ -43,6 +47,7 @@ public class TimeUtils {
 
     // Map from time unit to the corresponding duration.
     private static final Map<String, Duration> DURATION_FOR_TIME_UNIT_MAP;
+
     static {
         Map<String, Duration> durationMap = new HashMap<>();
         SECOND_UNITS_SET.forEach(u -> durationMap.put(u, Duration.ofSeconds(1)));
@@ -53,6 +58,7 @@ public class TimeUtils {
 
     // Map from time unit to the corresponding period.
     private static final Map<String, Period> PERIOD_FOR_TIME_UNIT_MAP;
+
     static {
         Map<String, Period> periodMap = new HashMap<>();
         DAY_UNITS_SET.forEach(u -> periodMap.put(u, Period.ofDays(1)));
@@ -224,7 +230,7 @@ public static LocalDateTime getRelativeLocalDateTime(String relativeString, Loca
         Matcher matcher = RELATIVE_PATTERN.matcher(relativeString);
         if (!matcher.matches()) {
             String message = String.format("The relative date time '%s' is not supported.", relativeString);
-            throw new RuntimeException(message);
+            throw new IllegalArgumentException(message);
         }
 
 
@@ -274,7 +280,7 @@ private LocalDateTime applyOffset(LocalDateTime localDateTime, String offsetSign
         }
 
         String message = String.format("The relative date time unit '%s' is not supported.", offsetUnit);
-        throw new RuntimeException(message);
+        throw new IllegalArgumentException(message);
     }
 
     /**
@@ -304,11 +310,11 @@ private LocalDateTime applySnap(LocalDateTime localDateTime, String snapUnit) {
         } else if (YEAR_UNITS_SET.contains(snapUnitLowerCase)) {
             return localDateTime.truncatedTo(ChronoUnit.DAYS).withDayOfYear(1);
         } else if (DAY_OF_THE_WEEK_FOR_SNAP_UNIT_MAP.containsKey(snapUnitLowerCase)) {
-            return applySnapToDayOfWeek(localDateTime, DAY_OF_THE_WEEK_FOR_SNAP_UNIT_MAP.get(snapUnit));
+            return applySnapToDayOfWeek(localDateTime, DAY_OF_THE_WEEK_FOR_SNAP_UNIT_MAP.get(snapUnitLowerCase));
         }
 
         String message = String.format("The relative date time unit '%s' is not supported.", snapUnit);
-        throw new RuntimeException(message);
+        throw new IllegalArgumentException(message);
     }
 
     /**
@@ -318,12 +324,9 @@ private LocalDateTime applySnap(LocalDateTime localDateTime, String snapUnit) {
     private LocalDateTime applySnapToDayOfWeek(LocalDateTime dateTime, DayOfWeek snapDayOfWeek) {
         LocalDateTime snappedDateTime = dateTime.truncatedTo(ChronoUnit.DAYS);
 
-        DayOfWeek dayOfWeek = dateTime.getDayOfWeek();
-        if (dayOfWeek.equals(snapDayOfWeek)) {
-            return snappedDateTime;
-        }
+        int daysToSnap = dateTime.getDayOfWeek().getValue() - snapDayOfWeek.getValue();
+        if (daysToSnap < 0) daysToSnap += DayOfWeek.values().length;
 
-        int daysToSnap = DayOfWeek.values().length - snapDayOfWeek.getValue() + dayOfWeek.getValue();
         return snappedDateTime.minusDays(daysToSnap);
     }
 }

ppl-spark-integration/src/test/java/org/opensearch/sql/expression/function/SerializableTimeUdfTest.java

@@ -29,9 +29,9 @@ public void relativeTimestampTest() {
          */
 
         testValid("-60m", "2000-01-03 00:01:01.100");
-        testValid("-h", "2000-01-03 00:01:01.100");
+        testValid("-H", "2000-01-03 00:01:01.100");
         testValid("+2wk", "2000-01-17 01:01:01.100");
-        testValid("-1h@w3", "1999-12-29 00:00:00");
+        testValid("-1h@W3", "1999-12-29 00:00:00");
         testValid("@d", "2000-01-03 00:00:00");
         testValid("now", "2000-01-03 01:01:01.100");
 

ppl-spark-integration/src/test/java/org/opensearch/sql/expression/function/TimeUtilsTest.java

@@ -20,9 +20,9 @@ public class TimeUtilsTest {
     @Test
     public void testRelative() {
         testValid("-60m", "2000-01-03T00:01:01.100");
-        testValid("-h", "2000-01-03T00:01:01.100");
+        testValid("-H", "2000-01-03T00:01:01.100");
         testValid("+2wk", "2000-01-17T01:01:01.100");
-        testValid("-1h@w3", "1999-12-29T00:00:00");
+        testValid("-1h@W3", "1999-12-29T00:00:00");
         testValid("@d", "2000-01-03T00:00");
         testValid("now", "2000-01-03T01:01:01.100");