[AURON #2368] Native arithmetic does not honor ANSI overflow semantics (e.g. negate of Int.MinValue) #2394
Open
ShreyeshArangath wants to merge 6 commits into
Open
Conversation
Route integral UnaryMinus through Spark expression evaluation when ANSI mode is enabled so Int/Long overflow raises ARITHMETIC_OVERFLOW instead of wrapping. Update AuronExpressionSuite UnaryMinus test to validate overflow behavior for both vanilla Spark and Auron. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a checked native unary-minus expression for signed integral types, thread the ANSI flag through the plan proto, and update the Spark regression test to cover Int.MinValue and Long.MinValue with native execution preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use SparkNegativeExpr for all unary-minus plans. It delegates standard and non-ANSI behavior to DataFusion NegativeExpr and applies checked integral negation only when ANSI mode is enabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Accept Spark's version-specific overflow wording while still requiring the unary minus plan to execute natively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use Spark's typed ANSI configuration and cover explicit ANSI overflow, non-ANSI wrapping, and Spark's version-specific default behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness gap where native unary minus (negative(x) / -x) did not follow Spark ANSI overflow semantics (notably Int.MinValue / Long.MinValue) by propagating Spark’s ANSI setting into the native plan and performing checked negation for signed integral types when ANSI is enabled.
Changes:
- Adds a native
SparkNegativeExprthat performs checked negation for signed integral types when ANSI mode is enabled; otherwise delegates to DataFusion’sNegativeExpr. - Plumbs Spark’s ANSI configuration through the Spark extension → protobuf → native planner path.
- Expands Spark-side test coverage for ANSI overflow (int/long), non-ANSI wrapping, native execution, and Spark’s default ANSI behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spark-extension/src/main/scala/org/apache/spark/sql/auron/NativeConverters.scala | Passes SQLConf.get.ansiEnabled into the protobuf negative node. |
| spark-extension-shims-spark/src/test/scala/org/apache/auron/AuronExpressionSuite.scala | Adds Spark tests covering ANSI overflow for int/long, non-ANSI behavior, and default ANSI behavior. |
| native-engine/datafusion-ext-exprs/src/spark_negative.rs | Introduces SparkNegativeExpr with ANSI checked negation for integral types plus unit tests. |
| native-engine/datafusion-ext-exprs/src/lib.rs | Exposes the new spark_negative module. |
| native-engine/auron-planner/src/planner.rs | Switches negative planning from DataFusion NegativeExpr to SparkNegativeExpr, consuming ansi_enabled. |
| native-engine/auron-planner/proto/auron.proto | Extends PhysicalNegativeNode to carry bool ansi_enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Cover overflow behavior for Int8, Int16, Int32, and Int64 in the native checked-negation unit test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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 #2368
Rationale for this change
Under ANSI mode, native unary minus wraps
Int.MinValueandLong.MinValueinstead of raising an arithmetic overflow like vanilla Spark. This also affects Spark 4.x where ANSI mode is enabled by default.What changes are included in this PR?
SparkNegativeExprwith checked negation for signed integral types when ANSI mode is enabled.Are there any user-facing changes?
Correctness fix. Native unary minus now follows Spark ANSI overflow semantics.
How was this patch tested?
Full local build and test matrix for Spark 3.0 through Spark 4.1, including the configured Scala and JDK combinations.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Copilot and GPT 5.6