You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cloud-fan <gi...@git.apache.org> on 2018/01/29 19:35:24 UTC
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
GitHub user cloud-fan opened a pull request:
https://github.com/apache/spark/pull/20427
[SPARK-23260][SQL] remove V2 from the class name of data source reader/writer
## What changes were proposed in this pull request?
All other classes in the reader/writer package doesn't have `V2` in their names, and the streaming reader/writer don't have `V2` either. It's more consistent to remove `V2` from `DataSourceV2Reader` and `DataSourceVWriter`.
Also this PR fixes some places that the mix-in interface doesn't extend the interface it aimed to mix in.
## How was this patch tested?
existing tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloud-fan/spark ds-v2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20427.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 #20427
----
commit ff6735973a502a989555251f898d09c3c8fbfb86
Author: Wenchen Fan <we...@...>
Date: 2018-01-29T19:27:46Z
remove V2 from the class name of data source reader/writer
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164620326
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
This is something we left behind. For a mix-in interface, it should extend the interface it aimed to mix in, so that we can guarantee, for example, classes implement `SessionConfigSupport` must also implement `DataSourceV2`.
We already did it in some mix-in interfaces, e.g. the streaming read support, `SupportsScanUnsafeRow`, `SupportsScanColumnarBatch`, etc. In this PR I just wanna fix the remaining ones, to make them consistent.
If you feel strongly about it, I can create a JIRA ticket for it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86805/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86805 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86805/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
* 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 pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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/20427#discussion_r164584822
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
+1 for @rdblue 's question.
@cloud-fan . Could you add more explanation about this change at PR description?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86793 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86793/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164543216
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/ReadSupport.java ---
@@ -18,23 +18,23 @@
package org.apache.spark.sql.sources.v2;
import org.apache.spark.annotation.InterfaceStability;
-import org.apache.spark.sql.sources.v2.reader.DataSourceV2Reader;
+import org.apache.spark.sql.sources.v2.reader.DataSourceReader;
/**
* A mix-in interface for {@link DataSourceV2}. Data sources can implement this interface to
* provide data reading ability and scan the data from the data source.
*/
@InterfaceStability.Evolving
-public interface ReadSupport {
+public interface ReadSupport extends DataSourceV2 {
/**
- * Creates a {@link DataSourceV2Reader} to scan the data from this data source.
+ * Creates a {@link DataSourceReader} to scan the data from this data source.
*
* If this method fails (by throwing an exception), the action would fail and no Spark job was
* submitted.
*
* @param options the options for the returned data source reader, which is an immutable
* case-insensitive string-to-string map.
*/
- DataSourceV2Reader createReader(DataSourceV2Options options);
+ DataSourceReader createReader(DataSourceV2Options options);
--- End diff --
Why not rename options as well?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164660069
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala ---
@@ -23,7 +23,7 @@ import org.apache.spark.sql.sources.v2.reader._
case class DataSourceV2Relation(
--- End diff --
Consider remove V2 in `DataSourceV2Relation` and `StreamingDataSourceV2Relation` ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20427
thanks, merging to master/2.3!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/357/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/346/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by sameeragarwal <gi...@git.apache.org>.
Github user sameeragarwal commented on the issue:
https://github.com/apache/spark/pull/20427
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20427
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164885723
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
@cloud-fan is sleeping. Let me submit the PR for resolving your concern. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164623731
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
Created https://issues.apache.org/jira/browse/SPARK-23262 .
I'd like to keep the changes in this PR, as it's just several lines. It's not ideal but we are rushing for the 2.3 release, I wanna speed this up a little, what do you think?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86779/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86779 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86779/testReport)** for PR 20427 at commit [`ff67359`](https://github.com/apache/spark/commit/ff6735973a502a989555251f898d09c3c8fbfb86).
* 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 #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/369/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/355/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86789/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86789 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86789/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
* 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 #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164544288
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
Why does this need to extend DataSourceV2? Why add this in a commit that appears to be nothing more than a rename?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164621094
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
It's best to keep commits clean and focused. I'd say create a new JIRA for it and do all of the mix-ins at once.
+1 when this is separated to its own PR. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20427
cc @sameeragarwal @rdblue @gatorsmile @gengliangwang
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86793/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86793 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86793/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
* This patch **fails due to an unknown error code, -9**.
* 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 #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86789 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86789/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164619561
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/ReadSupport.java ---
@@ -18,23 +18,23 @@
package org.apache.spark.sql.sources.v2;
import org.apache.spark.annotation.InterfaceStability;
-import org.apache.spark.sql.sources.v2.reader.DataSourceV2Reader;
+import org.apache.spark.sql.sources.v2.reader.DataSourceReader;
/**
* A mix-in interface for {@link DataSourceV2}. Data sources can implement this interface to
* provide data reading ability and scan the data from the data source.
*/
@InterfaceStability.Evolving
-public interface ReadSupport {
+public interface ReadSupport extends DataSourceV2 {
/**
- * Creates a {@link DataSourceV2Reader} to scan the data from this data source.
+ * Creates a {@link DataSourceReader} to scan the data from this data source.
*
* If this method fails (by throwing an exception), the action would fail and no Spark job was
* submitted.
*
* @param options the options for the returned data source reader, which is an immutable
* case-insensitive string-to-string map.
*/
- DataSourceV2Reader createReader(DataSourceV2Options options);
+ DataSourceReader createReader(DataSourceV2Options options);
--- End diff --
good point, let me rename it too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164619634
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/ReadSupport.java ---
@@ -18,23 +18,23 @@
package org.apache.spark.sql.sources.v2;
import org.apache.spark.annotation.InterfaceStability;
-import org.apache.spark.sql.sources.v2.reader.DataSourceV2Reader;
+import org.apache.spark.sql.sources.v2.reader.DataSourceReader;
/**
* A mix-in interface for {@link DataSourceV2}. Data sources can implement this interface to
* provide data reading ability and scan the data from the data source.
*/
@InterfaceStability.Evolving
-public interface ReadSupport {
+public interface ReadSupport extends DataSourceV2 {
/**
- * Creates a {@link DataSourceV2Reader} to scan the data from this data source.
+ * Creates a {@link DataSourceReader} to scan the data from this data source.
*
* If this method fails (by throwing an exception), the action would fail and no Spark job was
* submitted.
*
* @param options the options for the returned data source reader, which is an immutable
* case-insensitive string-to-string map.
*/
- DataSourceV2Reader createReader(DataSourceV2Options options);
+ DataSourceReader createReader(DataSourceV2Options options);
--- End diff --
I think we only need `V2` in the root interface: `DataSourceV2`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86779 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86779/testReport)** for PR 20427 at commit [`ff67359`](https://github.com/apache/spark/commit/ff6735973a502a989555251f898d09c3c8fbfb86).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164807449
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
Ping me on the new PR. I'm happy to review it (though it is non-binding).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SQL] remove V2 from the class name of data...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20427
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 pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164609599
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/ReadSupport.java ---
@@ -18,23 +18,23 @@
package org.apache.spark.sql.sources.v2;
import org.apache.spark.annotation.InterfaceStability;
-import org.apache.spark.sql.sources.v2.reader.DataSourceV2Reader;
+import org.apache.spark.sql.sources.v2.reader.DataSourceReader;
/**
* A mix-in interface for {@link DataSourceV2}. Data sources can implement this interface to
* provide data reading ability and scan the data from the data source.
*/
@InterfaceStability.Evolving
-public interface ReadSupport {
+public interface ReadSupport extends DataSourceV2 {
/**
- * Creates a {@link DataSourceV2Reader} to scan the data from this data source.
+ * Creates a {@link DataSourceReader} to scan the data from this data source.
*
* If this method fails (by throwing an exception), the action would fail and no Spark job was
* submitted.
*
* @param options the options for the returned data source reader, which is an immutable
* case-insensitive string-to-string map.
*/
- DataSourceV2Reader createReader(DataSourceV2Options options);
+ DataSourceReader createReader(DataSourceV2Options options);
--- End diff --
+1 on @rdblue's comment
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164676886
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala ---
@@ -23,7 +23,7 @@ import org.apache.spark.sql.sources.v2.reader._
case class DataSourceV2Relation(
--- End diff --
This is internal, we can clean it up at any time. I wanna focus on public APIs in this PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SPARK-23262][SQL] several data sour...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/20427#discussion_r164806676
--- Diff: sql/core/src/main/java/org/apache/spark/sql/sources/v2/SessionConfigSupport.java ---
@@ -25,7 +25,7 @@
* session.
*/
@InterfaceStability.Evolving
-public interface SessionConfigSupport {
+public interface SessionConfigSupport extends DataSourceV2 {
--- End diff --
Mixing large migration commits like this one with unrelated changes makes it harder to pick or revert changes without unintended side-effects. What happens if we realize that this rename was a bad idea? Reverting this commit would also revert the constraint that SessionConfigSupport extends DataSourceV2. Similarly, if we realize that these mix-ins don't need to extend DataSourceV2, then we would have to find and remove them all instead of reverting a commit. That might even sound okay, but when you're picking commits deliberately to patch branches, you need to make as few changes as possible and cherry-pick conflicts make that much harder.
The fact that you're rushing to get commits into 2.3 is even more concerning and reason to be careful, not a reason to relax our standards. Please move this to its own PR and fix all of the interfaces at once.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20427
**[Test build #86805 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86805/testReport)** for PR 20427 at commit [`b4fdbbe`](https://github.com/apache/spark/commit/b4fdbbe265943012093fbc0f54e8b22184fa2987).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20427: [SPARK-23260][SPARK-23262][SQL] several data source v2 n...
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:
https://github.com/apache/spark/pull/20427
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org