From 566b7a5274bef77d1a786663bfe908f3e8dc9892 Mon Sep 17 00:00:00 2001 From: Sunitha Kambhampati Date: Tue, 5 Dec 2017 14:29:52 -0800 Subject: [PATCH 1/5] Add getInt, getLong, getBoolean to DataSourceV2Options and add unit tests --- .../sql/sources/v2/DataSourceV2Options.java | 18 ++++++++++++ .../sources/v2/DataSourceV2OptionsSuite.scala | 29 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java index 9a89c8193dd6e..7452ee542f969 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java +++ b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java @@ -49,4 +49,22 @@ public DataSourceV2Options(Map originalMap) { public Optional get(String key) { return Optional.ofNullable(keyLowerCasedMap.get(toLowerCase(key))); } + + public Optional getBoolean(String key) { + String lcaseKey = toLowerCase(key); + return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? + Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : null); + } + + public Optional getInt(String key) { + String lcaseKey = toLowerCase(key); + return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? + Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : null); + } + + public Optional getLong(String key) { + String lcaseKey = toLowerCase(key); + return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? + Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : null); + } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala index 933f4075bcc8a..24672c1bf333b 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala @@ -37,4 +37,33 @@ class DataSourceV2OptionsSuite extends SparkFunSuite { val options = new DataSourceV2Options(Map("foo" -> "bAr").asJava) assert(options.get("foo").get == "bAr") } + + test("getInt") { + val options = new DataSourceV2Options(Map("numFoo" -> "1", "foo" -> "bar").asJava) + assert(options.getInt("numFoo").get == 1) + + intercept[NumberFormatException]{ + options.getInt("foo").isPresent + } + } + + test("getBoolean") { + val options = new DataSourceV2Options( + Map("isFoo" -> "true", "isFoo2" -> "false", "foo" -> "bar").asJava) + assert(options.getBoolean("isFoo").get == true) + assert(options.getBoolean("isFoo2").get == false) + assert(options.getBoolean("isFoo2").isPresent) + assert(options.getBoolean("foo").get == false) + assert(!options.getBoolean("isBar").isPresent) + } + + test("getLong") { + val options = new DataSourceV2Options(Map("numFoo" -> "9223372036854775807", + "foo" -> "bar").asJava) + assert(options.getLong("numFoo").get == 9223372036854775807L) + + intercept[NumberFormatException]{ + options.getLong("foo").isPresent + } + } } From bb01ff5ad214356a6bfdbebc36f1a7b2f37a46f1 Mon Sep 17 00:00:00 2001 From: Sunitha Kambhampati Date: Tue, 5 Dec 2017 14:40:23 -0800 Subject: [PATCH 2/5] Fix formatting --- .../apache/spark/sql/sources/v2/DataSourceV2Options.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java index 7452ee542f969..757c15afff933 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java +++ b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java @@ -53,18 +53,18 @@ public Optional get(String key) { public Optional getBoolean(String key) { String lcaseKey = toLowerCase(key); return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : null); + Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : null); } public Optional getInt(String key) { String lcaseKey = toLowerCase(key); return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : null); + Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : null); } public Optional getLong(String key) { String lcaseKey = toLowerCase(key); return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : null); + Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : null); } } From 15e8ce584180d014f8b1ffeb8f0e251299cb8f31 Mon Sep 17 00:00:00 2001 From: Sunitha Kambhampati Date: Wed, 6 Dec 2017 14:54:35 -0800 Subject: [PATCH 3/5] Add methods that take defaultValue and return the primitive --- .../sql/sources/v2/DataSourceV2Options.java | 19 +++++++++++ .../sources/v2/DataSourceV2OptionsSuite.scala | 32 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java index 757c15afff933..3e1e80140fdd7 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java +++ b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java @@ -67,4 +67,23 @@ public Optional getLong(String key) { return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : null); } + + public boolean getBoolean(String key, boolean defaultValue) { + String lcaseKey = toLowerCase(key); + return keyLowerCasedMap.containsKey(lcaseKey) ? + Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : defaultValue; + } + + public int getInt(String key, int defaultValue) { + String lcaseKey = toLowerCase(key); + return keyLowerCasedMap.containsKey(lcaseKey) ? + Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : defaultValue; + } + + public long getLong(String key, long defaultValue) { + String lcaseKey = toLowerCase(key); + return keyLowerCasedMap.containsKey(lcaseKey) ? + Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : defaultValue; + } + } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala index 24672c1bf333b..f79f11ac8a275 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala @@ -66,4 +66,36 @@ class DataSourceV2OptionsSuite extends SparkFunSuite { options.getLong("foo").isPresent } } + + test("getInt - defaultValue") { + val options = new DataSourceV2Options(Map("numFOo" -> "1", "foo" -> "bar").asJava) + assert(options.getInt("numFOO", 10) == 1) + assert(options.getInt("numFOO2", 10) == 10) + + intercept[NumberFormatException]{ + options.getInt("foo") + } + } + + test("getBoolean - defaultValue") { + val options = new DataSourceV2Options( + Map("isFoo" -> "true", "isFOO2" -> "false", "foo" -> "bar").asJava) + assert(options.getBoolean("isFoo", false)) + assert(!options.getBoolean("isFoo2", true)) + assert(options.getBoolean("isBar", true)) + assert(!options.getBoolean("isBar", false)) + assert(!options.getBoolean("FOO", true)) + } + + test("getLong - defaultValue") { + val options = new DataSourceV2Options(Map("numFoo" -> "9223372036854775807", + "foo" -> "bar").asJava) + assert(options.getLong("numFOO", 0L) == 9223372036854775807L) + assert(options.getLong("numFoo2", -1L) == -1L) + + intercept[NumberFormatException]{ + options.getLong("foo", 0L) + } + } + } From 5de7962755f2175d6d11dec262644e8b9afae08b Mon Sep 17 00:00:00 2001 From: Sunitha Kambhampati Date: Wed, 6 Dec 2017 15:08:39 -0800 Subject: [PATCH 4/5] Remove the version of the 3 methods that doesnt take a default value --- .../sql/sources/v2/DataSourceV2Options.java | 18 ---------- .../sources/v2/DataSourceV2OptionsSuite.scala | 36 ++----------------- 2 files changed, 3 insertions(+), 51 deletions(-) diff --git a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java index 3e1e80140fdd7..27baab221af91 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java +++ b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java @@ -50,24 +50,6 @@ public Optional get(String key) { return Optional.ofNullable(keyLowerCasedMap.get(toLowerCase(key))); } - public Optional getBoolean(String key) { - String lcaseKey = toLowerCase(key); - return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : null); - } - - public Optional getInt(String key) { - String lcaseKey = toLowerCase(key); - return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : null); - } - - public Optional getLong(String key) { - String lcaseKey = toLowerCase(key); - return Optional.ofNullable(keyLowerCasedMap.containsKey(lcaseKey) ? - Long.parseLong(keyLowerCasedMap.get(lcaseKey)) : null); - } - public boolean getBoolean(String key, boolean defaultValue) { String lcaseKey = toLowerCase(key); return keyLowerCasedMap.containsKey(lcaseKey) ? diff --git a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala index f79f11ac8a275..752d3c193cc74 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2OptionsSuite.scala @@ -39,45 +39,16 @@ class DataSourceV2OptionsSuite extends SparkFunSuite { } test("getInt") { - val options = new DataSourceV2Options(Map("numFoo" -> "1", "foo" -> "bar").asJava) - assert(options.getInt("numFoo").get == 1) - - intercept[NumberFormatException]{ - options.getInt("foo").isPresent - } - } - - test("getBoolean") { - val options = new DataSourceV2Options( - Map("isFoo" -> "true", "isFoo2" -> "false", "foo" -> "bar").asJava) - assert(options.getBoolean("isFoo").get == true) - assert(options.getBoolean("isFoo2").get == false) - assert(options.getBoolean("isFoo2").isPresent) - assert(options.getBoolean("foo").get == false) - assert(!options.getBoolean("isBar").isPresent) - } - - test("getLong") { - val options = new DataSourceV2Options(Map("numFoo" -> "9223372036854775807", - "foo" -> "bar").asJava) - assert(options.getLong("numFoo").get == 9223372036854775807L) - - intercept[NumberFormatException]{ - options.getLong("foo").isPresent - } - } - - test("getInt - defaultValue") { val options = new DataSourceV2Options(Map("numFOo" -> "1", "foo" -> "bar").asJava) assert(options.getInt("numFOO", 10) == 1) assert(options.getInt("numFOO2", 10) == 10) intercept[NumberFormatException]{ - options.getInt("foo") + options.getInt("foo", 1) } } - test("getBoolean - defaultValue") { + test("getBoolean") { val options = new DataSourceV2Options( Map("isFoo" -> "true", "isFOO2" -> "false", "foo" -> "bar").asJava) assert(options.getBoolean("isFoo", false)) @@ -87,7 +58,7 @@ class DataSourceV2OptionsSuite extends SparkFunSuite { assert(!options.getBoolean("FOO", true)) } - test("getLong - defaultValue") { + test("getLong") { val options = new DataSourceV2Options(Map("numFoo" -> "9223372036854775807", "foo" -> "bar").asJava) assert(options.getLong("numFOO", 0L) == 9223372036854775807L) @@ -97,5 +68,4 @@ class DataSourceV2OptionsSuite extends SparkFunSuite { options.getLong("foo", 0L) } } - } From 7d87edcccc0573554b6b43e0cac17994652397bb Mon Sep 17 00:00:00 2001 From: Sunitha Kambhampati Date: Wed, 6 Dec 2017 15:45:33 -0800 Subject: [PATCH 5/5] Add comments --- .../spark/sql/sources/v2/DataSourceV2Options.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java index 27baab221af91..b2c908dc73a61 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java +++ b/sql/core/src/main/java/org/apache/spark/sql/sources/v2/DataSourceV2Options.java @@ -50,18 +50,30 @@ public Optional get(String key) { return Optional.ofNullable(keyLowerCasedMap.get(toLowerCase(key))); } + /** + * Returns the boolean value to which the specified key is mapped, + * or defaultValue if there is no mapping for the key. The key match is case-insensitive + */ public boolean getBoolean(String key, boolean defaultValue) { String lcaseKey = toLowerCase(key); return keyLowerCasedMap.containsKey(lcaseKey) ? Boolean.parseBoolean(keyLowerCasedMap.get(lcaseKey)) : defaultValue; } + /** + * Returns the integer value to which the specified key is mapped, + * or defaultValue if there is no mapping for the key. The key match is case-insensitive + */ public int getInt(String key, int defaultValue) { String lcaseKey = toLowerCase(key); return keyLowerCasedMap.containsKey(lcaseKey) ? Integer.parseInt(keyLowerCasedMap.get(lcaseKey)) : defaultValue; } + /** + * Returns the long value to which the specified key is mapped, + * or defaultValue if there is no mapping for the key. The key match is case-insensitive + */ public long getLong(String key, long defaultValue) { String lcaseKey = toLowerCase(key); return keyLowerCasedMap.containsKey(lcaseKey) ?