You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "edubraqd (via GitHub)" <gi...@apache.org> on 2026/09/03 02:46:30 UTC

[PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

edubraqd opened a new pull request, #24897:
URL: https://github.com/apache/datafusion/pull/24897

   ## Which issue does this PR close?
   
   - Closes #24896.
   
   ## Rationale for this change
   
   `try_cast_numeric_literal` computes `10_i128.pow(scale as u32)` for both the literal's and the target's decimal scale. A negative scale wraps to a huge exponent, so simplifying `arrow_cast(1, 'Decimal128(10, -2)') = 100` panicked with "attempt to multiply with overflow" in the `unwrap_cast` simplifier.
   
   ## What changes are included in this PR?
   
   A negative scale means the decimal holds multiples of `10^-scale`, so the integer rescaling this path relies on does not apply. `try_cast_numeric_literal` now returns `None` when either side has a negative scale, so the simplifier leaves the cast in place, the same way it already does for other casts it cannot express.
   
   ## Are these changes tested?
   
   Yes. `test_try_cast_to_type_negative_scale_decimal` covers a negative-scale target for `Decimal32`/`Decimal64`/`Decimal128` and a negative-scale literal; each used to panic.
   
   ## Are there any user-facing changes?
   
   No panic. Such comparisons are no longer simplified by `unwrap_cast`; they were never simplified correctly before, since the process panicked.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #24897:
URL: https://github.com/apache/datafusion/pull/24897#issuecomment-5582985369

   @edubraqd 
   
   When I ran this in datafusion-cli:
   ```
   SELECT arrow_cast(1, 'Decimal128(10, -2)') = 100;
   ```
   I got
   ```
   DataFusion CLI v55.0.0
   +--------------------------------------------------------------+
   | arrow_cast(Int64(1),Utf8("Decimal128(10, -2)")) = Int64(100) |
   +--------------------------------------------------------------+
   | false                                                        |
   +--------------------------------------------------------------+
   ```
   
   Shouldn't it be `true`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24897:
URL: https://github.com/apache/datafusion/pull/24897#issuecomment-5583951785

   ## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24897?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `96.96970%` with `1 line` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 81.63%. Comparing base ([`d02bd78`](https://app.codecov.io/gh/apache/datafusion/commit/d02bd786de49f2ad3dc15cb947608fb0f01fe142?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`62269ce`](https://app.codecov.io/gh/apache/datafusion/commit/62269ceefba24a00cefcbcb33e958f53e5714a8d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 72 commits behind head on main.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24897?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [datafusion/expr-common/src/casts.rs](https://app.codecov.io/gh/apache/datafusion/pull/24897?src=pr&el=tree&filepath=datafusion%2Fexpr-common%2Fsrc%2Fcasts.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByLWNvbW1vbi9zcmMvY2FzdHMucnM=) | 96.96% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24897?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##             main   #24897   +/-   ##
   =======================================
     Coverage   81.63%   81.63%           
   =======================================
     Files        1123     1123           
     Lines      409546   409579   +33     
     Branches   409546   409579   +33     
   =======================================
   + Hits       334319   334350   +31     
   - Misses      55591    55596    +5     
   + Partials    19636    19633    -3     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24897?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #24897:
URL: https://github.com/apache/datafusion/pull/24897#issuecomment-5590884513

   Marking as draft as I think this PR is no longer waiting on feedback and I am trying to make it easier to find PRs in need of review. Please mark it as ready for review when it is ready for another look 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed pull request #24897: fix: do not overflow in `unwrap_cast` for decimals with a negative scale
URL: https://github.com/apache/datafusion/pull/24897


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: do not overflow in `unwrap_cast` for decimals with a negative scale [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #24897:
URL: https://github.com/apache/datafusion/pull/24897#issuecomment-5640799119

   Closing per https://github.com/apache/datafusion/pull/24893#issuecomment-5640786567


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org