[
https://issues.apache.org/jira/browse/SPARK-26382?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16724319#comment-16724319
]
ASF GitHub Bot commented on SPARK-26382:
asfgit closed pull request #23334: [SPARK-26382][CORE] prefix comparator should
handle -0.0
URL: https://github.com/apache/spark/pull/23334
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
index 0910db22af004..bef1bdadb27aa 100644
---
a/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
+++
b/core/src/main/java/org/apache/spark/util/collection/unsafe/sort/PrefixComparators.java
@@ -69,6 +69,8 @@ public static long computePrefix(byte[] bytes) {
* details see http://stereopsis.com/radix.html.
*/
public static long computePrefix(double value) {
+ // normalize -0.0 to 0.0, as they should be equal
+ value = value == -0.0 ? 0.0 : value;
// Java's doubleToLongBits already canonicalizes all NaN values to the
smallest possible
// positive NaN, so there's nothing special we need to do for NaNs.
long bits = Double.doubleToLongBits(value);
diff --git
a/core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
b/core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
index 73546ef1b7a60..38cb37c524594 100644
---
a/core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
+++
b/core/src/test/scala/org/apache/spark/util/collection/unsafe/sort/PrefixComparatorsSuite.scala
@@ -125,6 +125,7 @@ class PrefixComparatorsSuite extends SparkFunSuite with
PropertyChecks {
val nan2Prefix =
PrefixComparators.DoublePrefixComparator.computePrefix(nan2)
assert(nan1Prefix === nan2Prefix)
val doubleMaxPrefix =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.MaxValue)
+// NaN is greater than the max double value.
assert(PrefixComparators.DOUBLE.compare(nan1Prefix, doubleMaxPrefix) === 1)
}
@@ -134,22 +135,34 @@ class PrefixComparatorsSuite extends SparkFunSuite with
PropertyChecks {
assert(java.lang.Double.doubleToRawLongBits(negativeNan) < 0)
val prefix =
PrefixComparators.DoublePrefixComparator.computePrefix(negativeNan)
val doubleMaxPrefix =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.MaxValue)
+// -NaN is greater than the max double value.
assert(PrefixComparators.DOUBLE.compare(prefix, doubleMaxPrefix) === 1)
}
test("double prefix comparator handles other special values properly") {
-val nullValue = 0L
+// See `SortPrefix.nullValue` for how we deal with nulls for float/double
type
+val smallestNullPrefix = 0L
+val largestNullPrefix = -1L
val nan =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.NaN)
val posInf =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.PositiveInfinity)
val negInf =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.NegativeInfinity)
val minValue =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.MinValue)
val maxValue =
PrefixComparators.DoublePrefixComparator.computePrefix(Double.MaxValue)
val zero = PrefixComparators.DoublePrefixComparator.computePrefix(0.0)
+val minusZero =
PrefixComparators.DoublePrefixComparator.computePrefix(-0.0)
+
+// null is greater than everything including NaN, when we need to treat it
as the largest value.
+assert(PrefixComparators.DOUBLE.compare(largestNullPrefix, nan) === 1)
+// NaN is greater than the positive infinity.
assert(PrefixComparators.DOUBLE.compare(nan, posInf) === 1)
assert(PrefixComparators.DOUBLE.compare(posInf, maxValue) === 1)
assert(PrefixComparators.DOUBLE.compare(maxValue, zero) === 1)
assert(PrefixComparators.DOUBLE.compare(zero, minValue) === 1)
assert(PrefixComparators.DOUBLE.compare(minValue, negInf) === 1)
-assert(PrefixComparators.DOUBLE.compare(negInf, nullValue) === 1)
+// null is smaller than everything including negative infinity, when we
need to treat it as
+// the smallest value.
+assert(PrefixComparators.DOUBLE.compare(negInf, smallestNullPrefix) === 1)
+// 0.0 should be equal to -0.0.
+assert(PrefixComparators.DOUBLE.compare(zero, minusZero) === 0)
}
}
This is an automated message from the Apache Git Service.
To respond to the message, pleas