fix: make native pow implementation compatible with Spark#5033
Open
andygrove wants to merge 1 commit into
Open
fix: make native pow implementation compatible with Spark#5033andygrove wants to merge 1 commit into
andygrove wants to merge 1 commit into
Conversation
andygrove
changed the title
| CREATE TABLE test_pow(base double, exp double) USING parquet | ||
|
|
||
| statement | ||
| INSERT INTO test_pow VALUES (0.0, -1), (2.0, 3.0), (0.0, 0.0), (-1.0, 2.0), (-1.0, 0.5), (2.0, -1.0), (NULL, 2.0), (2.0, NULL), (cast('NaN' as double), 2.0), (cast('Infinity' as double), 2.0), (2.0, cast('Infinity' as double)) |
Contributor
There was a problem hiding this comment.
What about negative zero cases? and subnormal numbers?
Contributor
There was a problem hiding this comment.
negative infinity can be interesting too. we probably need a bit more combinations.
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
2 participants
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?
There is no dedicated tracking issue. The native path's incompatibility stems from the upstream DataFusion behavior in apache/datafusion#22598.
Closes #.
Rationale for this change
Spark's
Powexpression delegates to Java'sMath.pow, which returnsInfinityfor a zero base raised to a negative exponent (pow(0, -1)). Comet's native path routedpowto DataFusion'sPowerFunc, whosefloat64_power_checkedinstead errors on that input (apache/datafusion#22598). Because of that single edge case,CometPowwas markedUnsupported, so any query usingpow/powerfell back to Spark for that part of the plan.Rust's
f64::powffollows the same IEEE-754 pow semantics as Java'sMath.pow(including0^-1 = Infinity,(-0)^-1 = -Infinity, and theNaN/Infinitycases), so a thin native kernel overpowfmatches Spark exactly and letspowrun natively by default instead of falling back.What changes are included in this PR?
spark_pownative kernel (native/spark-expr/src/math_funcs/pow.rs) that computesbase.powf(exp)element-wise with null propagation, covering all Array/Scalar argument combinations."pow"tospark_powincreate_comet_physical_fun_with_eval_mode, so it no longer falls through to DataFusion's checkedpower, plus the module/re-export wiring.CometPowto a plainCompatibleserde (drop theUnsupported/"Power has correctness issues" reporting).How are these changes tested?
pow.rscovering the basic cases,pow(0, -1) == Infinity, null propagation, and the scalar-base path.expressions/math/pow.sqlSQL file test (run viaCometSqlFileTestSuite) now asserts native execution matches Spark across zero-base-negative-exponent,NaN,Infinity, null, and column/literal argument combinations, replacing the previousexpect_fallbackassertions.