Chore: Fix Scala code warnings - Spark module #2558

andy-hf-kwok · 2025-10-13T06:25:39Z

Which issue does this PR close?

Partially closes #2255

Rationale for this change

To perform code clean / refactor on the codebase, in order to comply with the new maven profile chore: Fix Scala code warnings #2255, one module at a time.

What changes are included in this PR?

Disable -Xlint:_ and -Ywarn-dead-code as a huge of refactor / package relocation will be required.
Address all numeric-widen issues
Import re-order
Replace map.put with update to have no return (Side effect only)
Replace the reference of MapStatus with AnyRef, to avoid direct reference to package private member.

How are these changes tested?

mvn install -pl :comet-spark-spark3.5_2.12 -Pstrict-warnings

And make sure mvn can produce the artifact with all test passed.

Signed-off-by: Andy HF Kwok <[email protected]> # Conflicts: # spark/src/main/scala/org/apache/comet/Native.scala

Signed-off-by: Andy HF Kwok <[email protected]>

wForget · 2025-10-13T06:33:13Z

spark/src/main/scala/org/apache/comet/serde/aggregates.scala

      inputs: Seq[Attribute],
      binding: Boolean,
-      conf: SQLConf): Option[ExprOuterClass.AggExpr] = {
+      @unused conf: SQLConf): Option[ExprOuterClass.AggExpr] = {


If this is unused can we just remove it?

wForget · 2025-10-13T06:37:02Z

spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

      partitionBuilder.addPartitionedFile(fileBuilder.build())
    })
-    nativeScanBuilder.addFilePartitions(partitionBuilder.build())
+    nativeScanBuilder.addFilePartitions(partitionBuilder.build()); ()


This seems incomprehensible.

coderfender · 2025-10-13T07:15:05Z

spark/src/main/scala/org/apache/comet/parquet/SourceFilterSerde.scala

+            ExprOuterClass.Expr
+              .newBuilder()
+              .setBound(boundExpr)
+              .build()))


Not sure if this formatting error is intended ?

coderfender · 2025-10-13T07:17:01Z

pom.xml

+                              <arg>-Xlint:_</arg>
+                              UnsupportedException being thrown on SparkPlan default.
+                              <arg>-Ywarn-dead-code</arg>
+                              -->


Not sure if this comment is necessary ? or may be I am missing something here

coderfender · 2025-10-13T07:18:21Z

spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimCometScanExec.scala

  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))


Unintended formatting issue ?

coderfender · 2025-10-13T07:18:39Z

spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimCometScanExec.scala


 import org.apache.hadoop.fs.Path
-
 import org.apache.spark.sql.SparkSession


Unintended formatting issue ?

coderfender · 2025-10-13T07:19:02Z

spark/src/main/scala/org/apache/spark/Plugins.scala

      logInfo(s"Setting $extensionKey=$extensionClass")
-      conf.set(extensionKey, extensionClass)
+      conf.set(extensionKey, extensionClass); ()
    } else {


Perhaps an unintended () ?

coderfender · 2025-10-13T07:20:05Z

spark/src/main/scala/org/apache/spark/sql/comet/CometBroadcastExchangeExec.scala

  override protected def doPrepare(): Unit = {
    // Materialize the future.
-    relationFuture
+    relationFuture; ()


Perhaps an unintended () ?

coderfender · 2025-10-13T07:21:07Z

...k/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/CometNativeShuffleWriter.scala

-  var mapStatus: MapStatus = _
+  // Store MapStatus opaquely as AnyRef,
+  // to avoid private[spark] visibility issues; cast back when needed.
+  var mapStatus: AnyRef = _


Any reason why we are removing type assignment of AnyRef ?

coderfender · 2025-10-13T07:22:51Z

@andy-hf-kwok , Thank you for the PR . I would recommend you to run make format and make release to hash out unwanted formatting / compile issues .

Thank you

codecov-commenter · 2025-10-13T17:59:06Z

Codecov Report

❌ Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.79%. Comparing base (f09f8af) to head (7003584).
⚠️ Report is 596 commits behind head on main.

Files with missing lines	Patch %	Lines
...park/src/main/scala/org/apache/spark/Plugins.scala	0.00%	2 Missing ⚠️
...e/spark/sql/comet/CometBroadcastExchangeExec.scala	0.00%	2 Missing ⚠️
...pache/spark/sql/comet/CometColumnarToRowExec.scala	0.00%	2 Missing ⚠️
...n/scala/org/apache/comet/rules/CometScanRule.scala	0.00%	1 Missing ⚠️
...apache/spark/sql/comet/CometCollectLimitExec.scala	0.00%	1 Missing ⚠️
...ark/sql/comet/CometTakeOrderedAndProjectExec.scala	0.00%	1 Missing ⚠️

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

andy-hf-kwok · 2025-10-16T04:08:20Z

Hi @coderfender @wForget , thx for the review.
After going through your comments, I realized that fixing all warnings in a single PR might not be the best approach — it could get quite complex.
Instead, I’m planning to split this effort into multiple PRs, with each one addressing a specific warning option individually.
Below is the first one which aim to address to numeric widen issue.
#2588

Thanks,

andy-hf-kwok added 11 commits October 10, 2025 23:28

Suppress native method not used waning

2a0013f

Signed-off-by: Andy HF Kwok <[email protected]> # Conflicts: # spark/src/main/scala/org/apache/comet/Native.scala

Up cast

4c47c69

Signed-off-by: Andy HF Kwok <[email protected]>

Scala warning fix

9e5d9f8

Signed-off-by: Andy HF Kwok <[email protected]>

Fix

01ee249

Signed-off-by: Andy HF Kwok <[email protected]>

Format fix

7e5b0fd

Signed-off-by: Andy HF Kwok <[email protected]>

Suppress warning

a5224e9

Signed-off-by: Andy HF Kwok <[email protected]>

Fix cast issue

feb749e

Signed-off-by: Andy HF Kwok <[email protected]>

Import fix

1378f1f

Signed-off-by: Andy HF Kwok <[email protected]>

Fix Lint

be3edd7

Signed-off-by: Andy HF Kwok <[email protected]>

Update comment

7cb8f54

Signed-off-by: Andy HF Kwok <[email protected]>

Typo

7003584

Signed-off-by: Andy HF Kwok <[email protected]>

wForget reviewed Oct 13, 2025

View reviewed changes

coderfender reviewed Oct 13, 2025

View reviewed changes

andy-hf-kwok mentioned this pull request Oct 13, 2025

chore: Fix Scala code warnings #2255

Open


		import org.apache.hadoop.fs.Path

		import org.apache.spark.sql.SparkSession

Chore: Fix Scala code warnings - Spark module #2558

Are you sure you want to change the base?

Chore: Fix Scala code warnings - Spark module #2558

Uh oh!

Conversation

andy-hf-kwok commented Oct 13, 2025

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderfender commented Oct 13, 2025

Uh oh!

codecov-commenter commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andy-hf-kwok commented Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Oct 13, 2025 •

edited

Loading