Skip to content

fix: round for float/double#4078

Open
kazuyukitanimura wants to merge 28 commits into
apache:mainfrom
kazuyukitanimura:fix-flaot-round
Open

fix: round for float/double#4078
kazuyukitanimura wants to merge 28 commits into
apache:mainfrom
kazuyukitanimura:fix-flaot-round

Conversation

@kazuyukitanimura
Copy link
Copy Markdown
Contributor

@kazuyukitanimura kazuyukitanimura commented Apr 25, 2026

Which issue does this PR close?

Part of #2551

Rationale for this change

Spark round behavior for float/double is JVM version specific

What changes are included in this PR?

useed UDFs to solve the issue

How are these changes tested?

Tests added

@kazuyukitanimura kazuyukitanimura marked this pull request as ready for review April 25, 2026 02:45
@kazuyukitanimura kazuyukitanimura marked this pull request as draft April 25, 2026 10:53
@andygrove
Copy link
Copy Markdown
Member

Thanks @kazuyukitanimura. This looks good overall, but we need to think about configs for this. We have not enabled any JVM UDFs by default yet - they should be opt-in until the UDF functionality is more mature.

There is a general discussion on the config approach in #4310

@kazuyukitanimura
Copy link
Copy Markdown
Contributor Author

kazuyukitanimura commented May 22, 2026

Thanks @kazuyukitanimura. This looks good overall, but we need to think about configs for this. We have not enabled any JVM UDFs by default yet - they should be opt-in until the UDF functionality is more mature.

There is a general discussion on the config approach in #4310

Thanks @andygrove
I reverted the changes in native/spark-expr/src/math_funcs/round.rs so that we can choose to use the incompatible native implementations in the future if we need to.
We are currently disabling round by default, no option to enable. This PR returns JVM compatible results at least. What about merging this PR first for now and that will unblock the TPCDS coverage issue?

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.

2 participants