mbutrovich commented on code in PR #2316:
URL: https://github.com/apache/datafusion-comet/pull/2316#discussion_r2333714625
##########
spark/src/main/scala/org/apache/comet/serde/datetime.scala:
##########
@@ -19,19 +19,43 @@
package org.apache.comet.serde
-import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateSub,
Hour, Literal, Minute, Second, TruncDate, TruncTimestamp, Year}
+import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateSub,
DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, Literal, Minute, Month,
Quarter, Second, TruncDate, TruncTimestamp, WeekDay, WeekOfYear, Year}
import org.apache.spark.sql.types.{DateType, IntegerType}
import org.apache.comet.CometSparkSessionExtensions.withInfo
+import org.apache.comet.serde.CometGetDateField.CometGetDateField
import org.apache.comet.serde.ExprOuterClass.Expr
import org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal,
optExprWithInfo, scalarFunctionExprToProto,
scalarFunctionExprToProtoWithReturnType, serializeDataType}
-object CometYear extends CometExpressionSerde[Year] {
- override def convert(
- expr: Year,
+private object CometGetDateField extends Enumeration {
+ type CometGetDateField = Value
+
+ // See:
https://datafusion.apache.org/user-guide/sql/scalar_functions.html#date-part
+ val Year: Value = Value("year")
Review Comment:
Since all we do is call `.toString` on this to create a `Litera`, is there a
reason to define the enum using `Value` and not string literals?
##########
spark/src/main/scala/org/apache/comet/serde/datetime.scala:
##########
@@ -19,19 +19,43 @@
package org.apache.comet.serde
-import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateSub,
Hour, Literal, Minute, Second, TruncDate, TruncTimestamp, Year}
+import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateSub,
DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, Literal, Minute, Month,
Quarter, Second, TruncDate, TruncTimestamp, WeekDay, WeekOfYear, Year}
import org.apache.spark.sql.types.{DateType, IntegerType}
import org.apache.comet.CometSparkSessionExtensions.withInfo
+import org.apache.comet.serde.CometGetDateField.CometGetDateField
import org.apache.comet.serde.ExprOuterClass.Expr
import org.apache.comet.serde.QueryPlanSerde.{exprToProtoInternal,
optExprWithInfo, scalarFunctionExprToProto,
scalarFunctionExprToProtoWithReturnType, serializeDataType}
-object CometYear extends CometExpressionSerde[Year] {
- override def convert(
- expr: Year,
+private object CometGetDateField extends Enumeration {
+ type CometGetDateField = Value
+
+ // See:
https://datafusion.apache.org/user-guide/sql/scalar_functions.html#date-part
+ val Year: Value = Value("year")
Review Comment:
Since all we do is call `.toString` on this to create a `Litera`, is there a
reason to define the enum using `Value` and not string literals?
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
