You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/21 22:22:12 UTC
[PR] deps: bump the iceberg-rust pin to bb1e4a4 and document why it is pinned [datafusion-comet]
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] deps: bump the iceberg-rust pin to bb1e4a4 and document why it is pinned [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #6094:
URL: https://github.com/apache/datafusion-comet/pull/6094
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org