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, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
