You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by MaxGekk <gi...@git.apache.org> on 2018/09/13 15:23:04 UTC
[GitHub] spark pull request #22413: Session options shouldn't override extra options
GitHub user MaxGekk opened a pull request:
https://github.com/apache/spark/pull/22413
Session options shouldn't override extra options
## What changes were proposed in this pull request?
In the PR, I propose to change order of options application. Extra options specified via `.option()` should not be overwritten by session options.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MaxGekk/spark-1 session-options
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22413.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22413
----
commit 13729b5746b8132bdd02d96930ee9eecce7aef75
Author: Maxim Gekk <ma...@...>
Date: 2018-09-13T15:19:12Z
Session options shouldn't override extra options
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
Oh, it was @MaxGekk . I helped some test cases.
BTW, @MaxGekk . Could you send a backport PR to `branch-2.4`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217703802
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
The changes were added in https://github.com/apache/spark/pull/19861
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217632328
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -241,10 +241,12 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
val source = cls.newInstance().asInstanceOf[DataSourceV2]
source match {
case provider: BatchWriteSupportProvider =>
- val options = extraOptions ++
- DataSourceV2Utils.extractSessionConfigs(source, df.sparkSession.sessionState.conf)
+ val sessionOptions = DataSourceV2Utils.extractSessionConfigs(
+ source,
+ df.sparkSession.sessionState.conf)
+ val options = sessionOptions ++ extraOptions
- val relation = DataSourceV2Relation.create(source, options.toMap)
+ val relation = DataSourceV2Relation.create(source, options)
--- End diff --
+1 for tests
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96096 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96096/testReport)** for PR 22413 at commit [`eba46d9`](https://github.com/apache/spark/commit/eba46d99ff0eb4a685bce1327d08191289add35e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96043 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96043/testReport)** for PR 22413 at commit [`13729b5`](https://github.com/apache/spark/commit/13729b5746b8132bdd02d96930ee9eecce7aef75).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/22413
@dongjoon-hyun It cannot be merged to 2.3 easily because `DataSourceV2Relation` doesn't have the `options` field, and the test for read is not compilable. I will try to fix it tomorrow.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217878344
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
Thank you. Then, it was 2.3.0.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96057 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96057/testReport)** for PR 22413 at commit [`a443054`](https://github.com/apache/spark/commit/a4430544d51a35c1c662494e53157f77a737f252).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96043/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
Thank you so much, @MaxGekk . It would be great if we can have that Spark 2.4.0 RC2.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/22413
@cloud-fan Please, take a look.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/22413
@dongjoon-hyun Here is the PR for 2.4: https://github.com/apache/spark/pull/22474 . I will check does it have conflicts in 2.3 and if so, I will backport it to 2.3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217893695
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -241,10 +241,12 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
val source = cls.newInstance().asInstanceOf[DataSourceV2]
source match {
case provider: BatchWriteSupportProvider =>
- val options = extraOptions ++
- DataSourceV2Utils.extractSessionConfigs(source, df.sparkSession.sessionState.conf)
+ val sessionOptions = DataSourceV2Utils.extractSessionConfigs(
+ source,
+ df.sparkSession.sessionState.conf)
+ val options = sessionOptions ++ extraOptions
- val relation = DataSourceV2Relation.create(source, options.toMap)
+ val relation = DataSourceV2Relation.create(source, options)
--- End diff --
For write path, I think we can use the traditional way instead of introducing mocking. Let me try.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217632335
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -241,10 +241,12 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
val source = cls.newInstance().asInstanceOf[DataSourceV2]
source match {
case provider: BatchWriteSupportProvider =>
- val options = extraOptions ++
- DataSourceV2Utils.extractSessionConfigs(source, df.sparkSession.sessionState.conf)
+ val sessionOptions = DataSourceV2Utils.extractSessionConfigs(
+ source,
+ df.sparkSession.sessionState.conf)
+ val options = sessionOptions ++ extraOptions
- val relation = DataSourceV2Relation.create(source, options.toMap)
+ val relation = DataSourceV2Relation.create(source, options)
--- End diff --
+1 for tests
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
@MaxGekk . This looks worth for JIRA issue. Could you file a JIRA issue for this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96078/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
I see. Thank you, @MaxGekk !
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
Thank you, @MaxGekk . Merged to master/2.4/2.3.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96051/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96098/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217532816
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
Also, please add a test case for this. If this has a long history than SPARK-23203 (fixed at 2.4.0), we need to verify this during backporting.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
@MaxGekk . I made a write-path test case PR to you. Could you review that, https://github.com/MaxGekk/spark-1/pull/8 ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96057 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96057/testReport)** for PR 22413 at commit [`a443054`](https://github.com/apache/spark/commit/a4430544d51a35c1c662494e53157f77a737f252).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96091 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96091/testReport)** for PR 22413 at commit [`83789c6`](https://github.com/apache/spark/commit/83789c60e0cb1e556414f65490b24a16e1dda2c7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DataSourceV2WithSessionConfigs extends SimpleDataSourceV2 with SessionConfigSupport `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96098 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96098/testReport)** for PR 22413 at commit [`325b9c4`](https://github.com/apache/spark/commit/325b9c4790213b567153d6c9f85ae65ff64cc8e2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96098 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96098/testReport)** for PR 22413 at commit [`325b9c4`](https://github.com/apache/spark/commit/325b9c4790213b567153d6c9f85ae65ff64cc8e2).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options override session...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217893030
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2Suite.scala ---
@@ -385,6 +400,9 @@ class SimpleDataSourceV2 extends DataSourceV2 with BatchReadSupportProvider {
}
}
+class DataSourceV2WithSessionConfigs extends SimpleDataSourceV2 with SessionConfigSupport {
+ def keyPrefix(): String = "test"
+}
--- End diff --
Ur, it seems that we can use the existing one.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/22413
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217878576
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/v2/DataSourceV2Suite.scala ---
@@ -385,6 +400,9 @@ class SimpleDataSourceV2 extends DataSourceV2 with BatchReadSupportProvider {
}
}
+class DataSourceWithSessionConfV2 extends SimpleDataSourceV2 with SessionConfigSupport {
--- End diff --
`DataSourceWithSessionConfV2` -> `DataSourceV2WithSessionConfig`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96078 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96078/testReport)** for PR 22413 at commit [`48d4cef`](https://github.com/apache/spark/commit/48d4cefd5677bf6e5f690a2b608ee2667e8c5dcb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DataSourceWithSessionConfV2 extends SimpleDataSourceV2 with SessionConfigSupport `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217532312
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
Oh. It has more history. Thanks, @MaxGekk . Could you trace down when it started? We need to mark the affected version correctly in order to know the backport candidates.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96078 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96078/testReport)** for PR 22413 at commit [`48d4cef`](https://github.com/apache/spark/commit/48d4cefd5677bf6e5f690a2b608ee2667e8c5dcb).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96043 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96043/testReport)** for PR 22413 at commit [`13729b5`](https://github.com/apache/spark/commit/13729b5746b8132bdd02d96930ee9eecce7aef75).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on the issue:
https://github.com/apache/spark/pull/22413
Thanks @MaxGekk, sorry for the original omission!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96096 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96096/testReport)** for PR 22413 at commit [`eba46d9`](https://github.com/apache/spark/commit/eba46d99ff0eb4a685bce1327d08191289add35e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96091/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96051 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96051/testReport)** for PR 22413 at commit [`a443054`](https://github.com/apache/spark/commit/a4430544d51a35c1c662494e53157f77a737f252).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217533478
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -241,10 +241,12 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
val source = cls.newInstance().asInstanceOf[DataSourceV2]
source match {
case provider: BatchWriteSupportProvider =>
- val options = extraOptions ++
- DataSourceV2Utils.extractSessionConfigs(source, df.sparkSession.sessionState.conf)
+ val sessionOptions = DataSourceV2Utils.extractSessionConfigs(
+ source,
+ df.sparkSession.sessionState.conf)
+ val options = sessionOptions ++ extraOptions
- val relation = DataSourceV2Relation.create(source, options.toMap)
+ val relation = DataSourceV2Relation.create(source, options)
--- End diff --
Both read/write-side tests.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options overwrite sessio...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217736452
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -241,10 +241,12 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) {
val source = cls.newInstance().asInstanceOf[DataSourceV2]
source match {
case provider: BatchWriteSupportProvider =>
- val options = extraOptions ++
- DataSourceV2Utils.extractSessionConfigs(source, df.sparkSession.sessionState.conf)
+ val sessionOptions = DataSourceV2Utils.extractSessionConfigs(
+ source,
+ df.sparkSession.sessionState.conf)
+ val options = sessionOptions ++ extraOptions
- val relation = DataSourceV2Relation.create(source, options.toMap)
+ val relation = DataSourceV2Relation.create(source, options)
--- End diff --
I wrote a test for read path since I was able to grab options propagated to DataSource but I have no idea so far for write path, only mocking probably. Does it make sense to do that?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96057/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on the issue:
https://github.com/apache/spark/pull/22413
> There is some conflict. Please make backporting PRs to 2.4 and 2.3.
> BTW, @MaxGekk . Could you send a backport PR to branch-2.4?
Sure, I will do that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96096/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: [SPARK-25425][SQL] Extra options should override ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22413
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22413
@MaxGekk . Could you specify `DSv2` more clearly in the PR title and description because it happens DSv2 code path only?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: Session options shouldn't override extra options
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: Session options shouldn't override extra options
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217493693
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
Also, cc @rdblue since this is introduced at https://github.com/apache/spark/commit/aadf9535b4a11b42fd9d72f636576d2da0766199#diff-f70bda59304588cc3abfa3a9840653f4R197 .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96091 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96091/testReport)** for PR 22413 at commit [`83789c6`](https://github.com/apache/spark/commit/83789c60e0cb1e556414f65490b24a16e1dda2c7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22413
good catch! I believe this was a mistake. LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options should override session...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on the issue:
https://github.com/apache/spark/pull/22413
+1, thanks for fixing this, @dongjoon-hyun!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options override session option...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22413
**[Test build #96051 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96051/testReport)** for PR 22413 at commit [`a443054`](https://github.com/apache/spark/commit/a4430544d51a35c1c662494e53157f77a737f252).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22413: [SPARK-25425][SQL] Extra options overwrite session optio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22413
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22413: Session options shouldn't override extra options
Posted by MaxGekk <gi...@git.apache.org>.
Github user MaxGekk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22413#discussion_r217513373
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala ---
@@ -202,7 +202,7 @@ class DataFrameReader private[sql](sparkSession: SparkSession) extends Logging {
DataSourceOptions.PATHS_KEY -> objectMapper.writeValueAsString(paths.toArray)
}
Dataset.ofRows(sparkSession, DataSourceV2Relation.create(
- ds, extraOptions.toMap ++ sessionOptions + pathsOption,
+ ds, sessionOptions ++ extraOptions.toMap + pathsOption,
--- End diff --
@dongjoon-hyun The commit didn't change semantic actually. Before it was:
```
(extraOptions ++
DataSourceV2Utils.extractSessionConfigs(
ds = ds.asInstanceOf[DataSourceV2],
conf = sparkSession.sessionState.conf))
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org