Skip to content

chore: remove the Spark 3.4 shim source sets #7560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andygrove wants to merge 6 commits into
base: main
Choose a base branch
from
+61 −1,400
Open

chore: remove the Spark 3.4 shim source sets #7560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1a5f85e
chore: remove Spark 3.4 Maven profiles and stop running Spark 3.4 CI
andygrove Nov 5, 2026
3819297
address review: keep Scala 2.12 coverage, drop remaining Spark 3.4 re…
andygrove Nov 5, 2026
8de08af
Merge remote-tracking branch 'apache/main' into maven-workflow-pr-224…
andygrove Nov 7, 2026
29efaa9
chore: remove the Spark 3.4 shim source sets
andygrove Nov 7, 2026
87c0c0c
Merge branch 'main' into remove-spark-3.4-shims
andygrove Nov 7, 2026
63c524a
style: apply spotless formatting to ShimCometMapInBatch
andygrove Nov 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions dev/ci/compute-changes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@
"spark/src/main/scala/org/apache/comet/vector/**",
"spark/src/main/scala/org/apache/spark/sql/comet/CometMapInBatchExec.scala",
"spark/src/main/scala/org/apache/spark/sql/comet/shims/MapInBatchInfo.scala",
"spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimCometMapInBatch.scala",
"spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimCometMapInBatch.scala",
"spark/src/main/spark-4.0/org/apache/spark/sql/comet/shims/ShimCometMapInBatch.scala",
"spark/src/main/spark-4.0/org/apache/spark/sql/execution/python/CometArrowPythonRunner.scala",
Expand Down Expand Up @@ -205,7 +204,6 @@
"common/src/main/**",
"common/pom.xml",
"spark/src/main/**",
"!spark/src/main/spark-3.4/**",
"!spark/src/main/spark-4.0/**",
"!spark/src/main/spark-4.1/**",
"!spark/src/main/spark-4.2/**",
Expand Down Expand Up @@ -233,7 +231,6 @@
"common/src/main/**",
"common/pom.xml",
"spark/src/main/**",
"!spark/src/main/spark-3.4/**",
"!spark/src/main/spark-3.5/**",
"!spark/src/main/spark-3.x/**",
"!spark/src/main/spark-4.1/**",
Expand Down Expand Up @@ -261,7 +258,6 @@
"common/src/main/**",
"common/pom.xml",
"spark/src/main/**",
"!spark/src/main/spark-3.4/**",
"!spark/src/main/spark-3.5/**",
"!spark/src/main/spark-3.x/**",
"!spark/src/main/spark-4.0/**",
Expand Down Expand Up @@ -289,7 +285,6 @@
"common/src/main/**",
"common/pom.xml",
"spark/src/main/**",
"!spark/src/main/spark-3.4/**",
"!spark/src/main/spark-3.5/**",
"!spark/src/main/spark-3.x/**",
"!spark/src/main/spark-4.0/**",
Expand Down
2 changes: 1 addition & 1 deletion docs/source/about/faq.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ for one platform only.

### Why do I get a `ClassNotFoundException` for `CometShuffleManager`?

Spark could not find the Comet jar when it created the shuffle manager. On Spark 3.4 and 3.5,
Spark could not find the Comet jar when it created the shuffle manager. On Spark 3.5,
executors create the shuffle manager before they load jars passed with `--jars` or `--packages`, so
the jar must be on the startup classpath. Either copy it into `$SPARK_HOME/jars` on every machine, or
set `spark.driver.extraClassPath` and `spark.executor.extraClassPath` to its path, as the
Expand Down
2 changes: 1 addition & 1 deletion docs/source/contributor-guide/adding_a_new_expression.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ In addition to `getSupportLevel`, which governs runtime planning decisions, the
- `getIncompatibleReasons(): Seq[String]` - Reasons the expression may produce different results than Spark.
- `getUnsupportedReasons(): Seq[String]` - Reasons the expression, or certain usages of it, may not be supported by Comet.

These methods do not affect runtime behavior. They are called by `GenerateDocs` (`spark/src/main/scala/org/apache/comet/GenerateDocs.scala`) when building the user-facing Compatibility Guide pages. The Markdown templates live in `docs/source/user-guide/latest/compatibility/expressions/_category_template/` (for example, `math.md`, `datetime.md`, `array.md`, `aggregate.md`, `struct.md`); `docs/build.sh` copies them into per-Spark-version subdirectories (`spark-3.4/`, `spark-3.5/`, `spark-4.0/`, `spark-4.1/`) and runs `GenerateDocs` once per profile, so the published page reflects the expressions registered for that Spark version. Each reason is rendered as a bullet in the corresponding page.
These methods do not affect runtime behavior. They are called by `GenerateDocs` (`spark/src/main/scala/org/apache/comet/GenerateDocs.scala`) when building the user-facing Compatibility Guide pages. The Markdown templates live in `docs/source/user-guide/latest/compatibility/expressions/_category_template/` (for example, `math.md`, `datetime.md`, `array.md`, `aggregate.md`, `struct.md`); `docs/build.sh` copies them into per-Spark-version subdirectories (`spark-3.5/`, `spark-4.0/`, `spark-4.1/`) and runs `GenerateDocs` once per profile, so the published page reflects the expressions registered for that Spark version. Each reason is rendered as a bullet in the corresponding page.

`GenerateDocs` also rewrites the **Implementation** column of every table in [`docs/source/user-guide/latest/expressions.md`](../user-guide/latest/expressions.md) based on the serde's trait mixins: `NativeOptInAvailable` (or its subtype `CodegenDispatchFallback`) is reported as `Hybrid`, a plain `CometCodegenDispatch` is reported as `Codegen dispatch`, and anything else is reported as `Native`. Rows whose function name is not registered in Spark's `FunctionRegistry` (or has no matching serde) get an em-dash placeholder.

Expand Down
3 changes: 1 addition & 2 deletions docs/source/contributor-guide/iceberg-spark-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ records one of three writers:
- `jvm`: Comet's split operator planned the write but kept Iceberg's JVM writer (`IcebergWriteExec`).
The line includes the reasons Comet recorded for not converting it.
- `spark`: Spark's own V2 write operator ran the write, so Comet's split operator never saw it. Examples
are `WriteToDataSourceV2` for a streaming micro-batch, and on Spark 3.4 both `WriteDelta` for
merge-on-read and the CTAS and RTAS execs, which write the table themselves.
are `WriteToDataSourceV2` for a streaming micro-batch.

`dev/ci/summarize-iceberg-writes.py` turns these records into a table on the job's summary page. It
shows the count and share of each writer, the most common fallback reasons, and the Spark write
Expand Down
20 changes: 10 additions & 10 deletions docs/source/contributor-guide/iceberg-writes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ IcebergCommit driver: collect task commit messages, BatchWrite.c
+- <input query> scans, projects, exchanges, sorts; inside AQE with or without the split
```

| Component | Location | Role |
| ------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `IcebergWriteStrategy` | `spark/src/main/scala/org/apache/comet/iceberg/` | Planner strategy. Matches `AppendData`, `OverwriteByExpression`, `OverwritePartitionsDynamic`, `ReplaceData` (and Iceberg's own `ReplaceIcebergData`, which Iceberg 1.5.2 plans on Spark 3.4), plus Spark 3.5+ Iceberg `WriteDelta` under the testing split flag. |
| `IcebergWriteLogical` | same | Logical anchor for the writer, so AQE re-plans re-emit only the writer and not a second committer. |
| `IcebergWriteExec` | `spark/src/main/scala/org/apache/spark/sql/comet/` | JVM writer. Runs iceberg-java's `DataWriter` or, for `WriteDelta`, `DeltaWriter` per task and returns the serialized `WriterCommitMessage` as one binary row. |
| `IcebergCommitExec` | same | Driver committer. A `V2CommandExec`, so `run()` is memoized and the commit happens once. |
| `IcebergReplaceDataShim`, `IcebergDeltaLogicalShim`, `IcebergDeltaWriterShim`, `IcebergRefreshCacheShim`, `IcebergDriverMetricsShim` | `spark/src/main/spark-*/org/apache/comet/iceberg/` | Version differences: operation-coded `ReplaceData` / `WriteDelta` rows, cache refresh by name on 4.1+, driver metric reporting. |
| Component | Location | Role |
| ------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `IcebergWriteStrategy` | `spark/src/main/scala/org/apache/comet/iceberg/` | Planner strategy. Matches `AppendData`, `OverwriteByExpression`, `OverwritePartitionsDynamic`, `ReplaceData` plus Iceberg `WriteDelta` under the testing split flag. |
| `IcebergWriteLogical` | same | Logical anchor for the writer, so AQE re-plans re-emit only the writer and not a second committer. |
| `IcebergWriteExec` | `spark/src/main/scala/org/apache/spark/sql/comet/` | JVM writer. Runs iceberg-java's `DataWriter` or, for `WriteDelta`, `DeltaWriter` per task and returns the serialized `WriterCommitMessage` as one binary row. |
| `IcebergCommitExec` | same | Driver committer. A `V2CommandExec`, so `run()` is memoized and the commit happens once. |
| `IcebergReplaceDataShim`, `IcebergDeltaLogicalShim`, `IcebergDeltaWriterShim`, `IcebergRefreshCacheShim`, `IcebergDriverMetricsShim` | `spark/src/main/spark-*/org/apache/comet/iceberg/` | Version differences: operation-coded `ReplaceData` / `WriteDelta` rows, cache refresh by name on 4.1+, driver metric reporting. |

Things to know before changing this layer:

Expand All @@ -97,12 +97,12 @@ Things to know before changing this layer:
- **Commit-coordinator writes are not intercepted.** Iceberg's current `SparkWrite` and
position-delta write do not ask for one; the checks in `buildTwoOp` and `buildDeltaTwoOp`
are defensive.
- **WriteDelta stays JVM-backed.** Spark 3.5+ merge-on-read commands are intercepted by the split
- **WriteDelta stays JVM-backed.** Spark 3.5 and later merge-on-read commands are intercepted by the split
plan only when the testing flag `spark.comet.write.iceberg.splitOperator.enabled` is on, and then
`IcebergWriteExec` delegates their rows to Iceberg's JVM `DeltaWriter`; the native Iceberg writer
is explicitly declined. Since the split plan gives such a write nothing over Spark's operator,
`spark.comet.write.iceberg.enabled` alone leaves it on Spark's plan, as does Spark 3.4.
- **What is not intercepted:** streaming writes and CTAS/RTAS on Spark 3.4. Those keep Spark's plan.
`spark.comet.write.iceberg.enabled` alone leaves it on Spark's plan.
- **What is not intercepted:** streaming writes. Those keep Spark's plan.

On Spark 4.1+, `IcebergWriteSummaryShim` finds either Spark's `MergeRowsExec` or
`CometMergeRowsExec` in the executed query and forwards its eight action counters to
Expand Down
65 changes: 26 additions & 39 deletions docs/source/contributor-guide/shim_pattern.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading