Updated tests. Fixed exception to be less general.

Signed-off-by: MitchellGale-BitQuill <mitchellg@bitquilltech.com>

core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java

@@ -28,6 +28,7 @@
 import java.math.RoundingMode;
 import java.text.DecimalFormat;
 import java.time.Instant;
+import java.time.DateTimeException;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
@@ -38,6 +39,7 @@
 import java.time.format.DateTimeParseException;
 import java.time.format.TextStyle;
 import java.util.Locale;
+import java.util.Set;
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
@@ -613,19 +615,23 @@ private ExprValue exprAddDateDays(ExprValue date, ExprValue days) {
    * @return DateTime that has been converted to the to_tz timezone.
    */
   private ExprValue exprConvertTZ(ExprValue startingDateTime, ExprValue fromTz, ExprValue toTz) {
-    ZoneId convertedFromTz = ZoneId.of(fromTz.stringValue());
-    ZoneId convertedToTz = ZoneId.of(toTz.stringValue());
+    try {
+      ZoneId convertedFromTz = ZoneId.of(fromTz.stringValue());
+      ZoneId convertedToTz = ZoneId.of(toTz.stringValue());
 
-    if (!isTimeZoneValid(convertedFromTz)
+      if (!isTimeZoneValid(convertedFromTz)
           || !isTimeZoneValid(convertedToTz)) {
-      return ExprNullValue.of();
-    }
+        return ExprNullValue.of();
+      }
 
-    ZonedDateTime zonedDateTime =
-        startingDateTime.datetimeValue().atZone(convertedFromTz);
+      ZonedDateTime zonedDateTime =
+          startingDateTime.datetimeValue().atZone(convertedFromTz);
 
-    return new ExprDatetimeValue(
-        zonedDateTime.withZoneSameInstant(convertedToTz).toLocalDateTime());
+      return new ExprDatetimeValue(
+          zonedDateTime.withZoneSameInstant(convertedToTz).toLocalDateTime());
+    } catch (DateTimeException e) {
+      return ExprNullValue.of();
+    }
   }
 
   /**
@@ -668,7 +674,7 @@ private ExprValue exprDateTime(ExprValue dateTime, ExprValue timeZone) {
           new ExprDatetimeValue(zdtWithZoneOffset.toLocalDateTime()),
           new ExprStringValue(String.valueOf(fromTZ)),
           new ExprStringValue(String.valueOf(ZoneId.of(timeZone.stringValue()))));
-    } catch (Exception e) {
+    } catch (DateTimeParseException e) {
       LocalDateTime ldtFormatted = LocalDateTime.parse(dateTime.stringValue(), formatDT);
 
       convertTZResult = exprConvertTZ(
@@ -1076,14 +1082,10 @@ private Boolean isTimeZoneValid(ZoneId zone) {
     ZonedDateTime passedTzValidator =
         defaultDateTime.atZone(defaultTz).withZoneSameInstant(zone).withZoneSameLocal(defaultTz);
 
-    if ((passedTzValidator.isBefore(maxTzValidator)
+    return (passedTzValidator.isBefore(maxTzValidator)
         || passedTzValidator.isEqual(maxTzValidator))
         && (passedTzValidator.isAfter(minTzValidator)
-        || passedTzValidator.isEqual(minTzValidator))) {
-      return true;
-    }
-
-    return false;
+        || passedTzValidator.isEqual(minTzValidator));
   }
 
   /**

core/src/test/java/org/opensearch/sql/expression/datetime/ConvertTZTest.java

@@ -0,0 +1,130 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+
+package org.opensearch.sql.expression.datetime;
+
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.when;
+import static org.opensearch.sql.data.model.ExprValueUtils.missingValue;
+import static org.opensearch.sql.data.model.ExprValueUtils.nullValue;
+import static org.opensearch.sql.data.type.ExprCoreType.DATETIME;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.opensearch.sql.data.model.ExprDatetimeValue;
+import org.opensearch.sql.data.model.ExprValue;
+import org.opensearch.sql.expression.DSL;
+import org.opensearch.sql.expression.Expression;
+import org.opensearch.sql.expression.ExpressionTestBase;
+import org.opensearch.sql.expression.FunctionExpression;
+import org.opensearch.sql.expression.env.Environment;
+
+
+
+@ExtendWith(MockitoExtension.class)
+class ConvertTZTest extends ExpressionTestBase {
+
+  @Mock
+  Environment<Expression, ExprValue> env;
+
+  @Mock
+  Expression nullRef;
+
+  @Mock
+  Expression missingRef;
+
+  @BeforeEach
+  public void setup() {
+    when(nullRef.valueOf(env)).thenReturn(nullValue());
+    when(missingRef.valueOf(env)).thenReturn(missingValue());
+  }
+
+  @Test
+  public void convert_tz() {
+    when(nullRef.type()).thenReturn(DATETIME);
+    when(missingRef.type()).thenReturn(DATETIME);
+    assertEquals(nullValue(), eval(dsl.date(nullRef)));
+    assertEquals(missingValue(), eval(dsl.date(missingRef)));
+
+    FunctionExpression expr = dsl.convert_tz(dsl.datetime(
+        DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:00"),
+        DSL.literal("+10:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(new ExprDatetimeValue("2008-05-16 08:00:00"), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+        DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:00"),
+        DSL.literal("+16:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:00"),
+        DSL.literal("-16:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+15:00"),
+        DSL.literal("+01:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("-15:00"),
+        DSL.literal("+01:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("-12:00"),
+        DSL.literal("+15:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:00"),
+        DSL.literal("+14:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(new ExprDatetimeValue("2008-05-16 12:00:00"), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:01"),
+        DSL.literal("-13:59"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(new ExprDatetimeValue("2008-05-15 08:00:00"), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+)()"),
+        DSL.literal("+12:00"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+
+    expr = dsl.convert_tz(dsl.datetime(
+            DSL.literal("2008-05-15 22:00:00")),
+        DSL.literal("+00:00"),
+        DSL.literal("fesasgrsgr"));
+    assertEquals(DATETIME, expr.type());
+    assertEquals(nullValue(), expr.valueOf(env));
+  }
+
+  private ExprValue eval(Expression expression) {
+    return expression.valueOf(env);
+  }
+}

core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java

@@ -34,7 +34,6 @@
 import org.opensearch.sql.data.model.ExprDateValue;
 import org.opensearch.sql.data.model.ExprDatetimeValue;
 import org.opensearch.sql.data.model.ExprLongValue;
-import org.opensearch.sql.data.model.ExprNullValue;
 import org.opensearch.sql.data.model.ExprTimeValue;
 import org.opensearch.sql.data.model.ExprTimestampValue;
 import org.opensearch.sql.data.model.ExprValue;
@@ -225,63 +224,6 @@ public void adddate() {
     assertEquals(missingValue(), eval(dsl.adddate(nullRef, missingRef)));
   }
 
-  @Test
-  public void convert_tz() {
-    when(nullRef.type()).thenReturn(DATETIME);
-    when(missingRef.type()).thenReturn(DATETIME);
-    assertEquals(nullValue(), eval(dsl.date(nullRef)));
-    assertEquals(missingValue(), eval(dsl.date(missingRef)));
-
-    FunctionExpression expr = dsl.convert_tz(dsl.datetime(
-        DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("+00:00"),
-        DSL.literal("+10:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-16 08:00:00"), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-        DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("+00:00"),
-        DSL.literal("+16:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-            DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("+00:00"),
-        DSL.literal("-16:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-            DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("+15:00"),
-        DSL.literal("+01:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-            DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("-15:00"),
-        DSL.literal("+01:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-            DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("-12:00"),
-        DSL.literal("+15:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.convert_tz(dsl.datetime(
-            DSL.literal("2008-05-15 22:00:00")),
-        DSL.literal("+00:00"),
-        DSL.literal("+14:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-16 12:00:00"), expr.valueOf(env));
-  }
-
   @Test
   public void date() {
     when(nullRef.type()).thenReturn(DATE);
@@ -329,12 +271,6 @@ public void date_add() {
     assertEquals(new ExprDatetimeValue("2020-08-26 01:00:00"), expr.valueOf(env));
     assertEquals("date_add(\"2020-08-26\", interval(1, \"hour\"))", expr.toString());
 
-    expr = dsl
-        .date_add(DSL.literal("2020-08-26"), dsl.interval(DSL.literal(0), DSL.literal("hour")));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2020-08-26 00:00:00"), expr.valueOf(env));
-    assertEquals("date_add(\"2020-08-26\", interval(0, \"hour\"))", expr.toString());
-
     when(nullRef.type()).thenReturn(DATE);
     assertEquals(nullValue(), eval(dsl.date_add(nullRef, DSL.literal(1L))));
     assertEquals(nullValue(),
@@ -425,52 +361,6 @@ public void date_sub() {
     assertEquals(missingValue(), eval(dsl.date_sub(nullRef, missingRef)));
   }
 
-  @Test
-  public void date_time() {
-    when(nullRef.type()).thenReturn(DATETIME);
-    when(missingRef.type()).thenReturn(DATETIME);
-    assertEquals(nullValue(), eval(dsl.date(nullRef)));
-    assertEquals(missingValue(), eval(dsl.date(missingRef)));
-
-    FunctionExpression expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-15 22:00:00"), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00+01:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-15 22:00:00"), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00+01:00"),
-        DSL.literal("America/Los_Angeles"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-15 14:00:00"), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00"),
-        DSL.literal("America/Los_Angeles"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-15 15:00:00"), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00-11:00"),
-        DSL.literal("America/Los_Angeles"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(new ExprDatetimeValue("2008-05-16 02:00:00"), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00-12:00"),
-        DSL.literal("+15:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00-13:00"),
-        DSL.literal("+10:00"));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-
-    expr = dsl.datetime(DSL.literal("2008-05-15 22:00:00-11:00"),
-        DSL.literal(nullValue()));
-    assertEquals(DATETIME, expr.type());
-    assertEquals(nullValue(), expr.valueOf(env));
-  }
-
   @Test
   public void day() {
     when(nullRef.type()).thenReturn(DATE);
@@ -1113,4 +1003,4 @@ void testDateFormat(DateFormatTester dft) {
   private ExprValue eval(Expression expression) {
     return expression.valueOf(env);
   }
-}
+}