[MINOR][CORE] Remove residual Spark 3.2 compatibility code in gluten-core#12522
Merged
jackylee-ch merged 1 commit intoJul 16, 2026
Merged
Conversation
…core Spark 3.2 support was removed in prior PRs (apache#11351, apache#11687, apache#11731, apache#11887); currently supported versions are Spark 3.3, 3.4, 3.5, 4.0, 4.1. A few symbols that existed only as pre-Spark-3.3 shims are still around and dead code today. 1. `GlutenPlan.SupportsRowBasedCompatible` trait Introduced to provide `def supportsRowBased(): Boolean` for Spark < 3.3 where `SparkPlan.supportsRowBased` did not exist yet. The default body was `throw new GlutenException("Illegal state: The method is not expected to be called")`. On Spark 3.3+, `SparkPlan.supportsRowBased` is native and every concrete GlutenPlan / ColumnarInputAdapter overrides it directly, so the trait's default was already unreachable. Drop the trait and its two mixin sites. 2. `SparkVersionUtil.gteSpark33` Always true after Spark 3.2 was dropped. Also drop the single caller guard in `canPropagateConvention` (Transitions), which no longer needs to skip UnionExec on Spark 3.2. `eqSpark33` and `comparedWithSpark33` are kept: they distinguish Spark 3.3 from 3.4+ (different `TaskContextImpl` ctor signature and different write planning API), which is unrelated to the Spark 3.2 residual concern. 3. `SparkPlanUtil.supportsRowBased` reflection The reflection was needed on Spark 3.2 because `SparkPlan.supportsRowBased` did not exist as a member yet; the same compiled artifact ran on 3.2 and 3.3+ only by resolving the method reflectively at call time. Now that Spark 3.2 is dropped, a direct call `plan.supportsRowBased` compiles on all supported profiles and is strictly better (primitive Boolean instead of boxed, no per-call `getMethod` lookup, no `InvocationTargetException` wrapping). The 3 callers in `ConventionFunc` are on the planning hot path. Verified via compile on Spark 3.3, 3.5, and 4.1 (scala-2.13) profiles, plus gluten-core and gluten-substrait tests on Spark 3.5.
Contributor
Author
|
cc @jackylee-ch FYI，this is the first one and there are several more to follow. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes remaining Spark 3.2 compatibility shims and related dead code paths now that supported Spark versions start from 3.3.
Changes:
- Removed the
GlutenPlan.SupportsRowBasedCompatibleshim trait and its mixin sites. - Deleted
SparkVersionUtil.gteSpark33and simplifiedcanPropagateConventionby removing the no-longer-needed Spark 3.2 guard forUnionExec. - Replaced reflective
supportsRowBasedlookup with a directplan.supportsRowBasedcall.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| gluten-substrait/src/main/scala/org/apache/spark/sql/execution/ColumnarCollapseTransformStages.scala | Drops the legacy SupportsRowBasedCompatible mixin from ColumnarInputAdapter. |
| gluten-core/src/main/scala/org/apache/spark/util/SparkVersionUtil.scala | Removes gteSpark33 which is always true with Spark 3.2 dropped. |
| gluten-core/src/main/scala/org/apache/spark/util/SparkPlanUtil.scala | Switches supportsRowBased from reflection to direct API call. |
| gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/package.scala | Removes Spark-version gating and unused import; UnionExec is now always convention-propagatable. |
| gluten-core/src/main/scala/org/apache/gluten/execution/GlutenPlan.scala | Removes the Spark<3.3 shim trait and updates related Scaladoc/imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Run Gluten Clickhouse CI on x86 |
zml1206
approved these changes
Jul 15, 2026
zml1206
left a comment
zml1206
left a comment
Contributor
There was a problem hiding this comment.
lgtm, thanks, waiting for CI.
Contributor
|
Run Gluten Clickhouse CI on x86 |
Contributor
Author
|
Thank @jackylee-ch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Spark 3.2 support was removed in prior PRs (#11351, #11687, #11731, #11887); currently supported versions are Spark 3.3, 3.4, 3.5, 4.0, 4.1. A few symbols that existed only as pre-Spark-3.3 shims are still around and dead code today.
GlutenPlan.SupportsRowBasedCompatibletraitIntroduced to provide
def supportsRowBased(): Booleanfor Spark < 3.3 whereSparkPlan.supportsRowBaseddid not exist yet. The default body wasthrow new GlutenException("Illegal state: The method is not expected to be called"). On Spark 3.3+,SparkPlan.supportsRowBasedis native and every concreteGlutenPlan/ColumnarInputAdapteroverrides it directly, so the trait's default was already unreachable. Drop the trait and its two mixin sites.SparkVersionUtil.gteSpark33Always true after Spark 3.2 was dropped. Also drop the single caller guard in
canPropagateConvention(Transitions), which no longer needs to skipUnionExecon Spark 3.2.eqSpark33andcomparedWithSpark33are kept: they distinguish Spark 3.3 from 3.4+ (differentTaskContextImplctor signature and different write planning API), which is unrelated to the Spark 3.2 residual concern.SparkPlanUtil.supportsRowBasedreflectionThe reflection was needed on Spark 3.2 because
SparkPlan.supportsRowBaseddid not exist as a member yet; the same compiled artifact ran on 3.2 and 3.3+ only by resolving the method reflectively at call time. Now that Spark 3.2 is dropped, a direct callplan.supportsRowBasedcompiles on all supported profiles and is strictly better (primitiveBooleaninstead of boxed, no per-callgetMethodlookup, noInvocationTargetExceptionwrapping). The 3 callers inConventionFuncare on the planning hot path.How was this patch tested?
Verified via compile on Spark 3.3, 3.5, and 4.1 (scala-2.13) profiles, plus
gluten-coreandgluten-substraittests on Spark 3.5. All 27 + 45 unit tests pass.scalastyleandspotlessclean.