feat!: arrow 57, MSRV 1.85+ #1424

zachschuermann · 2025-10-23T18:45:28Z

What changes are proposed in this pull request?

add arrow-57 feature flag, remove arrow-55. also, since arrow 57 MSRV is 1.85 now, we bump from MSRV 1.84 to 1.85

This PR affects the following public APIs

arrow flag now defaults to arrow 57 instead of arrow 56, MSRV now rustc 1.85

How was this change tested?

existing

nicklan · 2025-10-23T18:56:30Z

kernel/src/arrow_compat.rs

-#[cfg(feature = "arrow-56")]
+#[cfg(feature = "arrow-57")]
+mod arrow_compat_shims {
+    pub use arrow_57 as arrow;


we should probably turn off these warnings if they are spurious

The compiler is usually pretty darn accurate. IMO we should figure out why it emitted the warnings instead of just assuming they're spurious.

hm maybe they disappeared now? sorry i don't know what messages this comment was linked to haha

kernel/Cargo.toml

kernel/src/arrow_compat.rs

hntd187

These were the exact changes I made, LGTM

scovich

Not sure this is quite ready for merge yet, see comments.

kernel/Cargo.toml

scovich · 2025-11-03T15:06:46Z

kernel/src/arrow_compat.rs

-#[cfg(feature = "arrow-56")]
+#[cfg(feature = "arrow-57")]
+mod arrow_compat_shims {
+    pub use arrow_57 as arrow;


The compiler is usually pretty darn accurate. IMO we should figure out why it emitted the warnings instead of just assuming they're spurious.

scovich · 2025-11-03T15:07:33Z

mem-test/tests/dhat_large_table_data.rs

+        .row_groups()
        .iter()
-        .map(|rg| rg.total_byte_size)
+        .map(|rg| rg.total_byte_size())


Did these accessor methods already exist in arrow-56, and arrow-57 just took the corresponding fields private? Or do we need conditional compilation here?

I don't think they existed - looks like they were just fields in parquet 56. But since this is in the separate (and internal) mem-test crate which selects its own version of arrow (57 now) this doesn't have to support both so I think we are good!

scovich · 2025-11-03T15:08:36Z

mem-test/Cargo.toml

 release = false

 [dependencies]
-arrow = "56"


This should change to "57"? I think the dependency was originally there to ensure we build mem-test with the latest available arrow version?

since kernel re-exports arrow I elected to remove this, and we just use the kernel arrow feature flag to pull in the latest arrow version kernel supports

Doesn't kernel pull in arrow-56 OR arrow-57 depending on the flags it was configured with, tho?

the arrow feature flag in kernel means "latest arrow" so was using that flag to get the latest arrow that we desire here

zachschuermann · 2025-11-03T15:26:28Z

Yep definitely not ready to merge haha. thanks for the reviews all! I did some exploration on this a while ago and didn't return to it since. I'll push an update shortly that removes 55

codecov · 2025-11-03T17:10:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.85%. Comparing base (81a8b7d) to head (eff31d2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1424   +/-   ##
=======================================
  Coverage   84.84%   84.85%           
=======================================
  Files         119      119           
  Lines       31031    31031           
  Branches    31031    31031           
=======================================
+ Hits        26329    26332    +3     
+ Misses       3425     3424    -1     
+ Partials     1277     1275    -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

scovich · 2025-11-04T15:48:12Z

ffi/src/transaction/mod.rs

        let res = writer.close().unwrap();

-        create_file_metadata(file_path, res.num_rows, metadata_schema)
+        create_file_metadata(file_path, res.file_metadata().num_rows(), metadata_schema)


Coming back to #1424 (comment) --

If these were just fields before, and now they're accessors (that didn't exist before), doesn't that mean this FFI code will fail to compile against arrow-56? The other example was hard-wired to arrow-57, but I thought FFI allowed either one?

yep correct this won't work for arrow-56 but FFI specifically takes a dependency on delta_kernel/arrow = latest version of arrow, so we directly switch to the 57 code here without issue

scovich · 2025-11-04T15:49:23Z

kernel/examples/read-table-multi-threaded/Cargo.toml

 common = { path = "../common" }
 delta_kernel = { path = "../../../kernel", features = [
-  "arrow-56",
+  "arrow",


We're intentionally pulling in whatever arrow the user configured, instead of forcing arrow-56 (which was latest)? Why do that, instead of forcing arrow-57 (which is now latest)?

(again below)

Update: I see... we just pull generic "arrow" from kernel (with the indirection it implies), but take actual arrow-57 for ourselves. I think that will lead to incompatible arrow versions in practice, as we pass record batches from arrow-56 kernel to arrow-57 example?

yep but I was actually using that as a desired behavior: this way, we pull in arrow-57 here in cargo.toml (which we need to do since we enable those other feature flags) and then we just say arrow feature flag for kernel. as we upgrade kernel, this example will break and force us to e.g. bump to 58 whenever we default to arrow 58 in the future. this is instead of the old way which would require us to remember to go update the two numbers (which admittedly isn't that bad but this felt more explicit)

So just to confirm (been a while, I'm forgetting): arrow brings in latest arrow version, unless the older version is also specifically requested?

scovich · 2025-11-04T15:51:32Z

kernel/src/checkpoint/tests.rs

 use crate::utils::test_utils::Action;
 use crate::{DeltaResult, FileMeta, LogPath, Snapshot};

-use arrow_56::{


huh, I wonder why it used to hard-wire the version like this?
(your change seems like the better way)

yea and actually this may highlight a test gap? i don't see how this could have previously compiled with arrow 55..?

scovich · 2025-11-04T15:53:08Z

mem-test/Cargo.toml

 release = false

 [dependencies]
-arrow = "56"


Doesn't kernel pull in arrow-56 OR arrow-57 depending on the flags it was configured with, tho?

zachschuermann requested review from DrakeLin, nicklan and scovich October 23, 2025 18:45

github-actions bot assigned zachschuermann Oct 23, 2025

nicklan removed the request for review from scovich October 23, 2025 18:46

github-actions bot added the breaking-change Change that require a major version bump label Oct 23, 2025

nicklan reviewed Oct 23, 2025

View reviewed changes

hntd187 approved these changes Nov 3, 2025

View reviewed changes

scovich reviewed Nov 3, 2025

View reviewed changes

zachschuermann force-pushed the arrow-57 branch from cf17bc9 to ed9f5ef Compare November 3, 2025 17:07

zachschuermann changed the title ~~feat!: arrow 57 support~~ feat!: arrow 57, MSRV 1.85+ Nov 3, 2025

zachschuermann force-pushed the arrow-57 branch 2 times, most recently from 058f892 to 01dfebe Compare November 3, 2025 17:18

zachschuermann requested review from nicklan and scovich November 3, 2025 17:18

zachschuermann force-pushed the arrow-57 branch from 01dfebe to 12bd863 Compare November 3, 2025 17:23

scovich reviewed Nov 4, 2025

View reviewed changes

zachschuermann added 2 commits November 4, 2025 10:47

arrow 57 support

58d9deb

bump msrv to 1.85

15a8e68

zachschuermann force-pushed the arrow-57 branch from 30eb955 to 15a8e68 Compare November 4, 2025 18:47

fix import

eff31d2

zachschuermann requested a review from scovich November 4, 2025 18:48

DrakeLin approved these changes Nov 5, 2025

View reviewed changes

OussamaSaoudi approved these changes Nov 5, 2025

View reviewed changes

lonless9 mentioned this pull request Nov 6, 2025

[Feature]: update to DataFusion 51 delta-io/delta-rs#3920

Open

3 tasks

feat!: arrow 57, MSRV 1.85+ #1424

Are you sure you want to change the base?

feat!: arrow 57, MSRV 1.85+ #1424

Conversation

zachschuermann commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

This PR affects the following public APIs

How was this change tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hntd187 left a comment

Choose a reason for hiding this comment

Uh oh!

scovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zachschuermann commented Nov 3, 2025

Uh oh!

codecov bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zachschuermann commented Oct 23, 2025 •

edited

Loading

codecov bot commented Nov 3, 2025 •

edited

Loading