Chore: Fix Scala code warnings - Spark module#2558
Chore: Fix Scala code warnings - Spark module#2558andy-hf-kwok wants to merge 11 commits intoapache:mainfrom
Conversation
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com> # Conflicts: # spark/src/main/scala/org/apache/comet/Native.scala
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
Signed-off-by: Andy HF Kwok <andy.hf.kwok@gmail.com>
| inputs: Seq[Attribute], | ||
| binding: Boolean, | ||
| conf: SQLConf): Option[ExprOuterClass.AggExpr] = { | ||
| @unused conf: SQLConf): Option[ExprOuterClass.AggExpr] = { |
There was a problem hiding this comment.
If this is unused can we just remove it?
| partitionBuilder.addPartitionedFile(fileBuilder.build()) | ||
| }) | ||
| nativeScanBuilder.addFilePartitions(partitionBuilder.build()) | ||
| nativeScanBuilder.addFilePartitions(partitionBuilder.build()); () |
| ExprOuterClass.Expr | ||
| .newBuilder() | ||
| .setBound(boundExpr) | ||
| .build())) |
There was a problem hiding this comment.
Not sure if this formatting error is intended ?
| <arg>-Xlint:_</arg> | ||
| UnsupportedException being thrown on SparkPlan default. | ||
| <arg>-Ywarn-dead-code</arg> | ||
| --> |
There was a problem hiding this comment.
Not sure if this comment is necessary ? or may be I am missing something here
| 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)) |
There was a problem hiding this comment.
Unintended formatting issue ?
|
|
||
| import org.apache.hadoop.fs.Path | ||
|
|
||
| import org.apache.spark.sql.SparkSession |
There was a problem hiding this comment.
Unintended formatting issue ?
| logInfo(s"Setting $extensionKey=$extensionClass") | ||
| conf.set(extensionKey, extensionClass) | ||
| conf.set(extensionKey, extensionClass); () | ||
| } else { |
There was a problem hiding this comment.
Perhaps an unintended () ?
| override protected def doPrepare(): Unit = { | ||
| // Materialize the future. | ||
| relationFuture | ||
| relationFuture; () |
There was a problem hiding this comment.
Perhaps an unintended () ?
| var mapStatus: MapStatus = _ | ||
| // Store MapStatus opaquely as AnyRef, | ||
| // to avoid private[spark] visibility issues; cast back when needed. | ||
| var mapStatus: AnyRef = _ |
There was a problem hiding this comment.
Any reason why we are removing type assignment of AnyRef ?
|
@andy-hf-kwok , Thank you for the PR . I would recommend you to run Thank you |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2558 +/- ##
=============================================
- Coverage 56.12% 41.79% -14.33%
- Complexity 976 1105 +129
=============================================
Files 119 147 +28
Lines 11743 13642 +1899
Branches 2251 2369 +118
=============================================
- Hits 6591 5702 -889
- Misses 4012 6974 +2962
+ Partials 1140 966 -174 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @coderfender @wForget , thx for the review. Thanks, |
4 participants
Which issue does this PR close?
Partially closes #2255
Rationale for this change
What changes are included in this PR?
-Xlint:_and-Ywarn-dead-codeas a huge of refactor / package relocation will be required.numeric-widenissuesupdateto have no return (Side effect only)MapStatuswith AnyRef, to avoid direct reference to package private member.How are these changes tested?
And make sure mvn can produce the artifact with all test passed.