LuciferYang opened a new issue, #13003:
URL: https://github.com/apache/gluten/issues/13003
### 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.
--
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]
