SEPURI-SAI-KRISHNA opened a new pull request, #58649:
URL: https://github.com/apache/spark/pull/58649

   ### What changes were proposed in this pull request?
   
   Backport of #58149 
([SPARK-58897](https://issues.apache.org/jira/browse/SPARK-58897)) to 
`branch-4.2`. uros-b asked for a separate PR because the original did not merge 
cleanly.
   
   `VectorFunctionImplUtils.vectorInfNorm` computed the maximum absolute value 
with a hand-rolled comparison against a running maximum seeded at zero:
   
   ```java
   float maxAbs = 0.0f;
   ...
   float absVal = Math.abs(vec.getFloat(i));
   if (absVal > maxAbs) {
     maxAbs = absVal;
   }
   ```
   
   Every comparison involving `NaN` is false under IEEE 754, so a `NaN` element 
never became the running maximum and never reached the result. The loop now 
uses `Math.max`, which returns `NaN` when either argument is `NaN`.
   
   **This diff is deliberately smaller than the one on master.** The master 
change also widened the accumulator and the return type from `float`/`Float` to 
`double`/`Double`. That widening came from 
[SPARK-58544](https://issues.apache.org/jira/browse/SPARK-58544), which is not 
on `branch-4.2`, and it was a consistency change rather than part of the `NaN` 
fix. Backporting it here would pull in an unrelated behaviour change, so this 
PR keeps the `float` accumulator and changes only the comparison. That is also 
why the original PR conflicted.
   
   For the same reason the test additions cover only the `SPARK-58897` cases. 
The `SPARK-58544` section that sits above them in master's `vector-norm.sql` is 
intentionally not included.
   
   No change is needed in `vectorNormalize`: `vectorNormalizeWithNorm` guards 
only `norm == 0.0f`, so a `NaN` norm falls through to the division and yields 
an all-`NaN` vector.
   
   ### Why are the changes needed?
   
   `vector_norm` with degree infinity returns a wrong result for any vector 
containing `NaN`. The single-element case is the damaging one: a vector of 
`NaN` is reported as having infinity norm `0.0`, making it indistinguishable 
from the zero vector, which in turn makes `vector_normalize` return `NULL` 
through its zero-norm path.
   
   ```sql
   SELECT vector_norm(array(float('nan')), float('inf'));                 -- 
0.0        expected NaN
   SELECT vector_norm(array(float('nan'), 5.0F), float('inf'));           -- 
5.0        expected NaN
   SELECT vector_norm(array(float('nan'), float('inf')), float('inf'));   -- 
Infinity   expected NaN
   SELECT vector_normalize(array(float('nan')), float('inf'));            -- 
NULL       expected [NaN]
   ```
   
   The infinity norm is the only degree that behaves this way; degrees 1.0 and 
2.0 both propagate `NaN`. It is also the only max-like function in Spark that 
does not treat `NaN` as the largest value, unlike `greatest`, `array_max`, 
`max` and `sort_array`.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, the same change as #58149. `vector_norm(v, float('inf'))` now returns 
`NaN` when `v` contains a `NaN` element, and `vector_normalize(v, 
float('inf'))` returns an all-`NaN` vector rather than `NULL`.
   
   | Query | Before | After |
   | --- | --- | --- |
   | `vector_norm(array(float('nan')), float('inf'))` | `0.0` | `NaN` |
   | `vector_norm(array(float('nan'), 5.0F), float('inf'))` | `5.0` | `NaN` |
   | `vector_norm(array(float('nan'), float('inf')), float('inf'))` | 
`Infinity` | `NaN` |
   | `vector_normalize(array(float('nan')), float('inf'))` | `NULL` | `[NaN]` |
   | `vector_normalize(array(float('nan'), 5.0F), float('inf'))` | `[NaN, 1.0]` 
| `[NaN, NaN]` |
   
   Behaviour for empty vectors (`0.0`), vectors containing `NULL` (`NULL`), and 
genuinely zero vectors is unchanged. A `NULL` element continues to take 
precedence over a `NaN` element.
   
   ### How was this patch tested?
   
   Added the `SPARK-58897` section to `sql-tests/inputs/vector-norm.sql`, 
matching the one merged to master: the single-element `NaN` case, `NaN` in 
either position, `NaN` alongside an infinite element, `NaN` in a longer 
16-element vector, the `NULL`-takes-precedence case, the L1 and L2 degrees as 
regression guards, and both `vector_normalize` cases.
   
   The golden files were regenerated on `branch-4.2` and the diff is purely 
additive, with no existing expectation changed. Every new expectation matches 
the value the same query produces on master, so the `float` accumulator kept 
here gives the same results as the widened one there.
   
   ```
   SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/testOnly 
org.apache.spark.sql.SQLQueryTestSuite -- -z vector-norm.sql"
   build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z 
vector-norm.sql"
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to