Skip to content
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

Release DataFusion 45.0.0 #14008

Closed
24 of 32 tasks
alamb opened this issue Jan 4, 2025 · 53 comments
Closed
24 of 32 tasks

Release DataFusion 45.0.0 #14008

alamb opened this issue Jan 4, 2025 · 53 comments
Assignees
Labels
enhancement New feature or request

Comments

@alamb
Copy link
Contributor

alamb commented Jan 4, 2025

Is your feature request related to a problem or challenge?

Tracking ticket for next release, also a place to track desired inclusions

Last release was https://crates.io/crates/datafusion/440.0 December 31, 2024 so next major release would be around Feb 1, 2025

Steps:

Pre-relese testing

Prior release tickets:

Please let me know if you would like to add any items on this list or move the categorization

Items to fix before release

Items maybe to complete (not sure if they are blockers)

Nice to Have (but non blockers -- e.g. bugs but not regressions)

@alamb alamb added the enhancement New feature or request label Jan 4, 2025
@alamb alamb mentioned this issue Jan 4, 2025
10 tasks
@alamb
Copy link
Contributor Author

alamb commented Jan 13, 2025

@andygrove would you like to coordinate this release or would you like me to? (or does anyone else want to do so?)

@alamb
Copy link
Contributor Author

alamb commented Jan 13, 2025

I also added some issues to the description above that I think would be worth fixing

@andygrove
Copy link
Member

@andygrove would you like to coordinate this release or would you like me to? (or does anyone else want to do so?)

I don't have a preference. I will traveling around this time though, so perhaps it would make sense for someone else to be release manager for this one.

@alamb
Copy link
Contributor Author

alamb commented Jan 13, 2025

I don't have a preference. I will traveling around this time though, so perhaps it would make sense for someone else to be release manager for this one.

I am happy to do it again for 45 if no one else would like the opportunity (see what I did there 😆 )

@xudong963
Copy link
Member

xudong963 commented Jan 14, 2025

I don't have a preference. I will traveling around this time though, so perhaps it would make sense for someone else to be release manager for this one.

I am happy to do it again for 45 if no one else would like the opportunity (see what I did there 😆 )

Thanks, alamb, I booked 46 in advance!

@alamb
Copy link
Contributor Author

alamb commented Jan 14, 2025

I am happy to do it again for 45 if no one else would like the opportunity (see what I did there 😆 )

Thanks, alamb, I booked 46 in advance!

Awesome -- I filed #14123 to track 46

@alamb
Copy link
Contributor Author

alamb commented Jan 15, 2025

I plan to start assembing the release candidate and test on the week of Jan 27 (in about 2 weeks time()

@shehabgamin
Copy link
Contributor

As promised, Sail is working on porting relevant tests into DataFusion.

A good starting point is a regression our tests caught in DataFusion 43, which still seems to persist in DataFusion 44. A regression was introduced in DataFusion 43.0.0 related to casting to UTF8 in various places. Upgrading to DataFusion 43.0.0 required adding explicit casting in several areas as a workaround. This PR (lakehq/sail#355) comments out those changes to expose the regression through the 12 additional failed tests compared to the main branch.

Once I’ve pinpointed the root cause(s) of the regression, I’ll create an issue in DataFusion to track the work. I want to ensure the issue accurately reflects the problem before filing it. I’m happy to address these regressions and port over the tests that cover them in the same PR. Hopefully, we can get this resolved in time for the DataFusion 45 release!

@alamb
Copy link
Contributor Author

alamb commented Jan 18, 2025

Once I’ve pinpointed the root cause(s) of the regression, I’ll create an issue in DataFusion to track the work. I want to ensure the issue accurately reflects the problem before filing it. I’m happy to address these regressions and port over the tests that cover them in the same PR. Hopefully, we can get this resolved in time for the DataFusion 45 release!

Thank you very much @shehabgamin 🙏

I strongly suspect this is related to switching to Utf8View by default in Parquet; You can validate this theory by disabling the following config setting:

https://datafusion.apache.org/user-guide/configs.html

datafusion.execution.parquet.schema_force_view_types

I think we are pretty close to closing out the Utf8View epic (now that we have upgraded to the latest arrow):

I'll add that to the list for 45 too

@alamb
Copy link
Contributor Author

alamb commented Jan 18, 2025

I plan to start preparing / testing / pushing this release the week of Jan 27, aiming to get an release candidate early the next week

@shehabgamin
Copy link
Contributor

I strongly suspect this is related to switching to Utf8View by default in Parquet; You can validate this theory by disabling the following config setting:

https://datafusion.apache.org/user-guide/configs.html

datafusion.execution.parquet.schema_force_view_types

I think we are pretty close to closing out the Utf8View epic (now that we have upgraded to the latest arrow):

* [[Epic] A Collection of Additional UTF8View support tickets #13504](https://github.com/apache/datafusion/issues/13504)

Thanks for the pointer @alamb!

I tried setting datafusion.execution.parquet.schema_force_view_types to false, but unfortunately, none of the 12 failed tests passed.

I'll take a deeper look into the issue after the weekend. Hope you have a great rest of your weekend!

@shehabgamin
Copy link
Contributor

I'll take a deeper look into the issue after the weekend. Hope you have a great rest of your weekend!

Most of the regressions are related to this issue: #14230. I should be able to resolve them well before the 45 release.

While testing my local Sail code with the latest commit on DataFusion's main branch, I encountered several breaking changes that may make DataFusion 45 a jarring upgrade for some users. Given the previous discussion about wanting to make releases less jarring (#13334 (comment)), I wanted to bring this to your attention, @alamb.

Aside from that, there is one remaining regression I haven't investigated yet, which seems to be related to Parquet.

@alamb
Copy link
Contributor Author

alamb commented Jan 22, 2025

While testing my local Sail code with the latest commit on DataFusion's main branch, I encountered several breaking changes that may make DataFusion 45 a jarring upgrade for some users

Thanks @shehabgamin -- Can you enumerate these changes (or point me at a PR) so we can see if there is some way to make jarring

@shehabgamin
Copy link
Contributor

Thanks @shehabgamin -- Can you enumerate these changes (or point me at a PR) so we can see if there is some way to make jarring

Yeah I'll work on that right now!

@shehabgamin
Copy link
Contributor

My apologies @alamb, the DataFusion upgrade from the latest main branch commit is smoother than I initially thought. After investigating the flood of errors, I discovered that many were resolved by simply updating Sail's serde-arrow dependency to Arrow 54. Projects without PyO3 or the pyarrow feature in DataFusion should experience a seamless upgrade (as of writing). Projects using PyO3 with the pyarrow feature enabled will have varying experiences based on their usage of PyO3.

PyO3 0.23.3
DataFusion 45 upgrades from PyO3 0.22 to 0.23.3. This is an exciting change, but may introduce significant breaking changes for PyO3 users. Since these changes vary based on PyO3 usage, I'm not listing Sail's specific changes here. Users can refer to the PyO3 migration guide: https://pyo3.rs/v0.23.0/migration

DataFusion
ValuesExec is now deprecated. The deprecation message is a bit confusing though. It currently states: "Use MemoryExec::try_new_as_values instead", but I think should say: "Use MemoryExec::try_new_as_values or MemoryExec::try_new_from_batches instead". Or, just simply: "Use MemoryExec instead".

If you'd like to see these changes, they're in my PR that's testing the regression fixes: lakehq/sail#355

@jayzhan211
Copy link
Contributor

ValuesExec is now deprecated. The deprecation message is a bit confusing though. It currently states: "Use MemoryExec::try_new_as_values instead", but I think should say: "Use MemoryExec::try_new_as_values or MemoryExec::try_new_from_batches instead". Or, just simply: "Use MemoryExec instead".

To replace ValuesExec, try_new_as_values is the right one to use not try_new_from_batches

@shehabgamin
Copy link
Contributor

To replace ValuesExec, try_new_as_values is the right one to use not try_new_from_batches

Some people currently use ValuesExec::try_new_from_batches, so MemoryExec::try_new_as_values wouldn't necessarily be a suitable substitute.

@jayzhan211
Copy link
Contributor

I see.

@shehabgamin
Copy link
Contributor

shehabgamin commented Feb 1, 2025

Derived TPC-DS Query 66 (https://github.com/lakehq/sail/blob/main/python/pysail/data/tpcds/queries/q66.sql) fails on Sail, when it previously did not.

I'm out for the day, but will look more into this tonight.

@shehabgamin
Copy link
Contributor

shehabgamin commented Feb 2, 2025

Derived TPC-DS Query 66 (https://github.com/lakehq/sail/blob/main/python/pysail/data/tpcds/queries/q66.sql) fails on Sail, when it previously did not.

Error is:

Exception: Error executing query q66 with error: Internal error:
Physical input schema should be the same as the one converted from logical input schema. 
Differences:
 - field nullability at index 7 [#98]: (physical) false vs (logical) true.

@alamb I created another PR that's dedicated only to the DF 45 upgrade. Could you please update the issue with this PR: lakehq/sail#365

Additionally, I created a tracking issue: #14408

@shehabgamin
Copy link
Contributor

Another regression is that implementing the is_nullable function in the ScalarUDFImpl trait no longer works. For example:

impl ScalarUDFImpl for SparkArray {
    ...
    fn is_nullable(&self, _args: &[Expr], _schema: &dyn ExprSchema) -> bool {
        false
    }
    ...
}

Digging into the code, I see that it's deprecated (it should still work even though it's deprecated). What's strange, however, is that the deprecation warning is not propagating to me as a downstream user. I only found this out due to a failed test.

@alamb
Copy link
Contributor Author

alamb commented Feb 2, 2025

hey @alamb quick question about the release process, are the release for datafusion and datafusion-python in locksteps? I see the next release for datafusion is 45.0.0 meanwhile datafusion-python's main branch is currently on 43.0.0

Hi @kevinjqliu -- I am not sure what the plan for datafusion-python is

The main branch seems to be on 44 to me 🤔
https://github.com/apache/datafusion-python/blob/8b513906315a0749b9f5cd6f34bf259ab4dd1add/Cargo.toml#L41-L44

@timsaucer updated it a few weeks ago

Any chance you can make a PR to test upgrading datafusion-python to 45?

Update: filed ticket to track:

@alamb
Copy link
Contributor Author

alamb commented Feb 2, 2025

Ok, given the feedback what I think we should do is:

  1. Complete the testing on our release branch (branch-45) and backport any fixes needed
  2. Continue development on main as previously planned.

Thanks @shehabgamin for the testing and reports. I'll take a more careful look tomorrow

@jayzhan211
Copy link
Contributor

Digging into the code, I see that it's deprecated (it should still work even though it's deprecated). What's strange, however, is that the deprecation warning is not propagating to me as a downstream user. I only found this out due to a failed test.

I found that we can't make this deprecated but a breaking change if the user override the is_nullable function because it takes schema: &dyn ExprSchema which is not the case for return_type_from_exprs

@timsaucer
Copy link
Contributor

Hi @kevinjqliu -- I am not sure what the plan for datafusion-python is

I am currently working on building the 44 release. I hope to get that done this morning or tomorrow. It's generally run about a month behind this upstream repo

@alamb
Copy link
Contributor Author

alamb commented Feb 3, 2025

Hi @kevinjqliu -- I am not sure what the plan for datafusion-python is

I am currently working on building the 44 release. I hope to get that done this morning or tomorrow. It's generally run about a month behind this upstream repo

In case anyone is interested, here is the release thread:

@alamb
Copy link
Contributor Author

alamb commented Feb 3, 2025

Update on the release:

main is open for new code (will be 46.0.0)

I have created a 45.0.0 branch from which to make the 45.0.0 release

I would very much like to get fixes for the following issues into the 45 branch as I think they are regressions that prevented some people from upgrading from 43 --> 44 and the fixes are fairly small:

It would also be really nice to sort out this issue from @shehabgamin but it isn't clear to me we can fix that with a small non-risky patch

@alamb
Copy link
Contributor Author

alamb commented Feb 3, 2025

@Omega359 has a fix for #11911

This afternoon I will make some backport PRs and hopefully get a release candidate created

@alamb
Copy link
Contributor Author

alamb commented Feb 3, 2025

Ok, I think we are ready with a release candidate:

tag here: https://github.com/apache/datafusion/releases/tag/45.0.0-rc1

Release voting thread: https://lists.apache.org/thread/g20ywc9yto8xp07lcllmvgyn8g5z4420

Content


This release candidate is based on commit: 26058ac 1
The proposed release tarball and signatures are hosted at 2.
The changelog is located at 3.

The standard verification procedure is documented at https://github.com/apache/datafusion/blob/main/dev/release/README.md#verifying-release-candidates.

@shehabgamin
Copy link
Contributor

shehabgamin commented Feb 3, 2025

@alamb I'll test on Sail again sometime today!

@shehabgamin
Copy link
Contributor

I updated #14408 (comment), but same issues as before with:
Test 2: Commit 26058ac (DataFusion 45.0.0-rc1).

@shehabgamin
Copy link
Contributor

It would also be really nice to sort out this issue from @shehabgamin but it isn't clear to me we can fix that with a small non-risky patch

* [DataFusion Regression (Starting in v43): Type Coercion for UDF Arguments (Int --> String) #14230](https://github.com/apache/datafusion/issues/14230)

I think we came to a temporary solution here #14268 (comment)
CC @alamb @Omega359 @jayzhan211

@Omega359
Copy link
Contributor

Omega359 commented Feb 4, 2025

I don't think I'll have the bandwidth to test a type coercion fix for UDF's myself this week to be honest. I'm about to fire off a full run of my application against the 45 branch but I likely can only do that once this week.

@Omega359
Copy link
Contributor

Omega359 commented Feb 5, 2025

Beyond the type coercion issues that I can work around my testing is working ... as long as I don't compile with 'release' target. That seems to segfault I think somewhere in DF code but I haven't yet been able to get a core dump from the crashing nodes to investigate further. I thought it was Rust 1.84.1 dependent but I retested with 1.83.0 and had the same issue.

I don't think it's a blocker but is something I'm going to continue to try and narrow down.

@wiedld
Copy link
Contributor

wiedld commented Feb 5, 2025

@alamb -- release candidate re-tested on InfluxData, and is good.

@alamb
Copy link
Contributor Author

alamb commented Feb 7, 2025

The release has been approved and published to crates.io: https://lists.apache.org/thread/73plk4nhdjby58knd5rvwsmsjzpbcg7s

With 4 +1 votes (3 binding) the release is approved!

The release is available here:
https://dist.apache.org/repos/dist/release/datafusion/datafusion-45.0.0

I have also published the release on crates.io: https://crates.io/crates/datafusion/45.0.0

Thanks everyone for your help / support -- I hope this one is epic (and a low drama release)

@alamb alamb closed this as completed Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants