[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #20427: [SPARK-23260][SQL] remove V2 from the class name ...
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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[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 Date: 2018-01-29T19:27:46Z remove V2 from the class name of data source reader/writer --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
