You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We are trying to make sure the upgrade experience for DataFusion is better and to do so we are hoping to improve the testing of major downstream projects
For version 44.0.0 we would like to test with Sail prior to release.
Describe the solution you'd like
I would like to create a PR to test DataFusion Sail with a pre-released version of DataFusion (pinned to the main branch)
After a brief review of the errors I suspect there may be something up with lists and structs but I would have to see the actual logic being tested to know for certain. Many of the errors are expected - functions not existing, etc.
After a brief review of the errors I suspect there may be something up with lists and structs but I would have to see the actual logic being tested to know for certain. Many of the errors are expected - functions not existing, etc.
I feel like we have had several bugs / issues with structs / lists the last few releases so it would be amazing if we could figure out what is going on before the release. @shehabgamin is this something you can help with?
After a brief review of the errors I suspect there may be something up with lists and structs but I would have to see the actual logic being tested to know for certain. Many of the errors are expected - functions not existing, etc.
I feel like we have had several bugs / issues with structs / lists the last few releases so it would be amazing if we could figure out what is going on before the release. @shehabgamin is this something you can help with?
@Omega359@alamb For sure, I will gladly take a look! If there are issues with lists and/or structs, it likely wouldn't be due to changes in the 44.0.0 pre-release that I tested, though. The Sail main branch and the PR branch that is testing the pre-release have the exact same errors (with the exception of slight rewording of some error logs).
This leads me to believe that any issues with lists and/or structs would likely be preexisting. I will take a close look at all of the errors, but, with that in mind, should we scope the issues into a separate GitHub issue if we do find errors? Also @Omega359, let me know if any errors in particular caught your eye.
This leads me to believe that any issues with lists and/or structs would likely be preexisting. I will take a close look at all of the errors, but, with that in mind, should we scope the issues into a separate GitHub issue if we do find errors? Also @Omega359, let me know if any errors in particular caught your eye.
That would make sense. Glad to hear the errors aren't new
Note: Additional errors in Test 2 are expected. These occur because code using the datafusion-functions-json crate (from datafusion-contrib) had to be commented out. This was necessary since the crate depends on DataFusion v43, while the current release candidate uses v44. This wasn't an issue in Test 1 because the commit from main hadn't yet been updated to version v44.
Closing this issue now, but please feel free to re-open if necessary @alamb.
Also, @Omega359 please let me know regarding our earlier discussion (linked below). I haven't taken a look yet because of the holidays, but will create a new issue if I discover anything. #13855 (comment)
Also, @Omega359 please let me know regarding our earlier discussion (linked below). I haven't taken a look yet because of the holidays, but will create a new issue if I discover anything. #13855 (comment)
Thanks for reminding me about this - I've added it to my TODO list and I'll do my best to try and get to this in the next week.
Activity
shehabgamin commentedon Dec 20, 2024
Take
shehabgamin commentedon Dec 20, 2024
Smooth Sailing testing commit
5d563d9from DataFusionmainbranch.The following test reports run tests on the PR branch and compares them against the tests from the Sail
mainbranch:Will re-test once there is a release candidate.
Omega359 commentedon Dec 20, 2024
After a brief review of the errors I suspect there may be something up with lists and structs but I would have to see the actual logic being tested to know for certain. Many of the errors are expected - functions not existing, etc.
alamb commentedon Dec 20, 2024
I feel like we have had several bugs / issues with structs / lists the last few releases so it would be amazing if we could figure out what is going on before the release. @shehabgamin is this something you can help with?
shehabgamin commentedon Dec 20, 2024
@Omega359 @alamb For sure, I will gladly take a look! If there are issues with lists and/or structs, it likely wouldn't be due to changes in the 44.0.0 pre-release that I tested, though. The Sail main branch and the PR branch that is testing the pre-release have the exact same errors (with the exception of slight rewording of some error logs).
This leads me to believe that any issues with lists and/or structs would likely be preexisting. I will take a close look at all of the errors, but, with that in mind, should we scope the issues into a separate GitHub issue if we do find errors? Also @Omega359, let me know if any errors in particular caught your eye.
alamb commentedon Dec 21, 2024
That would make sense. Glad to hear the errors aren't new
shehabgamin commentedon Dec 27, 2024
Smooth Sailing testing commit
073a3b1from DataFusion Release Candidate branch (https://github.com/apache/datafusion/tree/073a3b110852f97ccb7085ce4bfd19473b8a3f4f).The following test reports run tests on the PR branch and compares them against the tests from the Sail main branch:
Note: Additional errors in
Test 2are expected. These occur because code using thedatafusion-functions-jsoncrate (fromdatafusion-contrib) had to be commented out. This was necessary since the crate depends on DataFusionv43, while the current release candidate usesv44. This wasn't an issue inTest 1because the commit frommainhadn't yet been updated to versionv44.shehabgamin commentedon Dec 27, 2024
Closing this issue now, but please feel free to re-open if necessary @alamb.
Also, @Omega359 please let me know regarding our earlier discussion (linked below). I haven't taken a look yet because of the holidays, but will create a new issue if I discover anything.
#13855 (comment)
Happy Holidays!
alamb commentedon Dec 27, 2024
Thank you very much @shehabgamin for the help and assistance. I love it when incentives are aligned like this
Omega359 commentedon Dec 27, 2024
Thanks for reminding me about this - I've added it to my TODO list and I'll do my best to try and get to this in the next week.