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.

Sign up for GitHub

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

Jump to bottom

Update arrow/parquet to arrow/parquet 53.3.0 #13508

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Update arrow/parquet to arrow/parquet 53.3.0 #13508

merged 3 commits into from
Nov 21, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Nov 20, 2024

Which issue does this PR close?

Rationale for this change

There are some other fixes waiting on this arrow release so let's get them integrated into DataFusion

What changes are included in this PR?

Update to latest arrow/parquet

Are these changes tested?

By CI

Are there any user-facing changes?

@alamb alamb marked this pull request as ready for review November 20, 2024 22:57
@jayzhan211
Copy link
Contributor

Why is CI forced to run in arrow 53.3 🤔 ?

@jayzhan211

This comment was marked as outdated.

Sign in to view
@jayzhan211
fix ci test
1b8bec5 
Signed-off-by: jayzhan211 <[email protected]>
@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Nov 21, 2024
@jayzhan211
Copy link
Contributor

jayzhan211 commented Nov 21, 2024 •
edited
Loading

I push the fix since this is blocking CI.

Since #13291 looks like in discussion, so I fix up the Like test which is consistent with Postgres. We can revert this to old result if there is consensus

@jonahgao
Copy link
Member

Thanks @alamb @jayzhan211

@jonahgao jonahgao merged commit a2811fc into apache:main Nov 21, 2024
27 checks passed
@jonahgao
Copy link
Member

Why is CI forced to run in arrow 53.3 🤔 ?

It seems that CI uses the latest version of dependencies that are semver compatible

@alamb
Copy link
Contributor Author

alamb commented Nov 21, 2024

Why is CI forced to run in arrow 53.3 🤔 ?

My thinking was that explicitly using arrow 53.3 will ensure that any bugs we fix that rely on arrow 53.3 features will work correctly.

If we don't update DataFusion to explicitly use 53.3.0 in Cargo.toml, CI will run (and use 53.3.0) but other projects with DataFusion could potentially use 53.2.0. If we add code to DataFusion that relies on 53.3.0 (like some of the LIKE fixes) this would be broken

@alamb alamb deleted the alamb/update_to_arrow branch November 21, 2024 12:30
@alamb
Copy link
Contributor Author

alamb commented Nov 21, 2024

Thanks @jayzhan211

@findepi findepi mentioned this pull request Nov 23, 2024
Merged
findepi
findepi reviewed Nov 23, 2024
View reviewed changes
datafusion/sqllogictest/test_files/string/string_view.slt
@@ -39,8 +39,19 @@ drop table test_source

# TODO: Revisit this issue after upgrading to the arrow-rs version that includes apache/arrow-rs#6671.
Copy link
Member

Choose a reason for hiding this comment

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

This became now addressable - #13538

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

@findepi findepi findepi left review comments

@jonahgao jonahgao jonahgao approved these changes

Assignees
No one assigned
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alamb @jayzhan211 @jonahgao @findepi