Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19553#discussion_r147593724
  
    --- Diff: core/src/main/scala/org/apache/spark/api/java/JavaUtils.scala ---
    @@ -43,10 +43,17 @@ private[spark] object JavaUtils {
     
         override def size: Int = underlying.size
     
    -    override def get(key: AnyRef): B = try {
    -      underlying.getOrElse(key.asInstanceOf[A], null.asInstanceOf[B])
    -    } catch {
    -      case ex: ClassCastException => null.asInstanceOf[B]
    +    // Delegate to implementation because AbstractMap implementation 
iterates over whole key set
    +    override def containsKey(key: AnyRef): Boolean = {
    +      underlying.contains(key.asInstanceOf[A])
    +    }
    +
    +    override def get(key: AnyRef): B = {
    +      val value = underlying.get(key.asInstanceOf[A])
    +      if (value.isDefined && value.get.isInstanceOf[B]) {
    --- End diff --
    
    `underlying` values are already known to be `B`, so this isn't necessary. 
But a condition of the key is.
    
    ```
    if (key.instanceOf[A]) {
      underlying.getOrElse(key.asInstanceOf[A], null)
    } else {
      null
    }
    ```
    
    Might need an extra cast in there.


---

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

Reply via email to