Github user bavardage commented on the issue:
https://github.com/apache/spark/pull/21794
yep fair - the intent I think was clarity rather than necessarily perf:
it's misleading to have a method named 'nan safe' which has no special handling
of nans. I'll look at opening a different
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21794
I think we'd have to close this due to the behavior change, but would merge
an optimization of the existing behavior.
---
-
To
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21794
`spark-sql` suggests that -0 and 0 are considered the same though. `SELECT
-0.0 == 0.0;` returns `true`. It's probably essential not to change behavior
here, but if performance is the issue, I think
Github user bavardage commented on the issue:
https://github.com/apache/spark/pull/21794
it does seem that spark currently does distinguish -0 and 0, at least as
far as groupbys go
```
scala> case class Thing(x : Float)
defined class Thing
scala> val df =
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21794
BTW if it becomes necessary to not change the semantics, I think the
methods could at least be streamlined a bit:
```
if (x < y) {
-1
} else if (x > y) {
1
} else if
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21794
It would be good to add test cases for them since it is not covered now.
---
-
To unsubscribe, e-mail:
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21794
I think this is required since SparkSQL does not distinguish 0.0 from -0.0.
Am I correct?
cc @gatorsmile @maropu
---
-
To
Github user bavardage commented on the issue:
https://github.com/apache/spark/pull/21794
There is a way in which the `nanSafeCompare*` methods do differ from java
built-in, but that wasn't captured in the test cases (or in the suggestive
naming of the method), namely the handling of
Github user bavardage commented on the issue:
https://github.com/apache/spark/pull/21794
cc @JoshRosen who introduced this code originally, I think
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21794
Can one of the admins verify this patch?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21794
Can one of the admins verify this patch?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
11 matches
Mail list logo
