Repository: spark Updated Branches: refs/heads/master 86a2450e0 -> dd8f6b1ce
[SPARK-25541][SQL][FOLLOWUP] Remove overriding filterKeys in CaseInsensitiveMap ## What changes were proposed in this pull request? As per the discussion in https://github.com/apache/spark/pull/22553#pullrequestreview-159192221, override `filterKeys` violates the documented semantics. This PR is to remove it and add documentation. Also fix one potential non-serializable map in `FileStreamOptions`. The only one call of `CaseInsensitiveMap`'s `filterKeys` left is https://github.com/apache/spark/blob/c3c45cbd76d91d591d98cf8411fcfd30079f5969/sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveOptions.scala#L88-L90 But this one is OK. ## How was this patch tested? Existing unit tests. Closes #22562 from gengliangwang/SPARK-25541-FOLLOWUP. Authored-by: Gengliang Wang <gengliang.w...@databricks.com> Signed-off-by: Wenchen Fan <wenc...@databricks.com> Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/dd8f6b1c Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/dd8f6b1c Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/dd8f6b1c Branch: refs/heads/master Commit: dd8f6b1ce8ae7b2b75efda863fea40b29d52f657 Parents: 86a2450 Author: Gengliang Wang <gengliang.w...@databricks.com> Authored: Thu Sep 27 19:53:13 2018 +0800 Committer: Wenchen Fan <wenc...@databricks.com> Committed: Thu Sep 27 19:53:13 2018 +0800 ---------------------------------------------------------------------- .../apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala | 6 ++---- .../spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala | 6 ------ .../spark/sql/execution/streaming/FileStreamOptions.scala | 3 +-- 3 files changed, 3 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/dd8f6b1c/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala index 288a4f3..06f9598 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMap.scala @@ -24,6 +24,8 @@ import java.util.Locale * case-sensitive information is required. The primary constructor is marked private to avoid * nested case-insensitive map creation, otherwise the keys in the original map will become * case-insensitive in this scenario. + * Note: CaseInsensitiveMap is serializable. However, after transformation, e.g. `filterKeys()`, + * it may become not serializable. */ class CaseInsensitiveMap[T] private (val originalMap: Map[String, T]) extends Map[String, T] with Serializable { @@ -44,10 +46,6 @@ class CaseInsensitiveMap[T] private (val originalMap: Map[String, T]) extends Ma override def -(key: String): Map[String, T] = { new CaseInsensitiveMap(originalMap.filter(!_._1.equalsIgnoreCase(key))) } - - override def filterKeys(p: (String) => Boolean): Map[String, T] = { - new CaseInsensitiveMap(originalMap.filter(kv => p(kv._1))) - } } object CaseInsensitiveMap { http://git-wip-us.apache.org/repos/asf/spark/blob/dd8f6b1c/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/CaseInsensitiveMapSuite.scala ---------------------------------------------------------------------- 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 03eed4a..a8bb1d0 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,10 +44,4 @@ class CaseInsensitiveMapSuite extends SparkFunSuite { assert(m == Map("a" -> "b", "foo" -> "bar", "x" -> "y")) shouldBeSerializable(m) } - - test("CaseInsensitiveMap should be serializable after 'filterKeys' method") { - val m = CaseInsensitiveMap(Map("a" -> "b", "foo" -> "bar")).filterKeys(_ == "foo") - assert(m == Map("foo" -> "bar")) - shouldBeSerializable(m) - } } http://git-wip-us.apache.org/repos/asf/spark/blob/dd8f6b1c/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala index d54ed44..1d57cb0 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamOptions.scala @@ -54,8 +54,7 @@ class FileStreamOptions(parameters: CaseInsensitiveMap[String]) extends Logging Utils.timeStringAsMs(parameters.getOrElse("maxFileAge", "7d")) /** Options as specified by the user, in a case-insensitive map, without "path" set. */ - val optionMapWithoutPath: Map[String, String] = - parameters.filterKeys(_ != "path") + val optionMapWithoutPath: Map[String, String] = parameters - "path" /** * Whether to scan latest files first. If it's true, when the source finds unprocessed files in a --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org
