feat:support ansi mode rounding function#2542
Merged
andygrove merged 7 commits intoapache:mainfrom Oct 13, 2025
Merged
Conversation
1ae5c71 to
e05fc37
Compare
e05fc37 to
f1c7dc4
Compare
f1c7dc4 to
e2f5d90
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2542 +/- ##
============================================
+ Coverage 56.12% 59.09% +2.96%
- Complexity 976 1457 +481
============================================
Files 119 146 +27
Lines 11743 13645 +1902
Branches 2251 2360 +109
============================================
+ Hits 6591 8063 +1472
- Misses 4012 4362 +350
- Partials 1140 1220 +80 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@andygrove , This is the PR to support ANSI mode with rounding functions. Please take a look whenever you get a chance. |
andygrove
reviewed
Oct 12, 2025
andygrove
reviewed
Oct 12, 2025
andygrove
reviewed
Oct 12, 2025
andygrove
reviewed
Oct 12, 2025
spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@andygrove , I made the changes per review . Please take a look whenever you get a chance |
andygrove
reviewed
Oct 13, 2025
spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
Outdated
Show resolved
Hide resolved
andygrove
reviewed
Oct 13, 2025
spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
Outdated
Show resolved
Hide resolved
Member
|
@coderfender Could you also updatre the documentation (list of supported expressions) |
Contributor
Author
|
Sure let me update the documentation and the test cases @andygrove |
Contributor
Author
|
Thank you for the approval @andygrove |
coderfender
added a commit
to coderfender/datafusion-comet
that referenced
this pull request
Dec 13, 2025
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
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.
Which issue does this PR close?
Closes #466
Rationale for this change
Support ANSI mode as part of #313 (link to EPIC) .
What changes are included in this PR?
fail_on_errorin coherence with Spark definitions.CometRoundto passfail_on_errorrespecting ANSI modefalseinput to ensure existing functionality is unaffectedplanner.rsto respectfail_on_errorparamspark_roundfunction (specifically theround_integermacros) to respect ANSI mode (fail_on_error) and throw overflow error in case rounding op failsHow are these changes tested?
Unit tests on the scala side to test edge cases (with / without ANSI mode enabled)