[ABA-26] fix(vortex-array): scale-compensate Decimal scalar mul/div#9
Open
abnobdoss wants to merge 2 commits into
Open
[ABA-26] fix(vortex-array): scale-compensate Decimal scalar mul/div#9abnobdoss wants to merge 2 commits into
abnobdoss wants to merge 2 commits into
Conversation
…scale Add failing test `issue_aba26_decimal_scalar_mul_div_compensate_scale` that demonstrates decimal mul/div in `checked_binary_numeric` do not apply scale compensation: - 1.0 * 1.0 at scale=1 returns raw 100 (10.0) instead of 10 (1.0) - 2.5 * 4.0 at scale=1 returns raw 1000 (100.0) instead of 100 (10.0) - 10.0 / 2.0 at scale=1 returns raw 5 (0.5) instead of 50 (5.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Abanoub Doss <abanoub.doss@gmail.com>
`checked_binary_numeric` previously computed raw * raw for Mul and raw / raw for Div, ignoring the decimal scale. At scale S: mul: result_raw = (lhs_raw * rhs_raw) / 10^S div: result_raw = (lhs_raw * 10^S) / rhs_raw Both paths now compute the scale_factor via `i256::checked_pow(scale as u32)` before the operation. Negative scale returns None (not yet used for mul/div); intermediate overflow also returns None, consistent with existing checked_* semantics. Two pre-existing tests that asserted the buggy behaviour are updated: - test_decimal_scalar_checked_mul: expected 500 → corrected to 5 - test_decimal_scalar_checked_div: expected 100 → corrected to 10000 Fixes ABA-26. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Abanoub Doss <abanoub.doss@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DecimalScalar::checked_binary_numericpreviously computedlhs_raw * rhs_rawfor Mul andlhs_raw / rhs_rawfor Div, ignoring the decimal scale entirely.result_raw = (lhs_raw * rhs_raw) / 10^Sresult_raw = (lhs_raw * 10^S) / rhs_rawscale_factor = i256::from_i128(10).checked_pow(scale as u32)and apply it in the Mul/Div arms. Negative scale returnsNone(unused for mul/div today). Intermediate overflow also returnsNone, consistent with existing checked-arithmetic semantics.test_decimal_scalar_checked_mul: expected raw500→ corrected to5test_decimal_scalar_checked_div: expected raw100→ corrected to10000Linear
Closes ABA-26
Validation
Test
issue_aba26_decimal_scalar_mul_div_compensate_scale(added in commit 1) confirms the bug was present before the fix and passes after.🤖 Generated with Claude Code