-
Notifications
You must be signed in to change notification settings - Fork 344
feat: Update Datafusion to v49 #1704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
56ac1a4 to
8b07df4
Compare
8b07df4 to
2ac6349
Compare
kevinjqliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for working on the upgrade!
Datafusion 49 introduces a breaking change and is not backwards compatible with older versions.
More context: #1647 (comment)
This means pyiceberg-core will only work with datafusion 49 or newer.
I think we should add merge this into the next release 0.8.0 so that we can align with pyiceberg-core and pyiceberg
bindings/python/pyproject.toml
Outdated
|
|
||
| [tool.hatch.envs.dev] | ||
| dependencies = ["maturin>=1.0,<2.0", "pytest>=8.3.2", "datafusion==45.*", "pyiceberg[sql-sqlite,pyarrow]>=0.10.0", "fastavro>=1.11.1"] | ||
| dependencies = ["maturin>=1.0,<2.0", "pytest>=8.3.2", "datafusion==49.*", "pyiceberg[sql-sqlite,pyarrow]>=0.10.0", "fastavro>=1.11.1"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one major side effect is that upgrading to version 49 will make this backwards incompatible. Only other datafusion 49 can work with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the painpoint of doing major version upgrading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from what i heard, 49->50 is also backwards incompatible
kevinjqliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to prevent accidentally merging since we're still working on the 0.7.x release :)
Update datafusion-ffi Regenerate Cargo.lock Update to arrow 55.2 Update to parquet 55.2 Update rust_decimal to 1.37.2 Regenerate Cargo.lock Upgrade datafusion-python to 49
Fix test conflicts
91d7631 to
5046eb8
Compare
kevinjqliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! thanks for adding the "breaking" tag
|
@DerGut could you please rebase to fix the conflict? |
|
@DerGut I'm wonder if there're any updates for this PR 👀 |
Signed-off-by: Xuanwo <[email protected]>
Xuanwo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approve after CI
Which issue does this PR close?
Closes #1702
What changes are included in this PR?
Are these changes tested?