rui-mo commented on code in PR #10567:
URL:
https://github.com/apache/incubator-gluten/pull/10567#discussion_r2313575695
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala:
##########
@@ -276,26 +276,38 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
substraitExprName,
replaceWithExpressionTransformer0(r.child, attributeSeq,
expressionsMap),
r)
- case t: ToUnixTimestamp =>
+ case expr @ (_: ToUnixTimestamp | _: UnixTimestamp) =>
+ // Extract common fields - both ToUnixTimestamp and UnixTimestamp have
the same fields
+ val (timeExp, format, timeZoneId, failOnError) = expr match {
+ case t: ToUnixTimestamp => (t.timeExp, t.format, t.timeZoneId,
t.failOnError)
+ case u: UnixTimestamp => (u.timeExp, u.format, u.timeZoneId,
u.failOnError)
+ }
// The failOnError depends on the config for ANSI. ANSI is not
supported currently.
// And timeZoneId is passed to backend config.
- GenericExpressionTransformer(
- substraitExprName,
- Seq(
- replaceWithExpressionTransformer0(t.timeExp, attributeSeq,
expressionsMap),
- replaceWithExpressionTransformer0(t.format, attributeSeq,
expressionsMap)
- ),
- t
- )
- case u: UnixTimestamp =>
- GenericExpressionTransformer(
- substraitExprName,
- Seq(
- replaceWithExpressionTransformer0(u.timeExp, attributeSeq,
expressionsMap),
- replaceWithExpressionTransformer0(u.format, attributeSeq,
expressionsMap)
- ),
- ToUnixTimestamp(u.timeExp, u.format, u.timeZoneId, u.failOnError)
- )
+ val toUnixTimestamp = ToUnixTimestamp(timeExp, format, timeZoneId,
failOnError)
+ val timeExpTransformer =
+ replaceWithExpressionTransformer0(timeExp, attributeSeq,
expressionsMap)
+
+ // For timestamp and date inputs, the format parameter is ignored as
per Spark behavior.
+ timeExp.dataType match {
+ case _: TimestampType =>
+ // For timestamp input, use custom transformer to generate correct
signature
+ ToUnixTimestampTransformer(
+ substraitExprName,
+ timeExpTransformer,
+ toUnixTimestamp
+ )
+ case _ =>
+ // For other inputs (date, string), use original logic
+ GenericExpressionTransformer(
+ substraitExprName,
+ Seq(
+ timeExpTransformer,
+ replaceWithExpressionTransformer0(format, attributeSeq,
expressionsMap)
+ ),
+ toUnixTimestamp
+ )
+ }
Review Comment:
The specific transformer can be used to handle different logics between
Timestamp and String types, and 'ExpressionConverter' could be much simplified
to just call the transformer. FYI: I added the modified version in my commit:
https://github.com/rui-mo/gazelle-jni/commit/0430595e01cf0b8d59c6ee53248e81a4e2186cf7.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
