Skip to content

feat: support Interval + Timestamp and Interval + Date#4117

Merged
tustvold merged 2 commits intoapache:masterfrom
Weijun-H:enhance-add
Apr 25, 2023
Merged

feat: support Interval + Timestamp and Interval + Date#4117
tustvold merged 2 commits intoapache:masterfrom
Weijun-H:enhance-add

Conversation

@Weijun-H
Copy link
Member

Which issue does this PR close?

Closes #4094

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Apr 24, 2023

DataType::Interval(_)
if right.data_type().is_temporal()
&& left.data_type() != right.data_type() =>
Copy link
Contributor

@tustvold tustvold Apr 24, 2023

Choose a reason for hiding this comment

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

I think this will recurse indefinitely if given two interval types with different IntervalUnit

Perhaps we could just explicitly match DataType::Timestamp | DataType::Date32 | DataType::Date32? Longer-term I hope to clean this all up as part of #3999

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe that the trait-based approach is a superior method for long-term maintenance. 👍

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

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Interval + Timestamp and Interval + Date in addition to Timestamp + Interval and Interval + Date

2 participants