-
Notifications
You must be signed in to change notification settings - Fork 841
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
feat: impl *Assign ops for types in arrow-buffer #5832
Conversation
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
I run into this on migrating interval types in datafusion. fn increment_decrement<const INC: bool, T: OneTrait + SubAssign + AddAssign>() |
arrow-buffer/Cargo.toml
Outdated
@@ -37,6 +37,7 @@ bench = false | |||
bytes = { version = "1.4" } | |||
num = { version = "0.4", default-features = false, features = ["std"] } | |||
half = { version = "2.1", default-features = false } | |||
paste = { version = "1.0" } |
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.
Can we avoid using this
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.
please check 33b69f8
Signed-off-by: Ruihang Xia <[email protected]>
The failing integration test seems unrelated |
I wonder if we could use the I spent some more time trying to document them recently here #5798 |
I don't really follow the connection, these are operations on primitive scalars, not arrays. |
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.
I think this is fine, given the types in question are Copy this doesn't make any difference, but is harmless enough
TIL. Those mut-in-place APIs look good. I believe it can help improve UDF/Plan's implementations. As for |
By the way, I noticed that |
Good spot, I thought I had fixed that, but guess I forgot - #5837 |
Which issue does this PR close?
Closes #.
Rationale for this change
Impl
std::ops::*Assign
(likeAddAssign
) fori256
,IntervalDayTime
andIntervalMonthDayNano
.What changes are included in this PR?
The first commit (95da017) contains the plain implementation of
AddAssign
andSubAssign
as examplesAre there any user-facing changes?
Yes