apache / apache/datafusion-comet

Backport candidates for 1.0.x: triage of every PR merged since branch-1.0 was cut

Open
#5,815 0 comments 0 reactions 1 assignee Claimed by @andygrove View on GitHub
requires-triage
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

## Describe the problem

`branch-1.0` was cut at `af534e0f` (2026-08-03). Since then, 192 commits have landed on `main` and only three
backport batches have gone to `branch-1.0` (#5261, #5321, #5480). I went through every one of those 192
commits to work out which are genuine backport candidates for a 1.0.x patch release — pure bug fixes and
performance improvements that neither add features nor change intended behaviour — and which are not, and why.

This issue is the resulting list. It is a triage record, not a decision: the recommendations still need
someone to agree with them and open the backport PRs.

### How the list was produced

- Range: `git log af534e0f..main`, 192 commits.
- 85 carry a `fix:` or `perf:` prefix. I also checked all 47 `feat:` / `refactor:` / `deps:` commits for ones
that close a `bug`-labelled issue, which is how #5602 ended up in tier 1 despite its `feat:` prefix.
- Prefixes are not trustworthy on their own in either direction, so every candidate was checked against the
label on the issue it closes. Five `fix:`-prefixed PRs close `enhancement` issues and are excluded below.
- Applicability was checked mechanically: each commit was cherry-picked onto a pristine `branch-1.0` worktree,
and every conflicting path was tested for existence on `branch-1.0`. A conflict in a file that does not
exist on `branch-1.0` means the PR fixes code that only exists on `main` — not a candidate. A conflict in a
file that does exist is ordinary context drift and still a candidate.
- `branch-1.0` is on DataFusion 54.1.0 / Arrow 58.4.0; `main` is on 55.0.0 / 59.2.0 since #5262. That rules
out at least one otherwise-attractive fix.

Applying the recommended set as a stack in merge order, 27 of 37 commits cherry-pick clean and 10 need small
manual resolution. Cherry-picking clean is not the same as compiling — #5461 is a known example — so each
backport PR still needs `cargo check --workspace --all-targets` plus `./mvnw test-compile` before it goes up.

## Tier 1 — correctness, crashes and hangs

These change observable results, fail tasks, leak, or hang. I would take all of these.

| PR | What it fixes | Why it matters on 1.0.x | Cherry-pick |
| --- | --- | --- | --- |
| #5602 | Unicode case-insensitive Parquet field matching | Closes #5495 — `bug`, `correctness`, `priority:critical`, and already in the 1.0.1 milestone. Non-ASCII case pairs silently read as NULL | clean |
| #5623 | Codegen dispatcher null short-circuit skipped when a foldable subtree can raise | Closes #5608 — `bug`, `correctness`, `priority:high`. Swallows ANSI errors | clean |
| #5159 | `collect_list` / `collect_set` argument coercion made a normalization barrier | Closes #5158 — `bug`, `priority:high`. Grouped aggregate fails validating its own output batch | clean |
| #5757 | Dictionary hash fast path kept off nested and reseeded buffers | Dictionary-encoded values hashed differently from identical decoded values — wrong join/aggregate results and wrong shuffle partitioning | clean |
| #5754 | Parent struct's null mask applied before hashing its fields | Wrong answers from `hash` / `xxhash64` on a null struct; the same decision feeds shuffle partitioning | apply after #5757 |
| #5684 | Correctly rounded decimal to double/float cast | Last-ulp wrong answers for essentially every `DECIMAL(38,18)` value above 0.01 | clean |
| #5683 | ANSI bound check for float/double to integral casts | Spurious `CAST_OVERFLOW` for exactly representable `Int.MaxValue` / `Long.MaxValue` | clean |
| #5752 | ANSI errors preserved for rejected `TIMESTAMP_NTZ` casts | Closes #5749 — `bug`, `correctness`. Returns NULL where Spark raises `CAST_INVALID_INPUT` | clean |
| #5177 | Checked `TIMESTAMP_MILLIS` conversion in Parquet reads | Silently wraps overflowing values; Spark throws regardless of ANSI mode | rebase (`parquet_support.rs`) |
| #5740 | Same check for nested `TIMESTAMP_MILLIS` in unfiltered scans | Companion to #5177; take the two together | rebase (`parquet_support.rs`) |
| #5680 | `rpad` / `lpad` return NULL for a NULL length instead of panicking | Enabled by default; a NULL in the length column fails the task with `CometNativeException` | clean |
| #5630 | Map offsets rebased in `mapsort` | A sliced map fails the task with "Max offset exceeds length of entries" | rebase (test file only) |
| #5679 | `concat_ws` with array arguments falls back instead of failing natively | Query fails natively where Spark returns a result | clean |
| #5357 | Spark type names in ANSI `abs` overflow errors | Reports `Int64 overflow` where Spark reports `long overflow` | clean |
| #5747 | `copy_array` propagates offset-overflow errors instead of panicking | Closes #5706 — `bug`. Panic instead of a clean error | rebase (`copy.rs`) |
| #5539 | Arrow vector import rollback on failure | Closes #5534 — `bug`, `correctness`, `priority:medium`. Leaks native buffers, adding allocator pressure while the query is already failing | rebase (test file only) |
| #5748 | Tokio runtime threads attached to the JVM as daemon threads | The JVM hangs forever if an app returns from `main` without calling `spark.stop()`. #4734, which is on `branch-1.0`, does not cover this case | rebase (CI yml only) |
| #5561 | Spark 4.2 Python worker configuration | `branch-1.0` ships a `spark-4.2` profile and writes `workerConf` inside `writeCommand`, so accelerated `mapInArrow` / `mapInPandas` fails outright on 4.2 | clean |
| #5759 | Iceberg tables partitioned by an unknown transform | Read fails with `Invalid Iceberg scan task`; this is Iceberg's forward-compatibility contract, and it is what turned Iceberg CI red on `main` | rebase |
| #5493 | `CometDiskBlockWriter` spill registry made per-task | The registry is `static`, so task A force-spills task B's buffered bytes from A's thread and charges them to A | clean |
| #5494 | Task-shared memory pool as a ref-counted RAII guard | A stranded entry holds a JNI global ref to `CometTaskMemoryManager`, pinning `TaskMemoryManager` and `TaskContext` for the executor's lifetime. Nothing prunes the map | rebase (CI yml, `jni_api.rs`) |
| #5461 | Native shuffle reservation released after spill failure | Reservation, pinned buffers and repeated-spill bytes leak after a failed partition write | clean, but does **not** compile as-is: its inline test needs metric fields from #5370, so the test needs adapting |

## Tier 2 — performance, no behaviour change

| PR | What it does | Cherry-pick |
| --- | --- | --- |
| #4885 | `spark_base64`: 11–21% faster, bit-identical output | clean |
| #4911 | `spark_floor`: up to 4x faster on decimals | clean |
| #4939 | Vectorized integer-to-decimal cast — the second most frequent cast in TPC-DS | rebase (bench registration in `Cargo.toml`) |
| #5233 | `spark_size` list lengths via Arrow's `length` kernel | clean |
| #5395 | `spark_size` Map via the offset buffer; follow-up to #5233, take them together | after #5233 |
| #5046 | `ArrowWriter` preallocates fixed-width vectors from the known batch size | clean |
| #5442 | `ArrowWriter` bulk-copies fixed-width columns; follow-up to #5046, take them together | after #5046 |
| #5440 | Arrow cast for the decimal rescale check, replacing duplicated arithmetic | clean |
| #5735 | Arrow cast for date to `TIMESTAMP_NTZ` | rebase (`temporal.rs`) |
| #5737 | `posexplode` array expression evaluated once per batch instead of twice | clean |
| #5222 | Iceberg reflection lookups cached on the planning path — currently O(files) `Class.getMethod` per scan, and on the Spark 3.4 profile (Iceberg < 1.7) every call constructs a `NoSuchMethodException` | clean |
| #5568 | Per-partition scratch reused in the shuffle write path | rebase |
| #5038 | Arrow IPC compression context reused across shuffle blocks | clean |

Two caveats in this tier. #5038's measured benefit is narrower than it looks: the follow-up measurement in
#5727 shows 2.0–16.8% faster block encoding for 128-row batches and approximately no change for full-size
compressed batches. And #5368 (serialize Python input directly from Comet Arrow vectors) is the largest win
available here, but it is 10 files, it regressed timezone aliases, and the fix for that (#5556) is a separate
PR — take both or neither.

## Tier 3 — needs a decision before it can be classified

- **#5558** (`array_join` null short-circuiting) is a real correctness fix — Spark returns NULL whenever
`nullReplacement` is NULL, Comet returned the joined string — but it is bundled with flipping `array_join`
from `Incompatible` to natively enabled. Backporting as-is changes a compatibility flag in a patch release.
Worth splitting.
- **#5469** (normalize scalar float sort and window rank keys) is half applicable. The sort-key normalization
for signed zeros and NaN is a genuine correctness fix that applies to `branch-1.0`; the window-rank half
follows `WindowGroupLimitExec` (#4870), which is `main`-only. Needs splitting.
- **#5362** ("make `CometExplodeExec` respect batch size") is a real bug — `UnnestExec` never consults
`datafusion.execution.batch_size`, so an 8192-row batch of 100-element arrays comes back as one
819,200-row batch and peak memory scales with it. But the fix lands a new 1,491-line native `ExplodeExec`.
That is feature-sized for a patch release.
- **#5726** (pathless spill backend writes an index claiming bytes that never landed) would be a corrupt
shuffle file. On `branch-1.0` the only spill backend is local and always has a path, so as far as I can
tell the bug is latent there. Backport only if we want the invariant rather than the fix.
- **#5653** bundles three unrelated core fixes. One of them is genuinely user-visible — `Configuration#get`
expands `${...}` references and `Configuration.Entry#getValue` does not, so an object store option composed
from another property reached native unexpanded. Extract that hunk rather than cherry-picking the PR.
- **#5464** (RAII for tracing memory pool registration) is a real leak, but only reachable with native tracing
enabled. Low value for a patch release.
- **#5552** (preserve Arrow `Field` metadata across C Data exports) closes an `enhancement` (#5547) and is
part one of the Variant series. Nothing on `branch-1.0` consumes `Field` metadata, so there is no
observable effect there.

## Excluded, with reasons

Recording these so they do not get re-litigated for the next patch release.

**Fixes code that only exists on `main`.** The feature landed after the branch cut, so the fix has nothing to
apply to:

- Celeborn / RSS shuffle (features #5473, #5476, #5481, #5491, #5501, #5513, #5531, #5537): fixes #5475,
#5627, #5650, #5665, #5668, #5722, #5727
- Native Iceberg V2 writer (#5361) and the split writer/committer work (#4658, #5298): fixes #5410, #5651,
#5696, #5729, #5779
- Iceberg system functions (#5638): fix #5773
- Experimental native in-memory cache (#5051): fixes #5733, #5734
- Native `ExplodeExec` (#5362): fix #5667
- `CreateArray` map literal (#5452): fix #5766 — #5544 only becomes reachable once a folded map literal is
admitted natively

**Blocked on the dependency upgrade.** #5041 (enable FIRST/LAST partial merge) depends on the root fix in
DataFusion 55.0.0, which reached `main` via #5262. `branch-1.0` is on 54.1.0.

**Spark UI metrics accuracy only** — same call as for 1.0.1, these do not change query results or stability:
#5370, #5398, #5408, #5445, #5463, #5497, #5516, #5554, #5628.

**`fix:` prefix but closes an `enhancement` issue:** #5045 (#3389), #5225 (#5190), #5234 (#5096), #5359
(#5074), #5412 (#5258).

**Test, benchmark or CI only:** #5388, #5439, #5718, #5727, plus every `test:`, `chore:`, `chore(deps)`,
`docs:` and `ci:` commit in the range.

**Previously decided.** #5369 (preserve Catalyst nullability and field IDs in native Parquet writes) is a real
bug, but `spark.comet.parquet.write.enabled` defaults to `false` and is documented as experimental on
`branch-1.0`.

**Already on `branch-1.0`** via #5261, #5321 and #5480: #5138, #5162, #5167, #5185, #5192, #5282, #5315,
#5364, #5376, #5392, #5400, #5443.

## Additional context

Related open 1.0.1 items with no fix on `main` to backport: #5366 (`to_time` / `try_to_time` still rejects
`'T12'` and `'12:30:45.'`; #5364 fixed only the whitespace half) and #4412 (`CometHashAggregateExec` and
`AQEPropagateEmptyRelation`).

Optional companion: #5769 refactors #5754 and #5757 into a shared helper. Not required, but it keeps
`branch-1.0` closer to `main`'s shape if we want that.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.