[SPARK-18433][SQL] Improve DataSource option keys to be more case-insensitive #15884

dongjoon-hyun · 2016-11-15T00:25:18Z

What changes were proposed in this pull request?

This PR aims to improve DataSource option keys to be more case-insensitive

DataSource partially use CaseInsensitiveMap in code-path. For example, the following fails to find url.

val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
df.write.format("jdbc")
    .option("UrL", url1)
    .option("dbtable", "TEST.SAVETEST")
    .options(properties.asScala)
    .save()

This PR makes DataSource options to use CaseInsensitiveMap internally and also makes DataSource to use CaseInsensitiveMap generally except InMemoryFileIndex and InsertIntoHadoopFsRelationCommand. We can not pass them CaseInsensitiveMap because they creates new case-sensitive HadoopConfs by calling newHadoopConfWithOptions(options) inside.

How was this patch tested?

Pass the Jenkins test with newly added test cases.

cloud-fan · 2016-11-15T01:59:05Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JSONOptions.scala

can we just force users to pass a CaseInsensitiveMap?

Oh, sure! You mean require(parameters.isInstanceOf[CaseInsensitiveMap]), right?

Ah, just changing the function signature. parameters: CaseInsensitiveMap.

I will update this PR in that way.

JSONOptions is spark private, we can just declare the type is CaseInsensitiveMap, i.e. parameters: CaseInsensitiveMap

SparkQA · 2016-11-15T02:49:09Z

Test build #68642 has finished for PR 15884 at commit 30eff08.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class CaseInsensitiveMap(map: Map[String, String]) extends Map[String, String]

SparkQA · 2016-11-15T08:27:46Z

Test build #68654 has finished for PR 15884 at commit 937db99.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class FileStreamOptions(parameters: CaseInsensitiveMap) extends Logging

SparkQA · 2016-11-15T08:28:19Z

Test build #68655 has finished for PR 15884 at commit e69735c.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class FileStreamOptions(parameters: CaseInsensitiveMap) extends Logging

dongjoon-hyun · 2016-11-15T18:43:29Z

Thank you, @cloud-fan .
It becomes much better. Could you review again please?

…ensitive

dongjoon-hyun · 2016-11-16T04:53:53Z

Rebased to resolve conflicts.

cloud-fan · 2016-11-16T06:25:44Z

sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/json/JsonSuite.scala

  test("SPARK-6245 JsonRDD.inferSchema on empty RDD") {
    // This is really a test that it doesn't throw an exception
-    val emptySchema = InferSchema.infer(empty, "", new JSONOptions(Map()))
+    val emptySchema = InferSchema.infer(empty, "", new JSONOptions(Map.empty[String, String]))


unnecessary change?

In this case, we expect Map[String,String], but Map() or Map.empty is Map[Nothing,Nothing].
So, there was some compilation issue to find constructor of JSONOptions.

cloud-fan · 2016-11-16T06:27:26Z

LGTM

SparkQA · 2016-11-16T07:12:50Z

Test build #68695 has finished for PR 15884 at commit 6570665.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class FileStreamOptions(parameters: CaseInsensitiveMap) extends Logging

…ensitive ## What changes were proposed in this pull request? This PR aims to improve DataSource option keys to be more case-insensitive DataSource partially use CaseInsensitiveMap in code-path. For example, the following fails to find url. ```scala val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2) df.write.format("jdbc") .option("UrL", url1) .option("dbtable", "TEST.SAVETEST") .options(properties.asScala) .save() ``` This PR makes DataSource options to use CaseInsensitiveMap internally and also makes DataSource to use CaseInsensitiveMap generally except `InMemoryFileIndex` and `InsertIntoHadoopFsRelationCommand`. We can not pass them CaseInsensitiveMap because they creates new case-sensitive HadoopConfs by calling newHadoopConfWithOptions(options) inside. ## How was this patch tested? Pass the Jenkins test with newly added test cases. Author: Dongjoon Hyun <[email protected]> Closes #15884 from dongjoon-hyun/SPARK-18433. (cherry picked from commit 74f5c21) Signed-off-by: Wenchen Fan <[email protected]>

cloud-fan · 2016-11-16T09:13:10Z

thanks, merging to master/2.1!

dongjoon-hyun · 2016-11-16T09:18:36Z

Thank you so much, @cloud-fan ! Also, thank you for this issue, @gatorsmile .

…ensitive ## What changes were proposed in this pull request? This PR aims to improve DataSource option keys to be more case-insensitive DataSource partially use CaseInsensitiveMap in code-path. For example, the following fails to find url. ```scala val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2) df.write.format("jdbc") .option("UrL", url1) .option("dbtable", "TEST.SAVETEST") .options(properties.asScala) .save() ``` This PR makes DataSource options to use CaseInsensitiveMap internally and also makes DataSource to use CaseInsensitiveMap generally except `InMemoryFileIndex` and `InsertIntoHadoopFsRelationCommand`. We can not pass them CaseInsensitiveMap because they creates new case-sensitive HadoopConfs by calling newHadoopConfWithOptions(options) inside. ## How was this patch tested? Pass the Jenkins test with newly added test cases. Author: Dongjoon Hyun <[email protected]> Closes apache#15884 from dongjoon-hyun/SPARK-18433.

### What changes were proposed in this pull request? The case are not sensitive in JDBC options, after the PR #15884 is merged to Spark 2.1. ### How was this patch tested? N/A Author: gatorsmile <[email protected]> Closes #16734 from gatorsmile/fixDocCaseInsensitive. (cherry picked from commit c0eda7e) Signed-off-by: gatorsmile <[email protected]>

### What changes were proposed in this pull request? The case are not sensitive in JDBC options, after the PR apache#15884 is merged to Spark 2.1. ### How was this patch tested? N/A Author: gatorsmile <[email protected]> Closes apache#16734 from gatorsmile/fixDocCaseInsensitive.

cloud-fan reviewed Nov 15, 2016

View reviewed changes

dongjoon-hyun changed the title ~~[WIP][SPARK-18433][SQL] Improve DataSource option keys to be more case-insensitive~~ [SPARK-18433][SQL] Improve DataSource option keys to be more case-insensitive Nov 15, 2016

dongjoon-hyun added 2 commits November 15, 2016 20:35

[SPARK-18433][SQL] Improve DataSource option keys to be more case-ins…

8fbd18d

…ensitive

Replace the type of parameters with CaseInsensitiveMap.

6570665

cloud-fan reviewed Nov 16, 2016

View reviewed changes

dongjoon-hyun mentioned this pull request Nov 16, 2016

[SPARK-18413][SQL] Add maxConnections JDBCOption #15868

Closed

asfgit closed this in 74f5c21 Nov 16, 2016

dongjoon-hyun deleted the SPARK-18433 branch November 16, 2016 19:35

gatorsmile mentioned this pull request Jan 29, 2017

[SPARK-19396] [DOC] JDBC Options are Case In-sensitive #16734

Closed

[SPARK-18433][SQL] Improve DataSource option keys to be more case-insensitive #15884

[SPARK-18433][SQL] Improve DataSource option keys to be more case-insensitive #15884

Uh oh!

Conversation

dongjoon-hyun commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

cloud-fan Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 15, 2016

Uh oh!

SparkQA commented Nov 15, 2016

Uh oh!

SparkQA commented Nov 15, 2016

Uh oh!

dongjoon-hyun commented Nov 15, 2016

Uh oh!

dongjoon-hyun commented Nov 16, 2016

Uh oh!

cloud-fan Nov 16, 2016

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Nov 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Nov 16, 2016

Uh oh!

SparkQA commented Nov 16, 2016

Uh oh!

cloud-fan commented Nov 16, 2016

Uh oh!

dongjoon-hyun commented Nov 16, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dongjoon-hyun commented Nov 15, 2016 •

edited

Loading

dongjoon-hyun Nov 15, 2016 •

edited

Loading

dongjoon-hyun Nov 16, 2016 •

edited

Loading