datafusion-comet icon indicating copy to clipboard operation
datafusion-comet copied to clipboard

Chore: Fix Scala code warnings - Spark module

Open andy-hf-kwok opened this issue 2 months ago • 3 comments

Which issue does this PR close?

Partially closes https://github.com/apache/datafusion-comet/issues/2255

Rationale for this change

  • To perform code clean / refactor on the codebase, in order to comply with the new maven profile https://github.com/apache/datafusion-comet/issues/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.

andy-hf-kwok avatar Oct 13 '25 06:10 andy-hf-kwok

@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

coderfender avatar Oct 13 '25 07:10 coderfender

Codecov Report

:x: Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.79%. Comparing base (f09f8af) to head (7003584). :warning: Report is 643 commits behind head on main.

Files with missing lines Patch % Lines
...park/src/main/scala/org/apache/spark/Plugins.scala 0.00% 2 Missing :warning:
...e/spark/sql/comet/CometBroadcastExchangeExec.scala 0.00% 2 Missing :warning:
...pache/spark/sql/comet/CometColumnarToRowExec.scala 0.00% 2 Missing :warning:
...n/scala/org/apache/comet/rules/CometScanRule.scala 0.00% 1 Missing :warning:
...apache/spark/sql/comet/CometCollectLimitExec.scala 0.00% 1 Missing :warning:
...ark/sql/comet/CometTakeOrderedAndProjectExec.scala 0.00% 1 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Oct 13 '25 17:10 codecov-commenter

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. https://github.com/apache/datafusion-comet/pull/2588

Thanks,

andy-hf-kwok avatar Oct 16 '25 04:10 andy-hf-kwok