MaxGekk commented on code in PR #50355:
URL: https://github.com/apache/spark/pull/50355#discussion_r2010667620
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala:
##########
@@ -233,3 +233,74 @@ object MinuteExpressionBuilder extends ExpressionBuilder {
}
}
}
+
+@ExpressionDescription(
+ usage = """
+ _FUNC_(time_expr) - Returns the hour component of the given time.
+ """,
+ examples = """
+ Examples:
+ > SELECT _FUNC_(TIME'11:35:35.999999');
+ 11
+ """,
+ since = "4.1.0",
+ group = "datetime_funcs")
+// scalastyle:on line.size.limit
+case class HoursOfTime(child: Expression)
+ extends RuntimeReplaceable
+ with ExpectsInputTypes {
+
+ override def replacement: Expression = StaticInvoke(
+ classOf[DateTimeUtils.type],
+ IntegerType,
+ "getHoursOfTime",
+ Seq(child),
+ Seq(child.dataType)
+ )
+
+ override def inputTypes: Seq[AbstractDataType] = Seq(TimeType())
+
+ override def children: Seq[Expression] = Seq(child)
+
+ override def prettyName: String = "hour"
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): Expression = {
+ copy(child = newChildren.head)
+ }
+}
+
Review Comment:
nit: remove the blank line.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala:
##########
@@ -53,6 +53,61 @@ class TimeExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
parameters = Map("input" -> "'100:50'", "format" -> "'mm:HH'"))
}
+ test("HourExpressionBuilder") {
+ // Empty expressions list
+ checkError(
+ exception = intercept[AnalysisException] {
+ MinuteExpressionBuilder.build("hour", Seq.empty)
+ },
+ condition = "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ parameters = Map(
+ "functionName" -> "`hour`",
+ "expectedNum" -> "> 0",
+ "actualNum" -> "0",
+ "docroot" -> SPARK_DOC_ROOT)
+ )
+
+ // test TIME-typed child should build HoursOfTime
+ val timeExpr = Literal(localTime(12, 58, 59), TimeType())
+ val builtExprForTime = HourExpressionBuilder.build("hour", Seq(timeExpr))
+ assert(builtExprForTime.isInstanceOf[HoursOfTime])
+ assert(builtExprForTime.asInstanceOf[HoursOfTime].child eq timeExpr)
+
+ // test non TIME-typed child should build hour
+ val tsExpr = Literal("2007-09-03 10:45:23")
+ val builtExprForTs = HourExpressionBuilder.build("hour", Seq(tsExpr))
+ assert(builtExprForTs.isInstanceOf[Hour])
+ assert(builtExprForTs.asInstanceOf[Hour].child eq tsExpr)
+ }
+
+ test("Hour with TIME type") {
+ // A few test times in microseconds since midnight:
+ // time in microseconds -> expected minute
+ val testTimes = Seq(
+ localTime() -> 0,
+ localTime(1) -> 1,
+ localTime(0, 59) -> 0,
+ localTime(14, 30) -> 14,
+ localTime(12, 58, 59) -> 12,
+ localTime(23, 0, 1) -> 23,
+ localTime(23, 59, 59, 999999) -> 23
+ )
+
+ // Create a literal with TimeType() for each test microsecond value
+ // evaluate MinutesOfTime(...), and check that the result matches the
expected minute.
Review Comment:
```suggestion
// evaluate HoursOfTime(...), and check that the result matches the
expected minute.
```
##########
sql/core/src/test/resources/sql-functions/sql-expression-schema.md:
##########
@@ -223,7 +223,7 @@
| org.apache.spark.sql.catalyst.expressions.Md5 | md5 | SELECT md5('Spark') |
struct<md5(Spark):string> |
| org.apache.spark.sql.catalyst.expressions.MicrosToTimestamp |
timestamp_micros | SELECT timestamp_micros(1230219000123123) |
struct<timestamp_micros(1230219000123123):timestamp> |
| org.apache.spark.sql.catalyst.expressions.MillisToTimestamp |
timestamp_millis | SELECT timestamp_millis(1230219000123) |
struct<timestamp_millis(1230219000123):timestamp> |
-| org.apache.spark.sql.catalyst.expressions.Minute | minute | SELECT
minute('2009-07-30 12:58:59') | struct<minute(2009-07-30 12:58:59):int> |
+| org.apache.spark.sql.catalyst.expressions.MinuteExpressionBuilder | minute |
SELECT minute('2009-07-30 12:58:59') | struct<minute(2009-07-30 12:58:59):int> |
Review Comment:
Please, rebase on the master. This PR
https://github.com/apache/spark/pull/50361 has been merged.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala:
##########
@@ -53,6 +53,61 @@ class TimeExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
parameters = Map("input" -> "'100:50'", "format" -> "'mm:HH'"))
}
+ test("HourExpressionBuilder") {
+ // Empty expressions list
+ checkError(
+ exception = intercept[AnalysisException] {
+ MinuteExpressionBuilder.build("hour", Seq.empty)
+ },
+ condition = "WRONG_NUM_ARGS.WITHOUT_SUGGESTION",
+ parameters = Map(
+ "functionName" -> "`hour`",
+ "expectedNum" -> "> 0",
+ "actualNum" -> "0",
+ "docroot" -> SPARK_DOC_ROOT)
+ )
+
+ // test TIME-typed child should build HoursOfTime
+ val timeExpr = Literal(localTime(12, 58, 59), TimeType())
+ val builtExprForTime = HourExpressionBuilder.build("hour", Seq(timeExpr))
+ assert(builtExprForTime.isInstanceOf[HoursOfTime])
+ assert(builtExprForTime.asInstanceOf[HoursOfTime].child eq timeExpr)
+
+ // test non TIME-typed child should build hour
+ val tsExpr = Literal("2007-09-03 10:45:23")
+ val builtExprForTs = HourExpressionBuilder.build("hour", Seq(tsExpr))
+ assert(builtExprForTs.isInstanceOf[Hour])
+ assert(builtExprForTs.asInstanceOf[Hour].child eq tsExpr)
+ }
+
+ test("Hour with TIME type") {
+ // A few test times in microseconds since midnight:
+ // time in microseconds -> expected minute
Review Comment:
```suggestion
// time in microseconds -> expected hours
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
