[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread dongjoon-hyun 
Github user dongjoon-hyun commented on a diff in the pull request:

https://github.com/apache/spark/pull/22384#discussion_r216540786
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelationSuite.scala
 ---
@@ -147,7 +147,7 @@ class PropagateEmptyRelationSuite extends PlanTest {
   .where(false)
   .select('a)
   .where('a > 1)
-  .where('a != 200)
+  .where('a =!= 200)
--- End diff --

Oh, thank you for fixing this.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22384#discussion_r216454727
  
--- Diff: core/src/main/scala/org/apache/spark/status/LiveEntity.scala ---
@@ -584,8 +583,7 @@ private object LiveEntityHelpers {
   .filter { acc =>
 // We don't need to store internal or SQL accumulables as their 
values will be shown in
 // other places, so drop them to reduce the memory usage.
-!acc.internal && (!acc.metadata.isDefined ||
-  acc.metadata.get != 
Some(AccumulatorContext.SQL_ACCUM_IDENTIFIER))
+!acc.internal && acc.metadata != 
Some(AccumulatorContext.SQL_ACCUM_IDENTIFIER)
--- End diff --

CC @vanzin on this one for a double-check


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22384#discussion_r216455205
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/UnsafeArraySuite.scala
 ---
@@ -114,15 +114,15 @@ class UnsafeArraySuite extends SparkFunSuite {
 assert(unsafeDate.isInstanceOf[UnsafeArrayData])
 assert(unsafeDate.numElements == dateArray.length)
 dateArray.zipWithIndex.map { case (e, i) =>
-  assert(unsafeDate.get(i, DateType) == e)
+  assert(unsafeDate.get(i, DateType).asInstanceOf[Int] == e)
--- End diff --

Issue here is that you can't really compare `AnyRef` to a primitive, not 
via boxing or unboxing; this may just be making some Scala coercion explicit 
but seemed worthwhile


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22384#discussion_r216454913
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PropagateEmptyRelationSuite.scala
 ---
@@ -147,7 +147,7 @@ class PropagateEmptyRelationSuite extends PlanTest {
   .where(false)
   .select('a)
   .where('a > 1)
-  .where('a != 200)
+  .where('a =!= 200)
--- End diff --

(`!=` isn't actually the Column operator for historical reasons having to 
do with precedence)


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread srowen 
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/22384#discussion_r216454783
  
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -175,7 +175,7 @@ private[spark] object ClosureCleaner extends Logging {
   closure.getClass.isSynthetic &&
 closure
   .getClass
-  .getInterfaces.exists(_.getName.equals("scala.Serializable"))
+  .getInterfaces.exists(_.getName == "scala.Serializable")
--- End diff --

See JIRA for explanation of this one


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #22384: [SPARK-25398][CORE][MESOS] Minor bugs from compar...

2018-09-10 Thread srowen 
GitHub user srowen opened a pull request:

https://github.com/apache/spark/pull/22384

[SPARK-25398][CORE][MESOS] Minor bugs from comparing unrelated types

## What changes were proposed in this pull request?

Correct some comparisons between unrelated types to what they seem to… 
have been trying to do

## How was this patch tested?

Existing tests.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/srowen/spark SPARK-25398

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/22384.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #22384


commit 9e70b625992310a44880a9e42f6fead6c2068dc7
Author: Sean Owen 
Date:   2018-09-10T19:58:37Z

Correct some comparisons between unrelated types to what they seem to have 
been trying to do




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org