From 4fef581e4e13a8d3cddeb35de486b905bc0ec8a8 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 16 Jan 2025 15:43:20 -0800 Subject: [PATCH] Address review comments Signed-off-by: currantw --- docs/ppl-lang/functions/ppl-datetime.md | 9 ++++--- .../ppl/utils/BuiltinFunctionTransformer.java | 24 ++++++++++--------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/ppl-lang/functions/ppl-datetime.md b/docs/ppl-lang/functions/ppl-datetime.md index a56946951..ba73c4d0d 100644 --- 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/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/utils/BuiltinFunctionTransformer.java b/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/utils/BuiltinFunctionTransformer.java index 1211b857b..45a92bf22 100644 --- a/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/utils/BuiltinFunctionTransformer.java +++ b/ppl-spark-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 args) { @@ -237,9 +239,9 @@ static Expression[] createIntervalArgs(IntervalUnit unit, Expression value) { return args; } - private static Expression buildRelativeTimestampExpression(List 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())); } }