Address review comments

Signed-off-by: currantw <taylor.curran@improving.com>
opensearch-project · Jan 16, 2025 · 4fef581 · 4fef581
1 parent 282f3f6
commit 4fef581
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 14 deletions.
diff --git a/docs/ppl-lang/functions/ppl-datetime.md b/docs/ppl-lang/functions/ppl-datetime.md
@@ -831,9 +831,12 @@ made up of two optional components.
 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 Spark session time zone (`spark.sql.session.timeZone`) is used for determining
-relative timestamps, and accounts for changes in the time zone offset (e.g. daylight savings time); as a result, adding
-one day (`+1d`) is not the same as adding twenty-four hours (`+24h`).
+calculations for that query. The Spark session time zone (`spark.sql.session.timeZone`) is used for determining relative
+timestamps. Offsets using time units (seconds, minutes, or hours) represent a fixed time period; adding twenty-four
+hours (`+24h`) will yield a timestamp that is exactly twenty-four hours later, but which may not have the same local
+time (because of daylight savings, for example). Conversely, offsets using date units (days, weeks, months, quarters, or
+years) do not represent a fixed time period; adding one day (`+1d`) will yield a timestamp with the same local time,
+but which may not be exactly twenty-four hours later.
 
 The relative timestamp string is case-insensitive.
 

diff --git a/...rk-integration/src/main/java/org/opensearch/sql/ppl/utils/BuiltinFunctionTransformer.java b/...rk-integration/src/main/java/org/opensearch/sql/ppl/utils/BuiltinFunctionTransformer.java
@@ -182,19 +182,21 @@ public interface BuiltinFunctionTransformer {
         // Relative time functions
         .put(
                 RELATIVE_TIMESTAMP,
-                BuiltinFunctionTransformer::buildRelativeTimestampExpression)
+                args -> buildRelativeTimestamp(args.get(0)))
         .put(
                 EARLIEST,
-                args ->
-                        LessThanOrEqual$.MODULE$.apply(
-                                buildRelativeTimestampExpression(List.of(args.get(0))),
-                                args.get(1)))
+                args -> {
+                    Expression relativeTimestamp = buildRelativeTimestamp(args.get(0));
+                    Expression timestamp = args.get(1);
+                    return LessThanOrEqual$.MODULE$.apply(relativeTimestamp, timestamp);
+                })
         .put(
                 LATEST,
-                args ->
-                        GreaterThanOrEqual$.MODULE$.apply(
-                                buildRelativeTimestampExpression(List.of(args.get(0))),
-                                args.get(1)))
+                args -> {
+                    Expression relativeTimestamp = buildRelativeTimestamp(args.get(0));
+                    Expression timestamp = args.get(1);
+                    return GreaterThanOrEqual$.MODULE$.apply(relativeTimestamp, timestamp);
+                })
         .build();
 
     static Expression builtinFunction(org.opensearch.sql.ast.expression.Function function, List<Expression> args) {
@@ -237,9 +239,9 @@ static Expression[] createIntervalArgs(IntervalUnit unit, Expression value) {
         return args;
     }
 
-    private static Expression buildRelativeTimestampExpression(List<Expression> args) {
+    private static Expression buildRelativeTimestamp(Expression relativeStringExpression) {
         return SerializableUdf.visit(
                 RELATIVE_TIMESTAMP.getName().getFunctionName(),
-                List.of(args.get(0), CurrentTimestamp$.MODULE$.apply(), CurrentTimeZone$.MODULE$.apply()));
+                List.of(relativeStringExpression, CurrentTimestamp$.MODULE$.apply(), CurrentTimeZone$.MODULE$.apply()));
     }
 }