This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new ef9062acf0dc [SPARK-54918][SQL] Clean up text of
`NormalizeFloatingNumbers` docstring
ef9062acf0dc is described below
commit ef9062acf0dcdcd2bbf17f340adca31e9681f143
Author: Nicholas Chammas <[email protected]>
AuthorDate: Sun Jun 21 14:13:47 2026 -0700
[SPARK-54918][SQL] Clean up text of `NormalizeFloatingNumbers` docstring
### What changes were proposed in this pull request?
This PR rewords the docstring for `NormalizeFloatingNumbers` to eliminate
repeated text and add some useful references. I also tweaked the formatting of
the numbered lists so they are consistent.
This is a follow-up to #53695.
### Why are the changes needed?
These changes make it a bit easier for the future reader understand the
need for and context around this optimizer rule.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
N/A
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #56146 from nchammas/normalize-floats-docstring.
Authored-by: Nicholas Chammas <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 7cc020a747b43474e742eaf80d83a8d5f78cbd08)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../optimizer/NormalizeFloatingNumbers.scala | 57 ++++++++++++----------
1 file changed, 31 insertions(+), 26 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFloatingNumbers.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFloatingNumbers.scala
index 7d5fd9fe5791..1d25788fdb6c 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFloatingNumbers.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/NormalizeFloatingNumbers.scala
@@ -28,36 +28,40 @@ import org.apache.spark.sql.types._
import org.apache.spark.util.ArrayImplicits._
/**
- * We need to take care of special floating numbers (NaN and -0.0) in several
places:
- * 1. When compare values, different NaNs should be treated as same, `-0.0`
and `0.0` should be
- * treated as same.
- * 2. In aggregate grouping keys, different NaNs should belong to the same
group, `-0.0` and `0.0`
- * should belong to the same group.
- * 3. In join keys, different NaNs should be treated as same, `-0.0` and
`0.0` should be
- * treated as same.
- * 4. In window partition keys, different NaNs should belong to the same
partition, `-0.0`
- * and `0.0` should belong to the same partition.
- * 5. In hash-based array set operations, different NaNs should be treated
as same, `-0.0`
- * and `0.0` should be treated as same.
+ * Certain pairs of floating point numbers require special handling:
+ * 1. 0.0 / -0.0
+ * 2. NaN / NaN
+ * That's because we want to treat each of these pairs of numbers as equal,
even though they have
+ * different binary representations. (Note that IEEE 754 allows multiple
distinct bit patterns for
+ * NaN.)
*
- * Case 1 is fine, as we handle NaN and `-0.0` well during comparison. For
complex types, we
- * recursively compare the fields/elements, so it's also fine.
+ * There are multiple ways we compare values that require careful handling of
the above floating
+ * point pairs:
+ * 1. Directly, via `==` or via methods of `java.lang.{type}`
+ * 2. Via raw bytes in instances of
[[org.apache.spark.sql.catalyst.expressions.UnsafeRow]]
+ * 3. Via hash sets
*
- * Case 2, 3 and 4 are problematic, as Spark SQL turns grouping/join/window
partition keys into
- * binary `UnsafeRow` and compare the binary data directly. Different NaNs
have different binary
- * representation, and the same thing happens for `-0.0` and `0.0`.
+ * This special handling is required in several places where we compare values
via one of the above
+ * methods:
+ * 1. When comparing values (direct)
+ * 2. When grouping keys for aggregates (`UnsafeRow`)
+ * 3. When joining on keys (`UnsafeRow`)
+ * 4. When partitioning keys for windows (`UnsafeRow`)
+ * 5. When executing array set operations (hash sets)
*
- * Case 5 is problematic for a similar reason: hash-based array set operations
compare elements by
- * their binary representation via hash sets.
+ * Case 1 is handled in [[org.apache.spark.sql.catalyst.util.SQLOrderingUtil]]
and in the
+ * `genEqual` method of
[[org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext]].
+ * Case 2 is handled during planning in the `Aggregation` and
`StatefulAggregationStrategy` objects
+ * of [[org.apache.spark.sql.execution.SparkStrategies]].
+ * Cases 3-5 are handled by this optimizer rule.
*
* This rule runs in two places:
- * 1. Early in `FinishAnalysis` (right after `ReplaceExpressions` and
before `EvalInlineTables`)
- * so that array set-like operations are wrapped before optimizer rules
that pre-evaluate
- * expressions (e.g. `ConstantFolding`, `ConvertToLocalRelation`,
`EvalInlineTables`).
- *
- * 2. As a late batch at the end of the optimizer, because rules like
subquery rewrite and
- * join reorder can create new joins or join conditions after
`FinishAnalysis` that still
- * need their keys to be normalized.
+ * 1. Early in `FinishAnalysis` (right after `ReplaceExpressions` and before
`EvalInlineTables`)
+ * so that array set-like operations are wrapped before optimizer rules
that pre-evaluate
+ * expressions (e.g. `ConstantFolding`, `ConvertToLocalRelation`,
`EvalInlineTables`).
+ * 2. As a late batch at the end of the optimizer, because rules like
subquery rewrite and
+ * join reorder can create new joins or join conditions after
`FinishAnalysis` that still
+ * need their keys to be normalized.
*
* Ideally we should do the normalization in the physical operators that
compare the
* binary `UnsafeRow` directly. We don't need this normalization if the Spark
SQL execution engine
@@ -92,7 +96,8 @@ object NormalizeFloatingNumbers extends Rule[LogicalPlan] {
// TODO: ideally Aggregate should also be handled here, but its
grouping expressions are
// mixed in its aggregate expressions. It's unreliable to change the
grouping expressions
- // here. For now we normalize grouping expressions in `AggUtils`
during planning.
+ // here. For now we normalize grouping expressions during planning.
See Case 2 in the
+ // Scaladoc just above.
}
.transformAllExpressionsWithPruning(_.containsAnyPattern(
ARRAY_DISTINCT, ARRAY_UNION, ARRAY_INTERSECT, ARRAY_EXCEPT,
ARRAYS_OVERLAP)) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
