Skip to content

Conversation

@tustvold
Copy link
Contributor

@tustvold tustvold commented Jun 3, 2024

Which issue does this PR close?

Closes #5688

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @tustvold


[Full Changelog](https://github.com/apache/arrow-rs/compare/50.0.0...51.0.0)

**Breaking changes:**
Copy link
Contributor

Choose a reason for hiding this comment

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

A substantial and distinguished release

@alamb alamb merged commit 176a2d7 into apache:master Jun 3, 2024
@tustvold
Copy link
Contributor Author

tustvold commented Jun 3, 2024

I was intending to hold this until I had updated DataFusion...

@tustvold
Copy link
Contributor Author

tustvold commented Jun 3, 2024

I've gone through apache/datafusion#10765 and the remaining test failures are mostly either formatting or memory usage changes. Thank you @waynexia

The one exception to this are the symmetric hash join, but given the amount of interval-specific logic in DF for these I am inclined to think the issue likely does not lie in arrow-rs.

As such I am going to proceed with the release, worst case we have to cut a new RC or do a fast follow patch release which would not be the end of the world.

@waynexia
Copy link
Member

waynexia commented Jun 3, 2024 via email

@waynexia
Copy link
Member

waynexia commented Jun 5, 2024

The one exception to this are the symmetric hash join, but given the amount of interval-specific logic in DF for these I am inclined to think the issue likely does not lie in arrow-rs.

The fix commit is apache/datafusion@fc9a86b. I'm not sure if IntervalDayTime::ONE should be "1 day 1 milli" or "0 day 1 milli" 🤔 But this should not be a big problem as we can choose not to use that const if its value is not expected. Only concern about it may bring some misunderstanding.

@tustvold
Copy link
Contributor Author

tustvold commented Jun 5, 2024

I'm not sure if IntervalDayTime::ONE should be "1 day 1 milli"

ONE is defined as the multiplicative identity, which in this case is 1 day 1 milli. I agree there is definitely room for confusion when it comes to arithmetic on these types

@alamb alamb mentioned this pull request Jul 2, 2024
@alamb alamb added the development-process Related to development process of arrow-rs label Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of arrow-rs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release arrow-rs / parquet version 52.0.0

3 participants