-
Notifications
You must be signed in to change notification settings - Fork 839
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
Get the round result for decimal to a decimal with smaller scale #3224
Get the round result for decimal to a decimal with smaller scale #3224
Conversation
arrow-cast/src/cast.rs
Outdated
v.checked_div(div).ok_or_else(|| { | ||
// cast to smaller scale, need to round the result | ||
// the div must be gt_eq 10, we don't need to check the overflow for the `div`/`mod` operation | ||
let d = v / div; |
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.
Do we need to make consistent with the usage and clear compilation result?
@@ -3588,6 +3637,26 @@ mod tests { | |||
} | |||
} | |||
} | |||
|
|||
let cast_option = CastOptions { safe: false }; |
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.
add cast with safe is false
31586e9
to
7ee48e2
Compare
Benchmark runs are scheduled for baseline = bcfbd46 and contender = cb4170b. cb4170b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
#3139 just fix the
Safe is true
, and in this I miss theSafe is false
.follow-up Cast: should get the round result for decimal to a decimal with smaller scale #3139
change the ut and enhancement the macro
generate_cast_test_case
to add case withSafe is false
.Closes #3137
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?