Skip to content

feat:support ansi mode rounding function#2542

Merged
andygrove merged 7 commits intoapache:mainfrom
coderfender:support_ansi_mode_rounding_function
Oct 13, 2025
Merged

feat:support ansi mode rounding function#2542
andygrove merged 7 commits intoapache:mainfrom
coderfender:support_ansi_mode_rounding_function

Conversation

@coderfender
Copy link
Contributor

@coderfender coderfender commented Oct 10, 2025

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?

  1. Changes to proto definition for scalar function to add a new parameter called fail_on_error in coherence with Spark definitions.
  2. Changes to CometRound to pass fail_on_error respecting ANSI mode
  3. Update other scalar functions with false input to ensure existing functionality is unaffected
  4. Changes in planner.rs to respect fail_on_error param
  5. Changes in spark_round function (specifically the round_integer macros) to respect ANSI mode (fail_on_error) and throw overflow error in case rounding op fails
  6. Unit tests on the scala side to test edge cases (with / without ANSI mode enabled)

How are these changes tested?

Unit tests on the scala side to test edge cases (with / without ANSI mode enabled)

@coderfender coderfender force-pushed the support_ansi_mode_rounding_function branch from 1ae5c71 to e05fc37 Compare October 10, 2025 17:51
@coderfender coderfender force-pushed the support_ansi_mode_rounding_function branch from e05fc37 to f1c7dc4 Compare October 12, 2025 00:05
@coderfender coderfender changed the title [WIP]: Support ansi mode rounding function feat:support ansi mode rounding function Oct 12, 2025
@coderfender coderfender force-pushed the support_ansi_mode_rounding_function branch from f1c7dc4 to e2f5d90 Compare October 12, 2025 01:09
@codecov-commenter
Copy link

codecov-commenter commented Oct 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.09%. Comparing base (f09f8af) to head (f6392d3).
⚠️ Report is 640 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderfender
Copy link
Contributor Author

@andygrove , This is the PR to support ANSI mode with rounding functions. Please take a look whenever you get a chance.

@coderfender
Copy link
Contributor Author

@andygrove , I made the changes per review . Please take a look whenever you get a chance

@coderfender coderfender requested a review from andygrove October 12, 2025 19:13
@andygrove
Copy link
Member

@coderfender Could you also updatre the documentation (list of supported expressions)

@coderfender
Copy link
Contributor Author

Sure let me update the documentation and the test cases @andygrove

Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @coderfender

@coderfender
Copy link
Contributor Author

Thank you for the approval @andygrove

@andygrove andygrove merged commit acfd03c into apache:main Oct 13, 2025
102 checks passed
coderfender added a commit to coderfender/datafusion-comet that referenced this pull request Dec 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Support ANSI mode for round

3 participants