[spark] branch branch-3.0 updated: [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.0 by this push:
     new 9729144  [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
9729144 is described below

commit 972914485f60cd97e28c7dbdb3df41937257527e
Author: Dongjoon Hyun <do...@apache.org>
AuthorDate: Mon Jul 20 22:20:16 2020 -0700

    [SPARK-32377][SQL] CaseInsensitiveMap should be deterministic for addition
    
    ### What changes were proposed in this pull request?
    
    This PR aims to fix `CaseInsensitiveMap` to be deterministic for addition.
    
    ### Why are the changes needed?
    
    ```scala
    import org.apache.spark.sql.catalyst.util.CaseInsensitiveMap
    var m = CaseInsensitiveMap(Map.empty[String, String])
    Seq(("paTh", "1"), ("PATH", "2"), ("Path", "3"), ("patH", "4"), ("path", "5")).foreach { kv =>
      m = (m + kv).asInstanceOf[CaseInsensitiveMap[String]]
      println(m.get("path"))
    }
    ```
    
    **BEFORE**
    ```
    Some(1)
    Some(2)
    Some(3)
    Some(4)
    Some(1)
    ```
    
    **AFTER**
    ```
    Some(1)
    Some(2)
    Some(3)
    Some(4)
    Some(5)
    ```
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes, but this is a bug fix on non-deterministic behavior.
    
    ### How was this patch tested?
    
    Pass the newly added test case.
    
    Closes #29172 from dongjoon-hyun/SPARK-32377.
    
    Authored-by: Dongjoon Hyun <do...@apache.org>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
    (cherry picked from commit 8c7d6f9733751503f80d5a1b2463904dfefd6843)
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala   | 2 +-
 .../org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala   | 2 +-
 .../apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala  | 8 ++++++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala b/sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
index 699b42b..aff294e 100644
--- a/sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
+++ b/sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
@@ -40,7 +40,7 @@ class CaseInsensitiveMap[T] private (val originalMap: Map[String, T]) extends Ma
     keyLowerCasedMap.contains(k.toLowerCase(Locale.ROOT))
 
   override def +[B1 >: T](kv: (String, B1)): Map[String, B1] = {
-    new CaseInsensitiveMap(originalMap + kv)
+    new CaseInsensitiveMap(originalMap.filter(!_._1.equalsIgnoreCase(kv._1)) + kv)
   }
 
   override def iterator: Iterator[(String, T)] = keyLowerCasedMap.iterator
diff --git a/sql/catalyst/src/main/scala-2.13/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala b/sql/catalyst/src/main/scala-2.13/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
index 1f8cc6c..a74636a 100644
--- a/sql/catalyst/src/main/scala-2.13/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
+++ b/sql/catalyst/src/main/scala-2.13/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala
@@ -40,7 +40,7 @@ class CaseInsensitiveMap[T] private (val originalMap: Map[String, T]) extends Ma
     keyLowerCasedMap.contains(k.toLowerCase(Locale.ROOT))
 
   override def updated[B1 >: T](key: String, value: B1): Map[String, B1] = {
-    new CaseInsensitiveMap[B1](originalMap + (key -> value))
+    new CaseInsensitiveMap[B1](originalMap.filter(!_._1.equalsIgnoreCase(key)) + (key -> value))
   }
 
   override def iterator: Iterator[(String, T)] = keyLowerCasedMap.iterator
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala
index a8bb1d0..db8b740 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala
@@ -44,4 +44,12 @@ class CaseInsensitiveMapSuite extends SparkFunSuite {
     assert(m == Map("a" -> "b", "foo" -> "bar", "x" -> "y"))
     shouldBeSerializable(m)
   }
+
+  test("SPARK-32377: CaseInsensitiveMap should be deterministic for addition") {
+    var m = CaseInsensitiveMap(Map.empty[String, String])
+    Seq(("paTh", "1"), ("PATH", "2"), ("Path", "3"), ("patH", "4"), ("path", "5")).foreach { kv =>
+      m = (m + kv).asInstanceOf[CaseInsensitiveMap[String]]
+      assert(m.get("path").contains(kv._2))
+    }
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org