Skip to content

build: allow the whole DataFusion 54 line - #3033

Merged
CTTY merged 2 commits into
apache:mainfrom
plusplusjiajia:feat/relax-datafusion-req
Aug 25, 2026
Merged

CTTY merged 2 commits into
apache:mainfrom
plusplusjiajia:feat/relax-datafusion-req

Conversation

@plusplusjiajia

@plusplusjiajia plusplusjiajia commented Aug 20, 2026 •

Copy link
Copy Markdown
Member

What changes are included in this PR?

datafusion = "54.1.0" resolves to ^54.1.0, which excludes the 54.0.x line. Downstream projects that must stay on 54.0 therefore cannot depend on iceberg-datafusion at all.

The concrete case: the datafusion-ffi capsule layout is not stable across minor versions, so a project embedding DataFusion next to a datafusion-python host has to match its version — and PyPI's datafusion-python currently tops out at 54.0.

This widens the requirement to >=54.0.0, <55. The lockfile still resolves to 54.1.0, so CI builds are unchanged; only the range downstreams may resolve within gets wider. Verified that iceberg and iceberg-datafusion compile and pass their consumer's tests against DataFusion 54.0.0.

@plusplusjiajia
plusplusjiajia marked this pull request as ready for review August 20, 2026 15:26
@plusplusjiajia

Copy link
Copy Markdown
Member Author

@CTTY @xanderbailey could you take a look when you have a chance? The exact 54.1.0 requirement means any project still on DataFusion 54.0.x cannot depend on iceberg-datafusion at all. This widens it to >=54.0.0, <55; the lockfile still resolves 54.1.0, so CI builds are unchanged. Happy to follow a different pinning policy if the integration crates have one.

@xanderbailey xanderbailey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change makes sense to me. Still semver compatible

@dannycjones dannycjones left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change seems reasonable, although I'm wondering what we may lose.

@mbutrovich bumped us from .0 to .1 in #2872. What was the motivation - testing? If so, should we have instructions to bump the DataFusion dependency in the lock file only in future?

Comment thread Cargo.toml Outdated
Comment on lines +82 to +85
datafusion = "54.0.0"
datafusion-cli = "54.0.0"
datafusion-ffi = "54.0.0"
datafusion-sqllogictest = "54.0.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should switch to this, so we don't find ourselves in this situation again?

Suggested change
datafusion = "54.0.0"
datafusion-cli = "54.0.0"
datafusion-ffi = "54.0.0"
datafusion-sqllogictest = "54.0.0"
datafusion = "54"
datafusion-cli = "54"
datafusion-ffi = "54"
datafusion-sqllogictest = "54"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dannycjones Taking the suggestion, thanks — "54" says the same thing and leaves nothing to bump.
On #2872: it reads as a routine follow-the-release bump, covered by existing tests — nothing needed 54.1 specifically. So yes, lockfile-only seems right for those.

@plusplusjiajia plusplusjiajia changed the title build: allow the DataFusion 54.0.x line build: allow the whole DataFusion 54 line Aug 24, 2026

@CTTY CTTY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@CTTY
CTTY merged commit 1203f2c into apache:main Aug 25, 2026
21 checks passed
@dentiny

dentiny commented Aug 27, 2026

Copy link
Copy Markdown
Member

I'm curious if we should follow the same guide for other crates as well (aka, using major version, but not patch version)

@blackmwk

Copy link
Copy Markdown
Contributor

I'm curious if we should follow the same guide for other crates as well (aka, using major version, but not patch version)

+1. We should write them in AGENTS.md or add some ci check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants