MaxGekk commented on code in PR #50557:
URL: https://github.com/apache/spark/pull/50557#discussion_r2043955377


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala:
##########
@@ -162,6 +163,186 @@ object TryToTimeExpressionBuilder extends 
ExpressionBuilder {
   }
 }
 
+/**
+ * Converts or extracts time from various input types.
+ */
+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = """
+    _FUNC_(expr) - Extracts the time part from or converts the given 
expression to a time.
+    The expression can be a string, timestamp, or numeric value.
+  """,
+  arguments = """
+    Arguments:
+      * expr - An expression that can be one of:
+          - A string representing a time
+          - A timestamp value
+          - A numeric value representing total seconds
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_('12:25:13.45');
+       12:25:13.45
+      > SELECT _FUNC_(timestamp'2020-04-30 12:25:13.45');
+       12:25:13.45
+      > SELECT _FUNC_(123);
+       00:02:03
+  """,
+  group = "datetime_funcs",
+  since = "4.1.0")
+// scalastyle:on line.size.limit
+case class Time(expr: Expression)
+  extends UnaryExpression with ExpectsInputTypes {
+
+  override def child: Expression = expr
+
+  override def dataType: DataType = TimeType()
+
+  override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType)
+
+  override def prettyName: String = "time"
+
+  override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): 
ExprCode = {
+    val castExpr = Cast(child, TimeType())
+    castExpr.genCode(ctx)
+  }
+
+  override protected def nullSafeEval(input: Any): Any = {
+    Cast(child, TimeType()).eval(null)

Review Comment:
   This is not right way of implementing a replacement. Please, take a look at
   
https://github.com/apache/spark/blob/203942c583187b6d0a012ff2b2d6aab5c664bd39/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala#L2180-L2184



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala:
##########
@@ -740,6 +744,28 @@ case class Cast(
       } else {
         buildCast[UTF8String](_, s => DateTimeUtils.stringToTime(s).orNull)
       }
+
+    case TimestampType =>

Review Comment:
   The scope of this PR should be adding the `time()` function alias for 
`CAST`. Let's implement CAST and test it separately, please.



-- 
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

Reply via email to