Skip to content

[MINOR][CH] Remove residual Spark 3.2 branches from clickhouse tests#12532

Merged
zzcclp merged 1 commit into
apache:mainfrom
LuciferYang:gluten-minor-ch-spark32-cleanup
Jul 16, 2026
Merged

[MINOR][CH] Remove residual Spark 3.2 branches from clickhouse tests#12532
zzcclp merged 1 commit into
apache:mainfrom
LuciferYang:gluten-minor-ch-spark32-cleanup

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

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 [MINOR] Align supported Spark version references #12525.

Explicitly kept for a separate follow-up PR (real refactor, not comment fix):

  • 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.
  • backends-clickhouse/.../CHColumnarWrite.scala:157 — the bucketSpec reflection was needed for Spark 3.2, may be replaceable with direct access on 3.3+.
  • CustomSum.scala:28 — historical provenance of a copied file, not a version gate; keep as-is.

How was this patch tested?

  • mvn -pl backends-clickhouse -am install -Pspark-3.3,backends-clickhouse,delta: SUCCESS
  • mvn -pl backends-clickhouse scalastyle:check spotless:check -Pspark-3.3,backends-clickhouse,delta: SUCCESS
  • The pruned branches were dead code on every currently supported Spark profile, so runtime behavior is unchanged.

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).
Copilot AI review requested due to automatic review settings July 16, 2026 05:29
@github-actions

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes dead Spark 3.2-specific branches from the ClickHouse backend test suites (and one stale Spark 3.2-only comment in main code), simplifying tests now that Gluten supports Spark 3.3+ only.

Changes:

  • Removed the spark32 version flag from ClickHouse whole-stage test base and pruned all reachable spark32/!spark32 branches in ClickHouse tests to the Spark 3.3+ path.
  • Simplified ClickHouse MergeTree/Delta/TPCH/TPCDS test assertions and SQL DDL string generation (e.g., always emitting SORTED BY, always expecting the Spark 3.3+ plan shapes).
  • Cleaned up outdated Spark 3.2-only comments in both tests and a ClickHouse SparkPlan exec.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
backends-clickhouse/src/test/scala/org/apache/gluten/execution/GlutenClickHouseWholeStageTransformerSuite.scala Removes dead spark32 helper used for version gating in tests.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/GlutenClickHouseTPCHBucketSuite.scala Collapses Spark 3.2 conditionals to the Spark 3.3+ expectations and makes hasSortByCol unconditionally true.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/tpch/GlutenClickHouseTPCHParquetBucketSuite.scala Removes Spark 3.2 branching in physical-plan assertions (keeps Spark 3.3+ behavior only).
backends-clickhouse/src/test/scala/org/apache/gluten/execution/GlutenClickHouseDeltaParquetWriteSuite.scala Removes Spark 3.2 branching in optimize/vacuum-related assertions, keeping the Spark 3.3+ expected counts.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/tpcds/GlutenClickHouseTPCDSParquetAQESuite.scala Updates comments to describe only the Spark 3.3+ plan shape.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/tpcds/GlutenClickHouseTPCDSParquetColumnarShuffleAQESuite.scala Updates comments to describe only the Spark 3.3+ plan shape.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreeWriteSuite.scala Removes Spark 3.2 branches (e.g., always emits SORTED BY, keeps only Spark 3.3+ assertions).
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreeOptimizeSuite.scala Drops Spark 3.2 conditional assertions, keeping only Spark 3.3+ expected behaviors/counts.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreeWriteOnHDFSSuite.scala Removes Spark 3.2 conditional DDL/assertions; always uses Spark 3.3+ path.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreeWriteOnHDFSWithRocksDBMetaSuite.scala Removes Spark 3.2 conditional DDL/assertions; always uses Spark 3.3+ path.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreeWriteOnS3Suite.scala Removes Spark 3.2 conditional DDL/assertions; always uses Spark 3.3+ path.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/mergetree/GlutenClickHouseMergeTreePathBasedWriteSuite.scala Removes Spark 3.2 guard around LocalTableScan plan assertion, keeping Spark 3.3+ expectation.
backends-clickhouse/src/test/scala/org/apache/gluten/execution/hive/GlutenClickHouseNativeWriteTableSuite.scala Removes a stale Spark 3.2-specific comment.
backends-clickhouse/src/main/scala/org/apache/spark/sql/execution/RowToCHNativeColumnarExec.scala Removes an outdated “For spark 3.2” comment above withNewChildInternal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@LuciferYang

Copy link
Copy Markdown
Contributor Author

@zzcclp zzcclp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zzcclp
zzcclp merged commit 8226101 into apache:main Jul 16, 2026
5 checks passed
@LuciferYang
LuciferYang deleted the gluten-minor-ch-spark32-cleanup branch July 16, 2026 11:46
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you @zzcclp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants