-
Notifications
You must be signed in to change notification settings - Fork 803
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.
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
Support cast between Durations + between Durations all numeric types #6452
Conversation
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
CI failure seems exist to master also. Likely unrelated to this patch .. |
I believe it is #6448 which was subsequently fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tisonkun -- this looks very nice 🙏
I think this PR just needs a few more tests and it will be good to merge
Signed-off-by: tison <[email protected]>
Signed-off-by: tison <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me -- thank you @tisonkun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @tisonkun
Which issue does this PR close?
This resolves #6425.
Rationale for this change
Durations are represent by i64. They can be casted from/to each other just like Timestamps.
What changes are included in this PR?
Support cast between Durations. And catch up support cast between Durations and all numeric type.
Are there any user-facing changes?
Not quite. Users would found new reasonable cast supported, nothing works currently changed.