[GLUTEN-4475][VL] Allow offloading Spark hour function (#4495)

apache · Jan 29, 2024 · f040fe8 · f040fe8
1 parent c6e0f1d
commit f040fe8
Show file tree

Hide file tree

Showing 11 changed files with 215 additions and 14 deletions.
diff --git a/backends-velox/src/test/scala/io/glutenproject/execution/TestOperator.scala b/backends-velox/src/test/scala/io/glutenproject/execution/TestOperator.scala
@@ -963,4 +963,12 @@ class TestOperator extends VeloxWholeStageTransformerSuite with AdaptiveSparkPla
       runQueryAndCompare("SELECT c1 % c2 FROM remainder")(df => checkFallbackOperators(df, 0))
     }
   }
+
+  test("Support HOUR function") {
+    withTable("t1") {
+      sql("create table t1 (c1 int, c2 timestamp) USING PARQUET")
+      sql("INSERT INTO t1 VALUES(1, NOW())")
+      runQueryAndCompare("SELECT c1, HOUR(c2) FROM t1 LIMIT 1")(df => checkFallbackOperators(df, 0))
+    }
+  }
 }
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
@@ -147,13 +147,6 @@ bool SubstraitToVeloxPlanValidator::validateExtractExpr(const std::vector<core::
       return false;
     }
 
-    // The first parameter specifies extracting from which field.
-    const auto& from = variant.value<std::string>();
-    // Hour causes incorrect result.
-    if (from == "HOUR") {
-      LOG_VALIDATION_MSG("Extract from hour.");
-      return false;
-    }
     return true;
   }
   LOG_VALIDATION_MSG("Constant is expected to be the first parameter in extract.");

diff --git a/...-ut/spark32/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala b/...-ut/spark32/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala
@@ -733,6 +733,7 @@ class ClickHouseTestSettings extends BackendTestSettings {
     .exclude("Gluten - TIMESTAMP_MICROS")
     .exclude("Gluten - unix_timestamp")
     .exclude("Gluten - to_unix_timestamp")
+    .exclude(GLUTEN_TEST + "Hour")
   enableSuite[GlutenDecimalExpressionSuite]
   enableSuite[GlutenHashExpressionsSuite]
     .exclude("sha2")

diff --git a/gluten-ut/spark32/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala b/gluten-ut/spark32/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala
@@ -206,6 +206,8 @@ class VeloxTestSettings extends BackendTestSettings {
     .exclude("unix_timestamp")
     // Replaced by a gluten test to pass timezone through config.
     .exclude("to_unix_timestamp")
+    // Replaced by a gluten test to pass timezone through config.
+    .exclude("Hour")
     // Unsupported format: yyyy-MM-dd HH:mm:ss.SSS
     .exclude("SPARK-33498: GetTimestamp,UnixTimestamp,ToUnixTimestamp with parseError")
     // Replaced by a gluten test to pass timezone through config.

diff --git a/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala b/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala
@@ -24,13 +24,13 @@ import org.apache.spark.sql.catalyst.util.DateTimeTestUtils._
 import org.apache.spark.sql.catalyst.util.DateTimeUtils
 import org.apache.spark.sql.catalyst.util.DateTimeUtils.{getZoneId, TimeZoneUTC}
 import org.apache.spark.sql.internal.SQLConf
-import org.apache.spark.sql.types.{DateType, IntegerType, LongType, StringType, TimestampType}
+import org.apache.spark.sql.types.{DateType, IntegerType, LongType, StringType, TimestampNTZType, TimestampType}
 import org.apache.spark.unsafe.types.UTF8String
 
 import java.sql.{Date, Timestamp}
 import java.text.SimpleDateFormat
-import java.time.ZoneId
-import java.util.{Locale, TimeZone}
+import java.time.{LocalDateTime, ZoneId}
+import java.util.{Calendar, Locale, TimeZone}
 
 import scala.concurrent.duration.MICROSECONDS
 
@@ -411,4 +411,68 @@ class GlutenDateExpressionsSuite extends DateExpressionsSuite with GlutenTestsTr
     // Test escaping of format
     GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\""), UTC_OPT) :: Nil)
   }
+
+  test(GLUTEN_TEST + "Hour") {
+    val outstandingTimezonesIds: Seq[String] = Seq(
+      // Velox doesn't support timezones like "UTC".
+      // "UTC",
+      // Due to known issue: "-08:00/+01:00 not found in timezone database",
+      // skip check PST, CET timezone here.
+      // https://github.com/facebookincubator/velox/issues/7804
+      // PST.getId, CET.getId,
+      "Africa/Dakar",
+      LA.getId,
+      "Asia/Urumqi",
+      "Asia/Hong_Kong",
+      "Europe/Brussels"
+    )
+    withDefaultTimeZone(UTC) {
+      Seq("legacy", "corrected").foreach {
+        legacyParserPolicy =>
+          withSQLConf(
+            SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+            SQLConf.SESSION_LOCAL_TIMEZONE.key -> UTC_OPT.get
+          ) {
+            assert(Hour(Literal.create(null, DateType), UTC_OPT).resolved === false)
+            assert(Hour(Literal(ts), UTC_OPT).resolved)
+            Seq(TimestampType, TimestampNTZType).foreach {
+              dt =>
+                checkEvaluation(Hour(Cast(Literal(d), dt, UTC_OPT), UTC_OPT), 0)
+                checkEvaluation(Hour(Cast(Literal(date), dt, UTC_OPT), UTC_OPT), 13)
+            }
+            checkEvaluation(Hour(Literal(ts), UTC_OPT), 13)
+          }
+
+          val c = Calendar.getInstance()
+          outstandingTimezonesIds.foreach {
+            zid =>
+              withSQLConf(
+                SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+                SQLConf.SESSION_LOCAL_TIMEZONE.key -> zid
+              ) {
+                val timeZoneId = Option(zid)
+                c.setTimeZone(TimeZone.getTimeZone(zid))
+                (0 to 24 by 5).foreach {
+                  h =>
+                    // validate timestamp with local time zone
+                    c.set(2015, 18, 3, h, 29, 59)
+                    checkEvaluation(
+                      Hour(Literal(new Timestamp(c.getTimeInMillis)), timeZoneId),
+                      c.get(Calendar.HOUR_OF_DAY))
+
+                    // validate timestamp without time zone
+                    val localDateTime = LocalDateTime.of(2015, 1, 3, h, 29, 59)
+                    checkEvaluation(Hour(Literal(localDateTime), timeZoneId), h)
+                }
+                Seq(TimestampType, TimestampNTZType).foreach {
+                  dt =>
+                    checkConsistencyBetweenInterpretedAndCodegen(
+                      (child: Expression) => Hour(child, timeZoneId),
+                      dt)
+                }
+              }
+          }
+      }
+    }
+  }
 }
diff --git a/...-ut/spark33/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala b/...-ut/spark33/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala
@@ -777,6 +777,7 @@ class ClickHouseTestSettings extends BackendTestSettings {
     .exclude("Gluten - TIMESTAMP_MICROS")
     .exclude("Gluten - unix_timestamp")
     .exclude("Gluten - to_unix_timestamp")
+    .exclude(GLUTEN_TEST + "Hour")
   enableSuite[GlutenDecimalExpressionSuite]
   enableSuite[GlutenHashExpressionsSuite]
     .exclude("sha2")

diff --git a/gluten-ut/spark33/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala b/gluten-ut/spark33/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala
@@ -123,6 +123,8 @@ class VeloxTestSettings extends BackendTestSettings {
     .exclude("unix_timestamp")
     // Replaced by a gluten test to pass timezone through config.
     .exclude("to_unix_timestamp")
+    // Replaced by a gluten test to pass timezone through config.
+    .exclude("Hour")
     // Unsupported format: yyyy-MM-dd HH:mm:ss.SSS
     .exclude("SPARK-33498: GetTimestamp,UnixTimestamp,ToUnixTimestamp with parseError")
     // Replaced by a gluten test to pass timezone through config.

diff --git a/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala b/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala
@@ -30,8 +30,8 @@ import org.apache.spark.unsafe.types.UTF8String
 
 import java.sql.{Date, Timestamp}
 import java.text.SimpleDateFormat
-import java.time.ZoneId
-import java.util.{Locale, TimeZone}
+import java.time.{LocalDateTime, ZoneId}
+import java.util.{Calendar, Locale, TimeZone}
 import java.util.concurrent.TimeUnit._
 
 class GlutenDateExpressionsSuite extends DateExpressionsSuite with GlutenTestsTrait {
@@ -410,4 +410,68 @@ class GlutenDateExpressionsSuite extends DateExpressionsSuite with GlutenTestsTr
     // Test escaping of format
     GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\""), UTC_OPT) :: Nil)
   }
+
+  test(GLUTEN_TEST + "Hour") {
+    val outstandingTimezonesIds: Seq[String] = Seq(
+      // Velox doesn't support timezones like "UTC".
+      // "UTC",
+      // Due to known issue: "-08:00/+01:00 not found in timezone database",
+      // skip check PST, CET timezone here.
+      // https://github.com/facebookincubator/velox/issues/7804
+      // PST.getId, CET.getId,
+      "Africa/Dakar",
+      LA.getId,
+      "Asia/Urumqi",
+      "Asia/Hong_Kong",
+      "Europe/Brussels"
+    )
+    withDefaultTimeZone(UTC) {
+      Seq("legacy", "corrected").foreach {
+        legacyParserPolicy =>
+          withSQLConf(
+            SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+            SQLConf.SESSION_LOCAL_TIMEZONE.key -> UTC_OPT.get
+          ) {
+            assert(Hour(Literal.create(null, DateType), UTC_OPT).resolved === false)
+            assert(Hour(Literal(ts), UTC_OPT).resolved)
+            Seq(TimestampType, TimestampNTZType).foreach {
+              dt =>
+                checkEvaluation(Hour(Cast(Literal(d), dt, UTC_OPT), UTC_OPT), 0)
+                checkEvaluation(Hour(Cast(Literal(date), dt, UTC_OPT), UTC_OPT), 13)
+            }
+            checkEvaluation(Hour(Literal(ts), UTC_OPT), 13)
+          }
+
+          val c = Calendar.getInstance()
+          outstandingTimezonesIds.foreach {
+            zid =>
+              withSQLConf(
+                SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+                SQLConf.SESSION_LOCAL_TIMEZONE.key -> zid
+              ) {
+                val timeZoneId = Option(zid)
+                c.setTimeZone(TimeZone.getTimeZone(zid))
+                (0 to 24 by 5).foreach {
+                  h =>
+                    // validate timestamp with local time zone
+                    c.set(2015, 18, 3, h, 29, 59)
+                    checkEvaluation(
+                      Hour(Literal(new Timestamp(c.getTimeInMillis)), timeZoneId),
+                      c.get(Calendar.HOUR_OF_DAY))
+
+                    // validate timestamp without time zone
+                    val localDateTime = LocalDateTime.of(2015, 1, 3, h, 29, 59)
+                    checkEvaluation(Hour(Literal(localDateTime), timeZoneId), h)
+                }
+                Seq(TimestampType, TimestampNTZType).foreach {
+                  dt =>
+                    checkConsistencyBetweenInterpretedAndCodegen(
+                      (child: Expression) => Hour(child, timeZoneId),
+                      dt)
+                }
+              }
+          }
+      }
+    }
+  }
 }
diff --git a/...-ut/spark34/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala b/...-ut/spark34/src/test/scala/io/glutenproject/utils/clickhouse/ClickHouseTestSettings.scala
@@ -614,6 +614,7 @@ class ClickHouseTestSettings extends BackendTestSettings {
     .exclude("Gluten - TIMESTAMP_MICROS")
     .exclude("Gluten - unix_timestamp")
     .exclude("Gluten - to_unix_timestamp")
+    .exclude(GLUTEN_TEST + "Hour")
   enableSuite[GlutenDecimalExpressionSuite].exclude("MakeDecimal")
   enableSuite[GlutenHashExpressionsSuite]
     .exclude("sha2")

diff --git a/gluten-ut/spark34/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala b/gluten-ut/spark34/src/test/scala/io/glutenproject/utils/velox/VeloxTestSettings.scala
@@ -103,6 +103,8 @@ class VeloxTestSettings extends BackendTestSettings {
     .exclude("unix_timestamp")
     // Replaced by a gluten test to pass timezone through config.
     .exclude("to_unix_timestamp")
+    // Replaced by a gluten test to pass timezone through config.
+    .exclude("Hour")
     // Unsupported format: yyyy-MM-dd HH:mm:ss.SSS
     .exclude("SPARK-33498: GetTimestamp,UnixTimestamp,ToUnixTimestamp with parseError")
     // Replaced by a gluten test to pass timezone through config.

diff --git a/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala b/...src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenDateExpressionsSuite.scala
@@ -30,8 +30,8 @@ import org.apache.spark.unsafe.types.UTF8String
 
 import java.sql.{Date, Timestamp}
 import java.text.SimpleDateFormat
-import java.time.ZoneId
-import java.util.{Locale, TimeZone}
+import java.time.{LocalDateTime, ZoneId}
+import java.util.{Calendar, Locale, TimeZone}
 import java.util.concurrent.TimeUnit._
 
 class GlutenDateExpressionsSuite extends DateExpressionsSuite with GlutenTestsTrait {
@@ -411,4 +411,67 @@ class GlutenDateExpressionsSuite extends DateExpressionsSuite with GlutenTestsTr
     GenerateUnsafeProjection.generate(FromUnixTime(Literal(0L), Literal("\""), UTC_OPT) :: Nil)
   }
 
+  test(GLUTEN_TEST + "Hour") {
+    val outstandingTimezonesIds: Seq[String] = Seq(
+      // Velox doesn't support timezones like "UTC".
+      // "UTC",
+      // Due to known issue: "-08:00/+01:00 not found in timezone database",
+      // skip check PST, CET timezone here.
+      // https://github.com/facebookincubator/velox/issues/7804
+      // PST.getId, CET.getId,
+      "Africa/Dakar",
+      LA.getId,
+      "Asia/Urumqi",
+      "Asia/Hong_Kong",
+      "Europe/Brussels"
+    )
+    withDefaultTimeZone(UTC) {
+      Seq("legacy", "corrected").foreach {
+        legacyParserPolicy =>
+          withSQLConf(
+            SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+            SQLConf.SESSION_LOCAL_TIMEZONE.key -> UTC_OPT.get
+          ) {
+            assert(Hour(Literal.create(null, DateType), UTC_OPT).resolved === false)
+            assert(Hour(Literal(ts), UTC_OPT).resolved)
+            Seq(TimestampType, TimestampNTZType).foreach {
+              dt =>
+                checkEvaluation(Hour(Cast(Literal(d), dt, UTC_OPT), UTC_OPT), 0)
+                checkEvaluation(Hour(Cast(Literal(date), dt, UTC_OPT), UTC_OPT), 13)
+            }
+            checkEvaluation(Hour(Literal(ts), UTC_OPT), 13)
+          }
+
+          val c = Calendar.getInstance()
+          outstandingTimezonesIds.foreach {
+            zid =>
+              withSQLConf(
+                SQLConf.LEGACY_TIME_PARSER_POLICY.key -> legacyParserPolicy,
+                SQLConf.SESSION_LOCAL_TIMEZONE.key -> zid
+              ) {
+                val timeZoneId = Option(zid)
+                c.setTimeZone(TimeZone.getTimeZone(zid))
+                (0 to 24 by 5).foreach {
+                  h =>
+                    // validate timestamp with local time zone
+                    c.set(2015, 18, 3, h, 29, 59)
+                    checkEvaluation(
+                      Hour(Literal(new Timestamp(c.getTimeInMillis)), timeZoneId),
+                      c.get(Calendar.HOUR_OF_DAY))
+
+                    // validate timestamp without time zone
+                    val localDateTime = LocalDateTime.of(2015, 1, 3, h, 29, 59)
+                    checkEvaluation(Hour(Literal(localDateTime), timeZoneId), h)
+                }
+                Seq(TimestampType, TimestampNTZType).foreach {
+                  dt =>
+                    checkConsistencyBetweenInterpretedAndCodegen(
+                      (child: Expression) => Hour(child, timeZoneId),
+                      dt)
+                }
+              }
+          }
+      }
+    }
+  }
 }