[CORE] Remaining Spark 3.3 cleanup after #12902
- Dominant language
- Scala
- Stars
- 1.6k
- Forks
- 657
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 80
Description
### Description
#12902 removed Spark 3.3 support, #12954 cleaned up the shim APIs that only existed to bridge it, and #12981 removed the version checks it left behind. A sweep of the tree at `810810ce9` turns up three categories that survived, plus one shim cleanup that #12954 could not have done at the time.
Most of what is left sits in `backends-bolt`. #12454 branched before #12954 and merged between #12954 and #12981 (`a44947e7e` → `53ad31ba2` → `2a68f3002`), so it reintroduced patterns the first cleanup had removed, and the second cleanup touched clickhouse, velox, gluten-substrait, gluten-core, gluten-iceberg and gluten-ut but not a single bolt file. #12999 repairs the resulting build break; this issue covers the rest. That no CI job builds `-Pbackends-bolt` is filed separately as #12998.
None of the items below break the build.
### 1. 95 version guards that are now constant
`testWithMinSparkVersion(name, v)` registers the test only when the runtime version is at least `v`. With the supported set closed at {3.4, 3.5, 4.0, 4.1}, a floor of 3.4 or below is always satisfied, so the call is a plain `test(...)` wearing a version check:
| floor | call sites |
|-|-|
| `"3.4"` | 84, across 20 files |
| `"3.3"` | 9, all in backends-bolt |
| `"3.2"` | 2, in backends-bolt |
95 in total, across 23 files. #12981 deliberately left the 3.4 group out of scope; the 3.3 and 3.2 groups arrived with #12454. Six more are the same shape written as an `if`: `isSparkVersionGE("3.4")` at `BoltParquetWriteForHiveSuite.scala:100,240,309,349,383` and `ArithmeticAnsiValidateSuite.scala:64`, where the `else` branch is unreachable.
### 2. Dead code
`BoltParquetWriteForHiveSuite.scala:409` registers a case through `testWithMaxSparkVersion(..., "3.3")`, so it runs on no supported version at all. It is not reported as skipped either, since the helper simply does not call `test()`. #12981 deleted the same case from the velox copy of that suite, so deleting it here follows a precedent rather than making a fresh judgement.
Unreachable branches: `BoltHashJoinSuite.scala:95,110` and `MiscOperatorSuite.scala:962` in bolt still branch on `startsWith("3.2")`, and `VeloxHashJoinSuite.scala:120` has a dead `startsWith("3.3.")` disjunct next to a live `startsWith("3.4.")` one.
Uncalled helpers: `GlutenQueryTest.scala:101` `ignoreWithSpecifiedSparkVersion` and `:113` `testWithRangeSparkVersion` have no call sites left.
Obsolete workaround: `CudfNodeValidationRule.scala:35` walks the plan with `find { ... }.isDefined` under a comment saying `Spark3.2 does not have exists`. `TreeNode.exists` is available on every supported version and is already used in `Validators.scala:281` and `ImplementJoinAggregate.scala:266`.
Dead files: `gluten-ut/spark33/` holds two bolt test-settings files and no `pom.xml`, and `backends-bolt/src/test/resources/tpch-approved-plan/*/spark33/` holds 84 approved-plan files. `gluten-ut/pom.xml` lists only `spark34/35/40/41`, so none of it is compiled, but `scala_code_format.yml` triggers on `gluten-ut/**` and still formats the two Scala files.
### 3. Comments that describe a version nobody builds
About 30, and each one costs the next reader a detour. The ones that mislead rather than merely age: `SparkDataSourceRDDPartition.scala:24` says the class exists "to make compatible with spark3.3 and before", when it differs from Spark's own class by carrying a `Seq[InputPartition]` and implementing `InputPartition`; the four copies of `WritableColumnVectorShim.java:25` name a 3.3 abstract-method addition as the reason the file is per-version, when the copies now split {3.4, 3.5} against {4.0, 4.1} for a different reason. The rest are `// Since spark 3.3.0` in `VeloxSparkPlanExecApi.scala:1469` and `CHSparkPlanExecApi.scala:1055`, about 15 in the gluten-ut settings files, and about 9 in clickhouse tests.
### 4. Two shim methods whose four implementations are now equivalent
`structFromAttributes` and `attributesFromStruct` are declared abstract at `SparkShims.scala:125,127`. spark34 spells the bodies out; spark35, spark40 and spark41 delegate to `DataTypeUtils`, whose implementation of both is spark34's body verbatim. So the four can collapse into concrete trait methods, and the trait body has to be spark34's form because `DataTypeUtils` is absent from the 3.4 catalyst jar. `invalidBucketFile` is a fourfold-duplicated `private def` that only needs `SparkException` and can move to `shims/common` alongside them.
Two that look collapsible and are not, worth recording so nobody retries: `createParquetFilters` has four byte-identical bodies but `LegacyBehaviorPolicy` lives under `SQLConf` in 3.4 and stands alone in 3.5+, so no single spelling compiles everywhere; `getShuffleBlockFetcherIterator` constructs a class that exists once per shim module, which `shims/common` cannot see.
More interesting than either: of the 27 abstract members, 14 are identical across 3.5, 4.0 and 4.1 with 3.4 as the sole outlier. Dropping 3.4 would collapse most of the trait, which is the direction the issue comment on #12807 was pointing at.
### Not included
The `Makefile` that arrived with #12454 has targets for profiles that no longer exist (`jar_spark33`, `-Pspark-3.3`, `-Pspark-3.2`, `-Pspark-3.2-las`). Whether to delete them or point them at a supported version is a bolt decision, so it goes on #12998 instead. `tools/workload/benchmark_velox/initialize.ipynb` installs Spark 3.3.1 as its cluster, which needs its own issue because someone has to pick the replacement version.
Documentation is clean: no doc claims 3.3 or earlier is supported, and no doc tells the reader to use a removed profile.
### Plan
Three PRs, none depending on the others: the 95 constant guards, the dead code and comments, and the two shim methods.
Contributor guide
Research direction
Start with the three-part plan, then inspect the named call sites in backends-bolt and the Spark shim files under shims; compare the 3.4–4.1 implementations before changing shared methods. Done means the constant guards, dead code, misleading comments, and equivalent shim methods are cleaned up across the listed files without removing the explicitly excluded Makefile or notebook work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, scala
- Domain
- backend, data-engineering
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100