andygrove opened a new issue, #5815:
URL: https://github.com/apache/datafusion-comet/issues/5815
## 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
