andygrove opened a new pull request, #6094:
URL: https://github.com/apache/datafusion-comet/pull/6094
## Which issue does this PR close?
Part of the native Iceberg writes epic, #5649.
This is the kind of periodic bump #5645 asks for, but it does not close it:
#5645 wants the
policy written down, and this PR only performs one bump and documents why
the dependency is a
git revision at all. Leaving #5645 open.
## Rationale for this change
`native/Cargo.toml` pins `iceberg` and `iceberg-storage-opendal` to
`665c64e4` (2026-09-03).
Upstream is now 51 commits ahead, and the pin has no note explaining itself,
so the reasonable
question "can we just use a release?" has to be re-derived from scratch each
time someone looks.
The answer is no, not yet: the latest published iceberg-rust is 0.10.1
(2026-08-01), which
requires arrow/parquet `^58`, while Comet is on 59.2. Those ranges are
disjoint, and since the
iceberg-rust writer API takes `arrow_array::RecordBatch` directly, a v58
`RecordBatch` is a
different type from the v59 one we hand it — a compile error, not just a
duplicate crate. The
currently pinned revision is in fact upstream's "deps: upgrade to DataFusion
55.0 and
Arrow/Parquet 59.2" commit, i.e. the pin has always been an arrow-alignment
pin rather than a
missing-API one. Every `iceberg::` path the native crate references already
exists in 0.10.1.
So the pin stays, but two things are worth doing now:
1. **Move it forward.** Several of the 51 commits land in the write path the
native writer
actually drives: apache/iceberg-rust#3159 removes a per-row partition-key
clone in the record
batch partition splitter, apache/iceberg-rust#3204 and
apache/iceberg-rust#3177 replace
`Vec<bool>` masks with `BooleanBuffer`, and apache/iceberg-rust#3171
stops cloning statistics
in `MinMaxColAggregator::update` (runs per column chunk). These are
upstream's own performance
claims; this PR does not measure them in Comet's context.
2. **Write down why the pin exists**, matching the comment convention
already used for the
`regex` pin a few lines above, so the next reader gets the arrow
constraint and the exit
condition without redoing the analysis.
Two things that look like reasons to bump but are not, checked and recorded
so they are not
re-litigated: the rustls 0.23.45 fix (apache/iceberg-rust#3244) does not
reach us — our lock
resolves rustls to 0.23.44 on both revisions — and the arrow-parquet group
bump
(apache/iceberg-rust#3209) is a no-op since we are already on parquet 59.3.0
either way. This
bump also does *not* unblock #5898; apache/iceberg-rust#3111 is still open.
## What changes are included in this PR?
- `native/Cargo.toml` / `native/Cargo.lock`: move both iceberg crates from
`665c64e4` to
`bb1e4a48` (2026-09-21). The lockfile change is confined to the three
iceberg entries — the
resolved graph is otherwise identical, same 615 crates with no version
changes on either side,
so there is no transitive churn to review.
- A three-line comment above the pin recording the arrow constraint, the
exit condition (a
release on arrow 59, at which point this becomes a plain version
requirement), and a pointer
to #5645.
No Comet code changes were needed, despite heavy upstream churn in modules
we import
(`scan/mod.rs` +442/-30, `scan/task.rs` +221/-50, `arrow/value.rs`,
`spec/partition.rs`,
`record_batch_partition_splitter.rs`).
## How are these changes tested?
By the existing suites; there is no new behavior to test.
Locally on the final state of the branch:
- `cargo check --workspace --all-targets` — clean, no errors or warnings.
- `cargo clippy --workspace --all-targets -- -D warnings` — clean.
- `cargo fmt --all -- --check` — clean.
- `cargo test -p datafusion-comet iceberg` — 74 passed, 0 failed. This
includes the parity tests
that would catch an upstream behavior change in the writer:
`iceberg_rust_transform_parity::{bucket,truncate,years_and_months}_agrees_with_iceberg_rust`,
`partition_spec_renders_like_iceberg_java`, the
`*_renders_like_iceberg_java` partition-path
cases, `encoded_manifest_round_trips_through_iceberg_parser`, the
rolling-grid cases, and
`test_metadata_field_id_constants_match_iceberg_rust`.
- `cargo test -p datafusion-comet --lib` — 430 passed, 0 failed.
The Rust tests are not sufficient sign-off on their own here: the native
scan's real coverage is
the JVM Iceberg suites, and `scan/mod.rs` and `scan/task.rs` changed
substantially upstream. I
have applied `run-iceberg-tests` so CI runs the Iceberg Spark SQL tests
against every Iceberg
version, per the bump policy sketched in #5645.
--
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]
