Skip to content

Conversation

@eitsupi
Copy link
Contributor

@eitsupi eitsupi commented Jun 24, 2025

Context #2739
Closes #2524

Since datafusion tends to require newer versions of arrow than adbc_core, the version of arrow used by adbc_core will be bumped by adbc_datafusion when compiling the entire workspace.

This PullRequest separates the arrow requirements for adbc_datafusion and others.
Since the newer arrow that meets adbc_datafusion requirements will be used when the entire workspace is compiled, a job to test other than adbc_datafuion by generating a file that locks the lower requirements on CI is added.
It make sure adbc_core will work with the older arrow.

@eitsupi
Copy link
Contributor Author

eitsupi commented Jun 24, 2025

Oops, I intended to check the behavior on my fork, but obviously I made a PullRequest to the wrong place...

@eitsupi eitsupi changed the title ci(rust): test minimum deps versions ci(rust): test adbc_core with minimal deps versions Jun 24, 2025
@eitsupi eitsupi changed the title ci(rust): test adbc_core with minimal deps versions chore(rust): separate adbc_core and adbc_datafusion arrow version requirements Jun 24, 2025
@eitsupi eitsupi marked this pull request as ready for review June 24, 2025 14:49
@github-actions github-actions bot added this to the ADBC Libraries 19 milestone Jun 24, 2025
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM. If it's possible it would be nice to see if the Arrow version could be relaxed further as Felipe suggested.

@eitsupi eitsupi marked this pull request as draft June 25, 2025 16:03
@eitsupi eitsupi marked this pull request as ready for review June 25, 2025 16:30
@eitsupi
Copy link
Contributor Author

eitsupi commented Jun 25, 2025

The workspace arrow requirement has been lowered to 53.1.0 (this is copied from #2525 and I have not examined the validity of this choice. It could be lowered more)

I believe this will likely lower the MSRV as well, but I have not done the work because it is tedious to go through the MSRVs for each dependent package. (So far, half 2.6.0 requires 1.81)
If requested, we can look into lowering the MSRV later.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

This looks good to me. I agree we can punt on changing the MSRV until later. @felipecrv does this solve your problem?

Copy link
Contributor

@felipecrv felipecrv left a comment

Choose a reason for hiding this comment

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

Yes @lidavidm, this solves my problem.

@felipecrv felipecrv merged commit 77f29cc into apache:main Jun 26, 2025
8 checks passed
@eitsupi eitsupi deleted the rust-version branch June 26, 2025 12:50
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.

rust: more flexible Arrow version requirements

3 participants