[GitHub] spark pull request #21145: [SPARK-24073][SQL]: Rename DataReaderFactory to I...

2018-05-09 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/21145


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21145: [SPARK-24073][SQL]: Rename DataReaderFactory to I...

2018-05-09 Thread rdblue
Github user rdblue commented on a diff in the pull request:

https://github.com/apache/spark/pull/21145#discussion_r187085278
  
--- Diff: 
sql/core/src/main/java/org/apache/spark/sql/sources/v2/reader/DataSourceReader.java
 ---
@@ -76,5 +76,5 @@
* If this method fails (by throwing an exception), the action would 
fail and no Spark job was
* submitted.
*/
-  List createDataReaderFactories();
+  List planInputPartitions();
--- End diff --

I think plan is a more accurate verb. To some Java people, `get` implies 
that the call is very cheap because it is associated with getters, which 
typically just return a field's value. Since that's not the case here and 
callers shouldn't consider this method cheap, I think it makes sense to use a 
different name that reflects what is actually happening: split planning.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21145: [SPARK-24073][SQL]: Rename DataReaderFactory to I...

2018-05-08 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/21145#discussion_r186927701
  
--- Diff: 
sql/core/src/main/java/org/apache/spark/sql/sources/v2/reader/DataSourceReader.java
 ---
@@ -76,5 +76,5 @@
* If this method fails (by throwing an exception), the action would 
fail and no Spark job was
* submitted.
*/
-  List createDataReaderFactories();
+  List planInputPartitions();
--- End diff --

in the hadoop world there is `InputFormat.getSplits`, shall we follow and 
use `getInputPartitions` here?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org