coderfender commented on code in PR #2558:
URL: https://github.com/apache/datafusion-comet/pull/2558#discussion_r2425400878
##########
pom.xml:
##########
@@ -741,12 +741,16 @@ under the License.
<arg>-deprecation</arg>
<arg>-unchecked</arg>
<arg>-feature</arg>
- <arg>-Xlint:_</arg>
- <arg>-Ywarn-dead-code</arg>
<arg>-Ywarn-numeric-widen</arg>
<arg>-Ywarn-value-discard</arg>
<arg>-Ywarn-unused:imports,patvars,privates,locals,params,-implicits</arg>
<arg>-Xfatal-warnings</arg>
+ <!--
+ Private trait reference under class
CometShuffleManager
+ <arg>-Xlint:_</arg>
+ UnsupportedException being thrown on SparkPlan
default.
+ <arg>-Ywarn-dead-code</arg>
+ -->
Review Comment:
Not sure if this comment is necessary ? or may be I am missing something here
##########
spark/src/main/scala/org/apache/comet/parquet/SourceFilterSerde.scala:
##########
@@ -50,11 +50,12 @@ object SourceFilterSerde extends Logging {
.setDatatype(dataType.get)
.build()
Some(
- field.dataType,
- ExprOuterClass.Expr
- .newBuilder()
- .setBound(boundExpr)
- .build())
+ (
+ field.dataType,
+ ExprOuterClass.Expr
+ .newBuilder()
+ .setBound(boundExpr)
+ .build()))
Review Comment:
Not sure if this formatting error is intended ?
##########
spark/src/main/scala/org/apache/spark/Plugins.scala:
##########
@@ -100,13 +100,13 @@ object CometDriverPlugin extends Logging {
val extensions = conf.get(extensionKey, "")
if (extensions.isEmpty) {
logInfo(s"Setting $extensionKey=$extensionClass")
- conf.set(extensionKey, extensionClass)
+ conf.set(extensionKey, extensionClass); ()
} else {
Review Comment:
Perhaps an unintended `()` ?
##########
spark/src/main/scala/org/apache/spark/sql/comet/CometBroadcastExchangeExec.scala:
##########
@@ -198,7 +198,7 @@ case class CometBroadcastExchangeExec(
override protected def doPrepare(): Unit = {
// Materialize the future.
- relationFuture
+ relationFuture; ()
Review Comment:
Perhaps an unintended `()` ?
##########
spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimCometScanExec.scala:
##########
@@ -42,7 +43,7 @@ trait ShimCometScanExec {
def isSparkVersionAtLeast355: Boolean = {
VersionUtils.majorMinorPatchVersion(SPARK_VERSION_SHORT) match {
- case Some((major, minor, patch)) => (major, minor, patch) >= (3, 5, 5)
+ case Some((major, minor, patch)) => (major, minor, patch) >= ((3, 5, 5))
Review Comment:
Unintended formatting issue ?
##########
spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimCometScanExec.scala:
##########
@@ -21,7 +21,6 @@ package org.apache.spark.sql.comet.shims
import org.apache.hadoop.fs.Path
-
import org.apache.spark.sql.SparkSession
Review Comment:
Unintended formatting issue ?
##########
spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometNativeShuffleWriter.scala:
##########
@@ -60,7 +60,9 @@ class CometNativeShuffleWriter[K, V](
private val OFFSET_LENGTH = 8
var partitionLengths: Array[Long] = _
- var mapStatus: MapStatus = _
+ // Store MapStatus opaquely as AnyRef,
+ // to avoid private[spark] visibility issues; cast back when needed.
+ var mapStatus: AnyRef = _
Review Comment:
Any reason why we are removing type assignment of `AnyRef` ?
--
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]
