Copilot commented on code in PR #12954:
URL: https://github.com/apache/gluten/pull/12954#discussion_r3911572198
##########
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala:
##########
@@ -1467,13 +1467,12 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi
with Logging {
left: ExpressionTransformer,
right: ExpressionTransformer,
original: Expression): ExpressionTransformer = {
- // Since spark 3.3.0
- val extract =
-
SparkShimLoader.getSparkShims.extractExpressionTimestampDiffUnit(original)
- if (extract.isEmpty) {
- throw new UnsupportedOperationException(s"Not support expression
TimestampDiff.")
+ val unit = original match {
+ case timestampDiff: TimestampDiff => timestampDiff.unit
+ case _ =>
+ throw new UnsupportedOperationException(s"Not support expression
TimestampDiff.")
}
Review Comment:
The thrown message "Not support expression TimestampDiff." is grammatically
incorrect and not very descriptive for users debugging unsupported expressions.
Consider rephrasing it to a clear, actionable error message (and drop the
unnecessary string interpolation).
##########
shims/common/src/main/scala/org/apache/gluten/execution/datasource/GlutenFormatWriterInjects.scala:
##########
@@ -76,13 +76,6 @@ object GlutenFormatFactory {
postRuleFactory = factory
}
Review Comment:
`injectPostRuleFactory` only assigns `postRuleFactory`, but
`postRuleFactory` is no longer read anywhere (the previous reader
`getExtendedColumnarPostRule` was removed). This leaves a public-ish injection
API that has no effect and can mislead callers; consider either removing
`postRuleFactory`/`injectPostRuleFactory` and their call sites, or
reintroducing a single well-defined read path for the injected rule.
--
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]
