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

    https://github.com/apache/spark/pull/22553#discussion_r220718383
  
    --- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
 ---
    @@ -42,7 +42,11 @@ class CaseInsensitiveMap[T] private (val originalMap: 
Map[String, T]) extends Ma
       override def iterator: Iterator[(String, T)] = keyLowerCasedMap.iterator
     
       override def -(key: String): Map[String, T] = {
    -    new 
CaseInsensitiveMap(originalMap.filterKeys(!_.equalsIgnoreCase(key)))
    +    new CaseInsensitiveMap(originalMap.filter(!_._1.equalsIgnoreCase(key)))
    +  }
    +
    +  override def filterKeys(p: (String) => Boolean): Map[String, T] = {
    --- End diff --
    
    This method right now doesn't follow the Scaladoc:
    ```
       *  @return an immutable map consisting only of those key value pairs of 
this map where the key satisfies
       *          the predicate `p`. The resulting map wraps the original map 
without copying any elements.
    ```
    Maybe we should not add it.
    
    This is a Scala issue but they didn't fix in until 2.13 
(https://github.com/scala/bug/issues/4776).


---

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

Reply via email to