Skip to content

Conversation

@hsiang-c
Copy link
Contributor

@hsiang-c hsiang-c commented Sep 12, 2025

Which issue does this PR close?

Partially closes #. #2255

Rationale for this change

What changes are included in this PR?

  • Depend on scala-collection-compat b/c Comet still have Scala 2.12 builds.
  • scala.collection.JavaConverters is deprecated, replace it with scala.jdk.CollectionConverters (scala) and scala.jdk.javaapi.CollectionConverters (Java)
  • Include scala-collection-compat in JAR and fixed a few release sanity checks.

Scala 2.12 and Scala 2.13 JAR content diff

➜  datafusion-comet git:(java_scala_converters) ✗ jar tf ~/.m2/repository/org/apache/datafusion/comet-spark-spark3.4_2.12/0.11.0-SNAPSHOT/comet-spark-spark3.4_2.12-0.11.0-SNAPSHOT.jar | grep "jdk"
scala/jdk/
scala/jdk/CollectionConverters$.class
scala/jdk/CollectionConverters.class
scala/jdk/OptionConverters$.class
scala/jdk/OptionConverters$RichOption$.class
scala/jdk/OptionConverters$RichOption.class
scala/jdk/OptionConverters$RichOptional$.class
scala/jdk/OptionConverters$RichOptional.class
scala/jdk/OptionConverters$RichOptionalDouble$.class
scala/jdk/OptionConverters$RichOptionalDouble.class
scala/jdk/OptionConverters$RichOptionalInt$.class
scala/jdk/OptionConverters$RichOptionalInt.class
scala/jdk/OptionConverters$RichOptionalLong$.class
scala/jdk/OptionConverters$RichOptionalLong.class
scala/jdk/OptionConverters.class
scala/jdk/OptionShape$$anon$1.class
scala/jdk/OptionShape$$anon$2.class
scala/jdk/OptionShape$$anon$3.class
scala/jdk/OptionShape$.class
scala/jdk/OptionShape.class
scala/jdk/javaapi/
scala/jdk/javaapi/CollectionConverters$.class
scala/jdk/javaapi/CollectionConverters.class

➜  datafusion-comet git:(java_scala_converters) ✗ jar tf ~/.m2/repository/org/apache/datafusion/comet-spark-spark3.4_2.13/0.11.0-SNAPSHOT/comet-spark-spark3.4_2.13-0.11.0-SNAPSHOT.jar | grep "jdk"
# No `scala/jdk` classes are included b/c they are part of Scala 2.13

Size diff (in bytes) before and after this patch

Spark 3.4

Before After Delta
Scala 2.12 34992968 35300148 + 0.87%
Scala 2.13 35013725 35019448 + 0.016%

Spark 3.5

Before After Delta
Scala 2.12 34995698 35302876 + 0.87%
Scala 2.13 N/A (my local build doesn't work for a different reason) N/A N/A

Spark 4.0

Before After Delta
Scala 2.13 35026870 35032594 + 0.016%

How are these changes tested?

  • Tested locally w/ Maven build profile -Pscala-2.12 and -Pscala-2.13
  • make release-nogit PROFILES="-Pspark-3.4 -Pscala2.12"
  • make release-nogit PROFILES="-Pspark-3.4 -Pscala2.13"
  • make release-nogit PROFILES="-Pspark-3.5 -Pscala2.12"
  • make release-nogit PROFILES="-Pspark-4.0 -Pscala2.13"
  • SparkSQL tests no longer have java.lang.NoClassDefFoundError: scala/jdk/javaapi/CollectionConverters exception.

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.06%. Comparing base (f09f8af) to head (4b0edba).
⚠️ Report is 500 commits behind head on main.

Files with missing lines Patch % Lines
...va/org/apache/comet/parquet/NativeBatchReader.java 0.00% 7 Missing ⚠️
.../apache/comet/parquet/CometParquetFileFormat.scala 0.00% 2 Missing ⚠️
...a/org/apache/comet/parquet/NativeColumnReader.java 0.00% 1 Missing ⚠️
...et/execution/shuffle/CometUnsafeShuffleWriter.java 0.00% 1 Missing ⚠️
...t/parquet/CometParquetPartitionReaderFactory.scala 0.00% 1 Missing ⚠️
...n/scala/org/apache/comet/rules/CometScanRule.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #2393       +/-   ##
=============================================
- Coverage     56.12%   33.06%   -23.07%     
+ Complexity      976      736      -240     
=============================================
  Files           119      147       +28     
  Lines         11743    13408     +1665     
  Branches       2251     2332       +81     
=============================================
- Hits           6591     4433     -2158     
- Misses         4012     8183     +4171     
+ Partials       1140      792      -348     

☔ 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.

@hsiang-c hsiang-c marked this pull request as draft September 12, 2025 22:06
@hsiang-c hsiang-c marked this pull request as ready for review September 14, 2025 22:22
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hsiang-c Triggering CI

Copy link
Contributor

@mbutrovich mbutrovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hsiang-c!

@mbutrovich mbutrovich merged commit f0f0653 into apache:main Sep 16, 2025
101 of 102 checks passed
@hsiang-c hsiang-c deleted the java_scala_converters branch September 16, 2025 22:48
coderfender pushed a commit to coderfender/datafusion-comet that referenced this pull request Dec 13, 2025
…pache#2393)

* Use scala-collection-compat for Spark 3.4

* Use scala.jdk.CollectionConverters for Scala files

* Use scala.jdk.javaapi.CollectionConverters for Java files

* Include scala-collection-compat

* Allow scala-collection-compat in JAR

* Exclude scala-collection-compat to avoid duplicated classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants