Skip to content

[MINOR][CH] Drop Spark 3.2 / 3.1-era compat workarounds in ClickHouse backend#12541

Merged
jackylee-ch merged 2 commits into
apache:mainfrom
LuciferYang:gluten-minor-ch-spark32-refactor
Jul 17, 2026
Merged

[MINOR][CH] Drop Spark 3.2 / 3.1-era compat workarounds in ClickHouse backend#12541
jackylee-ch merged 2 commits into
apache:mainfrom
LuciferYang:gluten-minor-ch-spark32-refactor

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Small ClickHouse-side Spark 3.2 / 3.1-era workarounds that are no longer justified now that Gluten supports only Spark 3.3+:

ExtendedGeneratorNestedColumnAliasing (backends-clickhouse/.../ExtendedColumnPruning.scala)

Previously reimplemented NestedColumnAliasing.getAttributeToExtractValues locally because the Spark 3.2 version only accepted two arguments. Every currently supported branch (3.3, 3.4, 3.5, 4.0, 4.1) exposes the three-argument overload with the extractor parameter, so both call sites now delegate to upstream and the private reimplementation (collectRootReferenceAndExtractValue, getAttributeToExtractValues, totalFieldNum) is removed. The Gluten-specific collectNestedGetStructFields extractor is kept and the call site now has a comment explaining why it is needed.

HadoopMapReduceAdapter.isBucketWrite (backends-clickhouse/.../CHColumnarWrite.scala)

Previously used reflection to read WriteJobDescription.bucketSpec, guarding against Spark 3.2 where the field did not exist. bucketSpec is a public val on every currently supported branch, so the reflection helper is replaced with a direct description.bucketSpec.isDefined check.

FakeRowOutputWriter.path() (backends-clickhouse/.../FakeRowOutputWriter.scala)

FakeRowOutputWriter implements OutputWriter.path() without the override keyword, with a comment saying it is left off for Spark 3.1 compatibility. Spark's OutputWriter.path(): String is abstract on every currently supported branch, so override is legal there and now matches the sibling override def write(...) / override def close() in the same class.

No behavior change on any supported Spark version.

How was this patch tested?

  • ./build/mvn -Pbackends-clickhouse -Pspark-3.3 -Pdelta -pl backends-clickhouse -am test-compile -DskipTests: SUCCESS
  • ./dev/format-scala-code.sh: no diff
  • Cross-checked NestedColumnAliasing.getAttributeToExtractValues bodies on upstream/branch-3.3, branch-3.4, branch-3.5, branch-4.0, branch-4.1: three-argument overload present on every branch. The internal canAlias guard reshuffled between 3.4 and 3.5, but for the Gluten call site using collectNestedGetStructFields the guard is a semantic no-op (single-attribute chains, never traverses lambda variables).
  • Cross-checked WriteJobDescription.bucketSpec on the same five branches: val bucketSpec: Option[WriterBucketSpec] on all of them.
  • Cross-checked abstract class OutputWriter { def path(): String } on the same five branches: path() is abstract on all of them.
  • ClickHouse-specific runtime tests (GlutenClickHouseHiveTableSuite's "Nested column pruning for Project(Filter(Generate))" cases, GlutenClickHouseTPCHParquetBucketSuite, MergeTree write paths that use FakeRowOutputWriter) rely on native artifacts and Linux, so they run in CI rather than locally.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude claude-opus-4-7

Copilot AI review requested due to automatic review settings July 17, 2026 03:34
@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 two Spark 3.2 compatibility workarounds in the ClickHouse backend now that Gluten targets Spark 3.3+ only, simplifying the code by delegating to stable upstream APIs.

Changes:

  • Replace reflective access to WriteJobDescription.bucketSpec with a direct description.bucketSpec.isDefined check in CHColumnarWrite.
  • Remove the local reimplementation of NestedColumnAliasing.getAttributeToExtractValues and delegate to Spark’s upstream implementation.
  • Retain the Gluten-specific nested GetStructField extractor and document why it’s required at the call site.

Reviewed changes

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

File Description
backends-clickhouse/src/main/scala/org/apache/spark/sql/execution/CHColumnarWrite.scala Drops Spark 3.2-era reflection and uses bucketSpec directly for bucketed-write file pattern selection.
backends-clickhouse/src/main/scala/org/apache/gluten/extension/ExtendedColumnPruning.scala Removes duplicated Spark logic and relies on upstream NestedColumnAliasing.getAttributeToExtractValues, keeping the Gluten-specific extractor where needed.

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

@jackylee-ch

jackylee-ch commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

cc @zzcclp @taiyang-li

…nd CHColumnarWrite

Two Spark 3.2-era pieces no longer justified now that Gluten supports
only Spark 3.3+:

* `ExtendedGeneratorNestedColumnAliasing` used to reimplement
  `NestedColumnAliasing.getAttributeToExtractValues` locally because the
  Spark 3.2 version only took two arguments. Every currently supported
  branch (3.3, 3.4, 3.5, 4.0, 4.1) exposes the three-argument overload,
  so both call sites now delegate to upstream and the private
  reimplementation (`collectRootReferenceAndExtractValue`,
  `getAttributeToExtractValues`, `totalFieldNum`) is removed. The
  Gluten-specific `collectNestedGetStructFields` extractor is kept, and
  its call site now carries a comment explaining why it is needed.

* `HadoopMapReduceAdapter.isBucketWrite` used reflection to read
  `WriteJobDescription.bucketSpec`, guarding against Spark 3.2 where the
  field did not exist. `bucketSpec` is a public `val` on every currently
  supported branch, so the reflection helper is replaced with a direct
  `description.bucketSpec.isDefined` check.

No behavior change on any supported Spark version.
@LuciferYang
LuciferYang force-pushed the gluten-minor-ch-spark32-refactor branch from fa2d79c to f2b5e01 Compare July 17, 2026 05:19
@github-actions

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li 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

…Writer

`FakeRowOutputWriter` implements `OutputWriter.path()` without the
`override` keyword and carries a comment saying it is left off for
Spark 3.1 compatibility. Spark's `OutputWriter.path(): String` is
abstract on every currently supported branch (3.3, 3.4, 3.5, 4.0, 4.1),
so `override` is legal there and now matches the sibling
`override def write(...)` / `override def close()` in the same class.

No behavior change on any supported Spark version.
Copilot AI review requested due to automatic review settings July 17, 2026 06:38
@LuciferYang LuciferYang changed the title [MINOR][CH] Drop Spark 3.2-era workarounds in ExtendedColumnPruning and CHColumnarWrite [MINOR][CH] Drop Spark 3.2 / 3.1-era compat workarounds in ClickHouse backend Jul 17, 2026
@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

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

@jackylee-ch
jackylee-ch merged commit 23ed0c1 into apache:main Jul 17, 2026
7 checks passed
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.

4 participants