Repository navigation
[GLUTEN-11379][CORE] Clean up Spark shims APIs following Spark 3.2 deprecation - #11687
Conversation
|
Run Gluten Clickhouse CI on x86 |
594e5be to
a4ce1dd
Compare
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
a4ce1dd to
04a00ef
Compare
|
Run Gluten Clickhouse CI on x86 |
04a00ef to
ac2e71e
Compare
|
Run Gluten Clickhouse CI on x86 |
| SparkShimLoader.getSparkShims.dateTimestampFormatInReadIsDefaultValue(csvOptions, timeZone) | ||
| csvOptions.dateFormatInRead == default.dateFormatInRead && | ||
| csvOptions.timestampFormatInRead == default.timestampFormatInRead && | ||
| csvOptions.timestampNTZFormatInRead == default.timestampNTZFormatInRead |
There was a problem hiding this comment.
We need to confirm timestampNTZFormatInRead exists in Spark 3.3's CSVOptions
There was a problem hiding this comment.
Just confirmed in source code. It exists. And the compilation also help ensures this. Thanks.
| schema: MessageType, | ||
| caseSensitive: Option[Boolean] = None): ParquetFilters | ||
|
|
||
| def genDecimalRoundExpressionOutput(decimalType: DecimalType, toScale: Int): DecimalType = { |
There was a problem hiding this comment.
We need to ensure the base trait's default implementation handles all Spark versions correctly, or that the method is also removed from the SparkPlanExecApi trait
There was a problem hiding this comment.
The method here is identical with the one in SparkPlanExecApi, so I think this one is not required to be called for overriding the one in SparkPlanExecApi. Let's just remove this one. Thanks.
| import org.apache.gluten.vectorized.NativePartitioning | ||
|
|
||
| import org.apache.spark.{SparkConf, TaskContext} | ||
| import org.apache.spark.{ShuffleUtils, SparkConf, TaskContext} |
There was a problem hiding this comment.
This ShuffleUtils class should be in the shims (it exists in shims/spark34/ etc.). We need to confirm it's available for Spark 3.3 as well
There was a problem hiding this comment.
Yes, it also exists in shims/spark33.
QCLyu
left a comment
There was a problem hiding this comment.
Looks Good: correctly removed Spark shim indirections that were only needed for Spark 3.2 compatibility. Just left a few in-line comments for further confirmation.
zhouyuan
left a comment
There was a problem hiding this comment.
👍 Thanks. The shim layer looks cleaner now.
|
|
||
| def convertPartitionTransforms(partitions: Seq[Transform]): (Seq[String], Option[BucketSpec]) | ||
|
|
||
| def generateFileScanRDD( |
There was a problem hiding this comment.
We may better add a note on when/why these shim APIs are introduced for future changes
…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.
Spark 3.2 was dropped by apache#11351/apache#11687/apache#11731/apache#11887; currently supported versions per README.md / docs/index.md / pom.xml profiles are Spark 3.3, 3.4, 3.5, 4.0, and 4.1. Various docs, comments, and small code paths still carry Spark 3.2 leftovers or predate the 4.0/4.1 additions. This PR aligns them. Docs: - `docs/get-started/Velox.md`: version table + prose updated to 3.3.1, 3.4.4, 3.5.5, 4.0.2, 4.1.1 (matching `<spark.version>` in each profile). - `docs/velox-backend-limitations.md`: three section headers "For Spark3.2 and Spark3.3" reduced to "For Spark3.3"; the "spark3.2/3.3" runtime warning reduced to "spark3.3". - `docs/get-started/VeloxQAT.md`: replaced hardcoded old version list with "all supported Spark versions" since the script iterates `SUPPORTED_SPARK_VERSIONS`. - `docs/developers/HowToRelease.md`: removed the stale `spark-3.2.tar.gz` example line. - `tools/gluten-it/README.md`: profile list updated to spark-3.3, spark-3.4, spark-3.5, spark-4.0, spark-4.1. - `.github/ISSUE_TEMPLATE/bug.yml`: dropdown updated to Spark-3.3.x through Spark-4.1.x (removed Spark-3.2.x, added Spark-4.1.x, normalized case). Comments / small code: - `shims/spark33/.../OrcFileFormat.scala`: comment no longer references Spark 3.2 or a hypothetical shims-spark32. - `backends-velox/.../RowToVeloxColumnarExec.scala`: removed `// For spark 3.2.` above `withNewChildInternal`; the override is the standard Spark 3.3+ API. - `backends-velox/.../CudfNodeValidationRule.scala`: replaced `.find(_).isDefined` (used because Spark 3.2 lacked `TreeNode.exists`) with `.exists(_)`; dropped the stale comment. - `backends-velox/.../VeloxAggregateFunctionsSuite.scala`: dropped stale "Spark 3.2 does not have this configuration" comment. - `backends-velox/.../ArithmeticAnsiValidateSuite.scala`: comment narrowed from "Spark 3.2 and 3.3" to "Spark 3.3". - `tools/gluten-it/common/.../SparkJvmOptions.java`: dropped the Spark-3.2 `ClassNotFoundException` fallback that returned ""; consolidated all reflection exceptions into a single multi-catch.
Spark 3.2 was dropped by apache#11351/apache#11687/apache#11731/apache#11887; currently supported versions per README.md / docs/index.md / pom.xml profiles are Spark 3.3, 3.4, 3.5, 4.0, and 4.1. Various docs, comments, and small code paths still carry Spark 3.2 leftovers or predate the 4.0/4.1 additions. This PR aligns them. Docs: - `docs/get-started/Velox.md`: version table + prose updated to 3.3.1, 3.4.4, 3.5.5, 4.0.2, 4.1.1 (matching `<spark.version>` in each profile). - `docs/velox-backend-limitations.md`: three section headers "For Spark3.2 and Spark3.3" reduced to "For Spark3.3"; the "spark3.2/3.3" runtime warning reduced to "spark3.3". - `docs/get-started/VeloxQAT.md`: replaced hardcoded old version list with "all supported Spark versions" since the script iterates `SUPPORTED_SPARK_VERSIONS`. - `docs/developers/HowToRelease.md`: removed the stale `spark-3.2.tar.gz` example line. - `tools/gluten-it/README.md`: profile list updated to spark-3.3, spark-3.4, spark-3.5, spark-4.0, spark-4.1. - `.github/ISSUE_TEMPLATE/bug.yml`: dropdown updated to Spark-3.3.x through Spark-4.1.x (removed Spark-3.2.x, added Spark-4.1.x, normalized case). Comments / small code: - `shims/spark33/.../OrcFileFormat.scala`: comment no longer references Spark 3.2 or a hypothetical shims-spark32. - `backends-velox/.../RowToVeloxColumnarExec.scala`: removed `// For spark 3.2.` above `withNewChildInternal`; the override is the standard Spark 3.3+ API. - `backends-velox/.../CudfNodeValidationRule.scala`: replaced `.find(_).isDefined` (used because Spark 3.2 lacked `TreeNode.exists`) with `.exists(_)`; dropped the stale comment. - `backends-velox/.../VeloxAggregateFunctionsSuite.scala`: dropped stale "Spark 3.2 does not have this configuration" comment. - `backends-velox/.../ArithmeticAnsiValidateSuite.scala`: comment narrowed from "Spark 3.2 and 3.3" to "Spark 3.3". - `tools/gluten-it/common/.../SparkJvmOptions.java`: dropped the Spark-3.2 `ClassNotFoundException` fallback that returned ""; consolidated all reflection exceptions into a single multi-catch.
Spark 3.2 was dropped by apache#11351 / apache#11687 / apache#11731 / apache#11887; the `spark32` protected def in `GlutenClickHouseWholeStageTransformerSuite` was defined as `sparkVersion.equals("3.2")` and has been dead since. This PR removes the definition and prunes every reachable `if (spark32) ...` / `if (!spark32) ...` / `${if (spark32) ... else ...}` branch to keep only the Spark 3.3+ path. Test-code changes (all in `backends-clickhouse/src/test/`): - `GlutenClickHouseWholeStageTransformerSuite.scala`: drop the `protected def spark32` definition (always false). - `GlutenClickHouseTPCHBucketSuite.scala`: `hasSortByCol = !spark32` collapses to `true` on all supported Sparks; version-gated `if (spark32) ...` branches removed. - `GlutenClickHouseTPCHParquetBucketSuite.scala`, `GlutenClickHouseDeltaParquetWriteSuite.scala`, `GlutenClickHouseMergeTreeWriteSuite.scala`, `GlutenClickHouseMergeTreeOptimizeSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSWithRocksDBMetaSuite.scala`, `GlutenClickHouseMergeTreeWriteOnS3Suite.scala`, `GlutenClickHouseMergeTreePathBasedWriteSuite.scala`: every reachable `if (spark32) ... else ...` block, `if (!spark32) ...` guard, and inline `${if (spark32) "" else "SORTED BY (...)"}` interpolation is reduced to the Spark 3.3+ path (always emit `SORTED BY`). - `GlutenClickHouseTPCDSParquetAQESuite.scala`, `GlutenClickHouseTPCDSParquetColumnarShuffleAQESuite.scala`: comments narrowed from "On Spark 3.2, ... on Spark 3.3, ..." to describe only the surviving Spark 3.3+ shape. - `hive/GlutenClickHouseNativeWriteTableSuite.scala`: drop stale `// spark 3.2 without orc or parquet suffix` comment. Main-code change (one file): - `RowToCHNativeColumnarExec.scala`: drop the `// For spark 3.2.` comment above `withNewChildInternal`. The override is required by `TreeNode`'s API on every Spark version currently supported by Gluten, not a Spark 3.2-only quirk. Mirrors the same cleanup for `RowToVeloxColumnarExec` included in apache#12525. Explicitly kept for a separate follow-up PR: - `backends-clickhouse/.../ExtendedColumnPruning.scala:66-72` — the local `getAttributeToExtractValues` re-implementation exists because Spark 3.2's upstream signature was 2-arg. On 3.3+ it is 3-arg; the local copy could be replaced with a delegate. That is a real refactor, not a comment fix. - `backends-clickhouse/.../CHColumnarWrite.scala:157` — the `bucketSpec` reflection was needed for Spark 3.2, may be replaceable with direct access on 3.3+. Also a real refactor. - `CustomSum.scala:28` — historical provenance of a copied file, not a version gate; keep as-is. Verified with `mvn -pl backends-clickhouse -am install -Pspark-3.3, backends-clickhouse,delta` (SUCCESS) and `scalastyle:check spotless:check` (SUCCESS).
Spark 3.2 was dropped by apache#11351 / apache#11687 / apache#11731 / apache#11887; the `spark32` protected def in `GlutenClickHouseWholeStageTransformerSuite` was defined as `sparkVersion.equals("3.2")` and has been dead since. This PR removes the definition and prunes every reachable `if (spark32) ...` / `if (!spark32) ...` / `${if (spark32) ... else ...}` branch to keep only the Spark 3.3+ path. Test-code changes (all in `backends-clickhouse/src/test/`): - `GlutenClickHouseWholeStageTransformerSuite.scala`: drop the `protected def spark32` definition (always false). - `GlutenClickHouseTPCHBucketSuite.scala`: `hasSortByCol = !spark32` collapses to `true` on all supported Sparks; version-gated `if (spark32) ...` branches removed. - `GlutenClickHouseTPCHParquetBucketSuite.scala`, `GlutenClickHouseDeltaParquetWriteSuite.scala`, `GlutenClickHouseMergeTreeWriteSuite.scala`, `GlutenClickHouseMergeTreeOptimizeSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSWithRocksDBMetaSuite.scala`, `GlutenClickHouseMergeTreeWriteOnS3Suite.scala`, `GlutenClickHouseMergeTreePathBasedWriteSuite.scala`: every reachable `if (spark32) ... else ...` block, `if (!spark32) ...` guard, and inline `${if (spark32) "" else "SORTED BY (...)"}` interpolation is reduced to the Spark 3.3+ path (always emit `SORTED BY`). - `GlutenClickHouseTPCDSParquetAQESuite.scala`, `GlutenClickHouseTPCDSParquetColumnarShuffleAQESuite.scala`: comments narrowed from "On Spark 3.2, ... on Spark 3.3, ..." to describe only the surviving Spark 3.3+ shape. - `hive/GlutenClickHouseNativeWriteTableSuite.scala`: drop stale `// spark 3.2 without orc or parquet suffix` comment. Main-code change (one file): - `RowToCHNativeColumnarExec.scala`: drop the `// For spark 3.2.` comment above `withNewChildInternal`. The override is required by `TreeNode`'s API on every Spark version currently supported by Gluten, not a Spark 3.2-only quirk. Mirrors the same cleanup for `RowToVeloxColumnarExec` included in apache#12525. Explicitly kept for a separate follow-up PR: - `backends-clickhouse/.../ExtendedColumnPruning.scala:66-72` — the local `getAttributeToExtractValues` re-implementation exists because Spark 3.2's upstream signature was 2-arg. On 3.3+ it is 3-arg; the local copy could be replaced with a delegate. That is a real refactor, not a comment fix. - `backends-clickhouse/.../CHColumnarWrite.scala:157` — the `bucketSpec` reflection was needed for Spark 3.2, may be replaceable with direct access on 3.3+. Also a real refactor. - `CustomSum.scala:28` — historical provenance of a copied file, not a version gate; keep as-is. Verified with `mvn -pl backends-clickhouse -am install -Pspark-3.3, backends-clickhouse,delta` (SUCCESS) and `scalastyle:check spotless:check` (SUCCESS).
…12532) [MINOR][CH] Remove residual Spark 3.2 branches from clickhouse tests (#12532) Spark 3.2 was dropped by #11351 / #11687 / #11731 / #11887; the `spark32` protected def in `GlutenClickHouseWholeStageTransformerSuite` was defined as `sparkVersion.equals("3.2")` and has been dead since. This PR removes the definition and prunes every reachable `if (spark32) ...` / `if (!spark32) ...` / `${if (spark32) ... else ...}` branch to keep only the Spark 3.3+ path. Test-code changes (all in `backends-clickhouse/src/test/`): - `GlutenClickHouseWholeStageTransformerSuite.scala`: drop the `protected def spark32` definition (always false). - `GlutenClickHouseTPCHBucketSuite.scala`: `hasSortByCol = !spark32` collapses to `true` on all supported Sparks; version-gated `if (spark32) ...` branches removed. - `GlutenClickHouseTPCHParquetBucketSuite.scala`, `GlutenClickHouseDeltaParquetWriteSuite.scala`, `GlutenClickHouseMergeTreeWriteSuite.scala`, `GlutenClickHouseMergeTreeOptimizeSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSSuite.scala`, `GlutenClickHouseMergeTreeWriteOnHDFSWithRocksDBMetaSuite.scala`, `GlutenClickHouseMergeTreeWriteOnS3Suite.scala`, `GlutenClickHouseMergeTreePathBasedWriteSuite.scala`: every reachable `if (spark32) ... else ...` block, `if (!spark32) ...` guard, and inline `${if (spark32) "" else "SORTED BY (...)"}` interpolation is reduced to the Spark 3.3+ path (always emit `SORTED BY`). - `GlutenClickHouseTPCDSParquetAQESuite.scala`, `GlutenClickHouseTPCDSParquetColumnarShuffleAQESuite.scala`: comments narrowed from "On Spark 3.2, ... on Spark 3.3, ..." to describe only the surviving Spark 3.3+ shape. - `hive/GlutenClickHouseNativeWriteTableSuite.scala`: drop stale `// spark 3.2 without orc or parquet suffix` comment. Main-code change (one file): - `RowToCHNativeColumnarExec.scala`: drop the `// For spark 3.2.` comment above `withNewChildInternal`. The override is required by `TreeNode`'s API on every Spark version currently supported by Gluten, not a Spark 3.2-only quirk. Mirrors the same cleanup for `RowToVeloxColumnarExec` included in #12525. Explicitly kept for a separate follow-up PR: - `backends-clickhouse/.../ExtendedColumnPruning.scala:66-72` — the local `getAttributeToExtractValues` re-implementation exists because Spark 3.2's upstream signature was 2-arg. On 3.3+ it is 3-arg; the local copy could be replaced with a delegate. That is a real refactor, not a comment fix. - `backends-clickhouse/.../CHColumnarWrite.scala:157` — the `bucketSpec` reflection was needed for Spark 3.2, may be replaceable with direct access on 3.3+. Also a real refactor. - `CustomSum.scala:28` — historical provenance of a copied file, not a version gate; keep as-is. Verified with `mvn -pl backends-clickhouse -am install -Pspark-3.3, backends-clickhouse,delta` (SUCCESS) and `scalastyle:check spotless:check` (SUCCESS).
What changes are proposed in this pull request?
Since Spark 3.2 has been dropped, we need to clean up those shims APIs which were introduced to fix Spark code differences between Spark 3.2 and later versions. Then, the implementation for those APIs can be moved to the caller side.
How was this patch tested?
Local build.
Was this patch authored or co-authored using generative AI tooling?
No.
Related issue: #11379