andygrove opened a new pull request, #5141:
URL: https://github.com/apache/datafusion-comet/pull/5141
## Which issue does this PR close?
No issue filed — this is a mechanical compiler-warning cleanup. Happy to
file one if the project would prefer to track it.
## Rationale for this change
Building Comet produces a large number of Scala compiler warnings, which
makes it easy to miss a genuinely new warning introduced by a change. Most of
them are Scala 2.13 deprecations that have accumulated since the 2.13 / Spark
4.x profiles were added.
| Profile | Before | After |
|---|---|---|
| Spark 4.1 / Scala 2.13 (default) | 127 | **8** |
| Spark 3.4 / Scala 2.12 | 2 | **1** |
| Spark 3.5 / Scala 2.12 | 15 | **1** |
| Spark 3.5 / Scala 2.13 | 127 | **8** |
| Spark 4.0 / Scala 2.13 | 127 | **8** |
## What changes are included in this PR?
Mostly mechanical 2.13 deprecation fixes:
- Symbol literals (`'col`) replaced with `$"col"` in `TPCH.scala` and
`ParquetReadSuite` (63 warnings).
- `.toIterable.asJava` replaced with `.toSeq.asJava` when handing
collections to protobuf `addAll*` builders.
- Implicit `Array` → `Seq` conversions (which silently copy) made explicit
with `.toIndexedSeq`.
- Auto-application to `()` given explicit empty argument lists
(`SparkSession.builder()`, `Random.nextLong()`, `.count()`, …).
- `Long + String` → string interpolation; `filterKeys` → `filter`;
`toIterator` → `iterator`; `hasNext()` → an `override def hasNext` that matches
`Iterator`.
- Two inferred existential types replaced with explicit types.
- `Float.NaN.equals(...)` → `getFloat(0).isNaN`.
Two changes are slightly more than mechanical:
- **`GenerateDocs.categoryPages`** triggered nine *"adapting argument list
by creating a 2-tuple"* warnings on 2.12, because `"array" -> ("array.md", ()
=> ...)` parses as a two-argument call to `->`. Introducing
`exprPage`/`aggPage` helpers removes both the ambiguity and ~40 lines of
duplication. The generated docs are byte-identical.
- **`FuzzDataGenerator`** no longer uses the deprecated
`RandomStringUtils.randomAlphabetic`. The replacement draws from the caller's
seeded `Random`, so generated strings are now reproducible from the fuzz seed —
previously they were not, since `RandomStringUtils` uses its own RNG.
`-processing` is added to the javac `-Xlint` list, silencing two *"No
processor claimed any of these annotations"* warnings.
### Warnings deliberately left in place
Each of these needs a shim or a larger change, so they felt out of scope
here:
1. **`AliasAwareOutputExpression` (4 warnings)** — uses `Stream`, deprecated
in 2.13. Can't be fixed without a shim: `multiTransformDown` returns `Stream`
on Spark 3.x / 2.12 and `LazyList` on Spark 4.x / 2.13, and `LazyList` doesn't
exist in 2.12. Worth flagging separately: **this trait appears to be dead
code** — `AliasAwareOutputExpression`, `projectExpression`, and
`generateCartesianProduct` have no references anywhere in the repo, and the
file hasn't been touched since #581 (Spark 3.2 drop). Deleting it would remove
all 4 warnings; I left that decision to reviewers.
2. **`CometSQLQueryTestHelper.getErrorClass` (2)** — renamed to
`getCondition` in Spark 4.0, so it needs a shim.
3. **`CometTestBase.withRowGroupSize` (1)** — parquet-version-specific
overload; already carries a TODO saying it needs a shim.
4. **`CometTPCDSQuerySuite` early initializer (1)** — the `extends { val
tpcdsAllQueries = … }` block exists so that `TPCDSBase`'s constructor sees the
query lists. Moving them to an `override val` in the class body would leave the
parent reading `null` during construction.
One warning was intentionally not "fixed" as the compiler suggests:
`CometFuzzTestSuite` keeps `.equals` rather than `==` when comparing default
values, because `==` on boxed floats goes through cooperative equality where
`NaN != NaN`, while `Float.equals` compares bits. It is restructured to compare
statically typed `AnyRef`s so the lint no longer fires, with a comment
explaining why.
## How are these changes tested?
No new tests — this is a warning cleanup with no intended behavior change.
- All five profiles build cleanly: default (Spark 4.1 / 2.13),
`-Pspark-3.4`, `-Pspark-3.5`, `-Pspark-3.5 -Pscala-2.13`, `-Pspark-4.0`.
- `spotless:check` and `scalastyle:check` pass.
- `GenerateDocs` output is unchanged (no diff under `docs/` after a full
`package`), confirming the `categoryPages` refactor is output-identical.
- Existing suites covering the touched code paths pass — 502 tests:
`CometFuzzTestSuite`, `DataGeneratorSuite`, `CometTemporalExpressionSuite`,
`CometNativeShuffleSuite`, `CometShuffleSuite`, `CometWindowExecSuite`,
`CometNativeReaderSuite`, `ParquetReadV1Suite`,
`CometIcebergRewriteActionSuite`, `CometExecSuite`.
--
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]
