{"action":"created","comment":{"url":"https://api.github.com/repos/apache/spark/pulls/comments/208660320","pull_request_review_id":144516311,"id":208660320,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIwODY2MDMyMA==","diff_hunk":"@@ -39,52 +36,43 @@ case class DataSourceV2ScanExec(\n     @transient source: DataSourceV2,\n     @transient options: Map[String, String],\n     @transient pushedFilters: Seq[Expression],\n-    @transient reader: DataSourceReader)\n+    @transient readSupport: ReadSupport,\n+    @transient scanConfig: ScanConfig)\n   extends LeafExecNode with DataSourceV2StringFormat with ColumnarBatchScan {\n \n   override def simpleString: String = \"ScanV2 \" + metadataString\n \n   // TODO: unify the equal/hashCode implementation for all data source v2 query plans.\n   override def equals(other: Any): Boolean = other match {\n     case other: DataSourceV2ScanExec =>\n-      output == other.output && reader.getClass == other.reader.getClass && options == other.options\n+      output == other.output && readSupport.getClass == other.readSupport.getClass &&\n+        options == other.options\n     case _ => false\n   }\n \n   override def hashCode(): Int = {\n     Seq(output, source, options).hashCode()\n   }\n \n-  override def outputPartitioning: physical.Partitioning = reader match {\n-    case r: SupportsScanColumnarBatch if r.enableBatchRead() && batchPartitions.size == 1 =>\n-      SinglePartition\n-\n-    case r: SupportsScanColumnarBatch if !r.enableBatchRead() && partitions.size == 1 =>\n-      SinglePartition\n-\n-    case r if !r.isInstanceOf[SupportsScanColumnarBatch] && partitions.size == 1 =>\n+  override def outputPartitioning: physical.Partitioning = readSupport match {\n+    case _ if partitions.length == 1 =>\n       SinglePartition\n \n     case s: SupportsReportPartitioning =>\n       new DataSourcePartitioning(\n-        s.outputPartitioning(), AttributeMap(output.map(a => a -> a.name)))\n+        s.outputPartitioning(scanConfig), AttributeMap(output.map(a => a -> a.name)))\n \n     case _ => super.outputPartitioning\n   }\n \n-  private lazy val partitions: Seq[InputPartition[InternalRow]] = {\n-    reader.planInputPartitions().asScala\n-  }\n+  private lazy val partitions: Seq[InputPartition] = readSupport.planInputPartitions(scanConfig)\n \n-  private lazy val batchPartitions: Seq[InputPartition[ColumnarBatch]] = reader match {\n-    case r: SupportsScanColumnarBatch if r.enableBatchRead() =>\n-      assert(!reader.isInstanceOf[ContinuousReader],\n-        \"continuous stream reader does not support columnar read yet.\")\n-      r.planBatchInputPartitions().asScala\n-  }\n+  private lazy val partitionReaderFactory = readSupport.createReaderFactory(scanConfig)\n \n-  private lazy val inputRDD: RDD[InternalRow] = reader match {\n-    case _: ContinuousReader =>\n+  private lazy val inputRDD: RDD[InternalRow] = readSupport match {\n+    case _: ContinuousReadSupport =>\n+      assert(!partitionReaderFactory.supportColumnarReads(),","path":"sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2ScanExec.scala","position":80,"original_position":80,"commit_id":"c22499964ac759670c3629c690f77018bc79a7c1","original_commit_id":"c22499964ac759670c3629c690f77018bc79a7c1","user":{"login":"cloud-fan","id":3182036,"node_id":"MDQ6VXNlcjMxODIwMzY=","avatar_url":"https://avatars3.githubusercontent.com/u/3182036?v=4","gravatar_id":"","url":"https://api.github.com/users/cloud-fan","html_url":"https://github.com/cloud-fan","followers_url":"https://api.github.com/users/cloud-fan/followers","following_url":"https://api.github.com/users/cloud-fan/following{/other_user}","gists_url":"https://api.github.com/users/cloud-fan/gists{/gist_id}","starred_url":"https://api.github.com/users/cloud-fan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cloud-fan/subscriptions","organizations_url":"https://api.github.com/users/cloud-fan/orgs","repos_url":"https://api.github.com/users/cloud-fan/repos","events_url":"https://api.github.com/users/cloud-fan/events{/privacy}","received_events_url":"https://api.github.com/users/cloud-fan/received_events","type":"User","site_admin":false},"body":"I think the answer is no. We may have columnar only data source, so I don't think it's a good idea to let Spark decide the scan mode.","created_at":"2018-08-08T17:04:43Z","updated_at":"2018-08-08T17:04:43Z","html_url":"https://github.com/apache/spark/pull/22009#discussion_r208660320","pull_request_url":"https://api.github.com/repos/apache/spark/pulls/22009","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/comments/208660320"},"html":{"href":"https://github.com/apache/spark/pull/22009#discussion_r208660320"},"pull_request":{"href":"https://api.github.com/repos/apache/spark/pulls/22009"}},"in_reply_to_id":208383098},"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/22009","id":206463009,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA2NDYzMDA5","html_url":"https://github.com/apache/spark/pull/22009","diff_url":"https://github.com/apache/spark/pull/22009.diff","patch_url":"https://github.com/apache/spark/pull/22009.patch","issue_url":"https://api.github.com/repos/apache/spark/issues/22009","number":22009,"state":"open","locked":false,"title":"[SPARK-24882][SQL] improve data source v2 API","user":{"login":"cloud-fan","id":3182036,"node_id":"MDQ6VXNlcjMxODIwMzY=","avatar_url":"https://avatars3.githubusercontent.com/u/3182036?v=4","gravatar_id":"","url":"https://api.github.com/users/cloud-fan","html_url":"https://github.com/cloud-fan","followers_url":"https://api.github.com/users/cloud-fan/followers","following_url":"https://api.github.com/users/cloud-fan/following{/other_user}","gists_url":"https://api.github.com/users/cloud-fan/gists{/gist_id}","starred_url":"https://api.github.com/users/cloud-fan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cloud-fan/subscriptions","organizations_url":"https://api.github.com/users/cloud-fan/orgs","repos_url":"https://api.github.com/users/cloud-fan/repos","events_url":"https://api.github.com/users/cloud-fan/events{/privacy}","received_events_url":"https://api.github.com/users/cloud-fan/received_events","type":"User","site_admin":false},"body":"## What changes were proposed in this pull request?\r\n\r\nImprove the data source v2 API according to the [design doc](https://docs.google.com/document/d/1DDXCTCrup4bKWByTalkXWgavcPdvur8a4eEu8x1BzPM/edit?usp=sharing)\r\n\r\nsummary of the changes\r\n1. rename `ReadSupport` -> `DataSourceReader` -> `InputPartition` -> `InputPartitionReader` to `BatchReadSupportProvider` -> `BatchReadSupport` -> `InputPartition`/`PartitionReaderFactory` -> `PartitionReader`. Similar renaming also happens at streaming and write APIs.\r\n2. create `ScanConfig` to store query specific information like operator pushdown result, streaming offsets, etc. This makes batch and streaming `ReadSupport`(previouslly named `DataSourceReader`) immutable. All other methods take `ScanConfig` as input, which implies applying operator pushdown and getting streaming offsets happen before all other things(get input partitions, report statistics, etc.).\r\n3. separate `InputPartition` to `InputPartition` and `PartitionReaderFactory`. This is a natural separation, data splitting and reading are orthogonal and we should not mix them in one interfaces. This also makes the naming consistent between read and write API: `PartitionReaderFactory` vs `DataWriterFactory`.\r\n4. separate the batch and streaming interfaces. Sometimes it's painful to force the streaming interface to extend batch interface, as we may need to override some batch methods to return false, or even leak the streaming concept to batch API(e.g. `DataWriterFactory#createWriter(partitionId, taskId, epochId)`)\r\n\r\n## How was this patch tested?\r\n\r\nexisting tests.","created_at":"2018-08-06T17:14:14Z","updated_at":"2018-08-08T17:04:43Z","closed_at":null,"merged_at":null,"merge_commit_sha":"e040304b6d20130771603b4985aa9d1dc44cd8bc","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/apache/spark/pulls/22009/commits","review_comments_url":"https://api.github.com/repos/apache/spark/pulls/22009/comments","review_comment_url":"https://api.github.com/repos/apache/spark/pulls/comments{/number}","comments_url":"https://api.github.com/repos/apache/spark/issues/22009/comments","statuses_url":"https://api.github.com/repos/apache/spark/statuses/c22499964ac759670c3629c690f77018bc79a7c1","head":{"label":"cloud-fan:redesign","ref":"redesign","sha":"c22499964ac759670c3629c690f77018bc79a7c1","user":{"login":"cloud-fan","id":3182036,"node_id":"MDQ6VXNlcjMxODIwMzY=","avatar_url":"https://avatars3.githubusercontent.com/u/3182036?v=4","gravatar_id":"","url":"https://api.github.com/users/cloud-fan","html_url":"https://github.com/cloud-fan","followers_url":"https://api.github.com/users/cloud-fan/followers","following_url":"https://api.github.com/users/cloud-fan/following{/other_user}","gists_url":"https://api.github.com/users/cloud-fan/gists{/gist_id}","starred_url":"https://api.github.com/users/cloud-fan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cloud-fan/subscriptions","organizations_url":"https://api.github.com/users/cloud-fan/orgs","repos_url":"https://api.github.com/users/cloud-fan/repos","events_url":"https://api.github.com/users/cloud-fan/events{/privacy}","received_events_url":"https://api.github.com/users/cloud-fan/received_events","type":"User","site_admin":false},"repo":{"id":19689699,"node_id":"MDEwOlJlcG9zaXRvcnkxOTY4OTY5OQ==","name":"spark","full_name":"cloud-fan/spark","owner":{"login":"cloud-fan","id":3182036,"node_id":"MDQ6VXNlcjMxODIwMzY=","avatar_url":"https://avatars3.githubusercontent.com/u/3182036?v=4","gravatar_id":"","url":"https://api.github.com/users/cloud-fan","html_url":"https://github.com/cloud-fan","followers_url":"https://api.github.com/users/cloud-fan/followers","following_url":"https://api.github.com/users/cloud-fan/following{/other_user}","gists_url":"https://api.github.com/users/cloud-fan/gists{/gist_id}","starred_url":"https://api.github.com/users/cloud-fan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cloud-fan/subscriptions","organizations_url":"https://api.github.com/users/cloud-fan/orgs","repos_url":"https://api.github.com/users/cloud-fan/repos","events_url":"https://api.github.com/users/cloud-fan/events{/privacy}","received_events_url":"https://api.github.com/users/cloud-fan/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/cloud-fan/spark","description":"Mirror of Apache Spark","fork":true,"url":"https://api.github.com/repos/cloud-fan/spark","forks_url":"https://api.github.com/repos/cloud-fan/spark/forks","keys_url":"https://api.github.com/repos/cloud-fan/spark/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cloud-fan/spark/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cloud-fan/spark/teams","hooks_url":"https://api.github.com/repos/cloud-fan/spark/hooks","issue_events_url":"https://api.github.com/repos/cloud-fan/spark/issues/events{/number}","events_url":"https://api.github.com/repos/cloud-fan/spark/events","assignees_url":"https://api.github.com/repos/cloud-fan/spark/assignees{/user}","branches_url":"https://api.github.com/repos/cloud-fan/spark/branches{/branch}","tags_url":"https://api.github.com/repos/cloud-fan/spark/tags","blobs_url":"https://api.github.com/repos/cloud-fan/spark/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cloud-fan/spark/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cloud-fan/spark/git/refs{/sha}","trees_url":"https://api.github.com/repos/cloud-fan/spark/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cloud-fan/spark/statuses/{sha}","languages_url":"https://api.github.com/repos/cloud-fan/spark/languages","stargazers_url":"https://api.github.com/repos/cloud-fan/spark/stargazers","contributors_url":"https://api.github.com/repos/cloud-fan/spark/contributors","subscribers_url":"https://api.github.com/repos/cloud-fan/spark/subscribers","subscription_url":"https://api.github.com/repos/cloud-fan/spark/subscription","commits_url":"https://api.github.com/repos/cloud-fan/spark/commits{/sha}","git_commits_url":"https://api.github.com/repos/cloud-fan/spark/git/commits{/sha}","comments_url":"https://api.github.com/repos/cloud-fan/spark/comments{/number}","issue_comment_url":"https://api.github.com/repos/cloud-fan/spark/issues/comments{/number}","contents_url":"https://api.github.com/repos/cloud-fan/spark/contents/{+path}","compare_url":"https://api.github.com/repos/cloud-fan/spark/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cloud-fan/spark/merges","archive_url":"https://api.github.com/repos/cloud-fan/spark/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cloud-fan/spark/downloads","issues_url":"https://api.github.com/repos/cloud-fan/spark/issues{/number}","pulls_url":"https://api.github.com/repos/cloud-fan/spark/pulls{/number}","milestones_url":"https://api.github.com/repos/cloud-fan/spark/milestones{/number}","notifications_url":"https://api.github.com/repos/cloud-fan/spark/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cloud-fan/spark/labels{/name}","releases_url":"https://api.github.com/repos/cloud-fan/spark/releases{/id}","deployments_url":"https://api.github.com/repos/cloud-fan/spark/deployments","created_at":"2014-05-12T07:32:26Z","updated_at":"2017-06-22T06:57:26Z","pushed_at":"2018-08-07T06:47:33Z","git_url":"git://github.com/cloud-fan/spark.git","ssh_url":"git@github.com:cloud-fan/spark.git","clone_url":"https://github.com/cloud-fan/spark.git","svn_url":"https://github.com/cloud-fan/spark","homepage":null,"size":271190,"stargazers_count":1,"watchers_count":1,"language":"Scala","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":1,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"apache:master","ref":"master","sha":"4446a0b0d9bd830f0e903d6780dedac4db572b5a","user":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"repo":{"id":17165658,"node_id":"MDEwOlJlcG9zaXRvcnkxNzE2NTY1OA==","name":"spark","full_name":"apache/spark","owner":{"login":"apache","id":47359,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ3MzU5","avatar_url":"https://avatars0.githubusercontent.com/u/47359?v=4","gravatar_id":"","url":"https://api.github.com/users/apache","html_url":"https://github.com/apache","followers_url":"https://api.github.com/users/apache/followers","following_url":"https://api.github.com/users/apache/following{/other_user}","gists_url":"https://api.github.com/users/apache/gists{/gist_id}","starred_url":"https://api.github.com/users/apache/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apache/subscriptions","organizations_url":"https://api.github.com/users/apache/orgs","repos_url":"https://api.github.com/users/apache/repos","events_url":"https://api.github.com/users/apache/events{/privacy}","received_events_url":"https://api.github.com/users/apache/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/apache/spark","description":"Mirror of Apache Spark","fork":false,"url":"https://api.github.com/repos/apache/spark","forks_url":"https://api.github.com/repos/apache/spark/forks","keys_url":"https://api.github.com/repos/apache/spark/keys{/key_id}","collaborators_url":"https://api.github.com/repos/apache/spark/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/apache/spark/teams","hooks_url":"https://api.github.com/repos/apache/spark/hooks","issue_events_url":"https://api.github.com/repos/apache/spark/issues/events{/number}","events_url":"https://api.github.com/repos/apache/spark/events","assignees_url":"https://api.github.com/repos/apache/spark/assignees{/user}","branches_url":"https://api.github.com/repos/apache/spark/branches{/branch}","tags_url":"https://api.github.com/repos/apache/spark/tags","blobs_url":"https://api.github.com/repos/apache/spark/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/apache/spark/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/apache/spark/git/refs{/sha}","trees_url":"https://api.github.com/repos/apache/spark/git/trees{/sha}","statuses_url":"https://api.github.com/repos/apache/spark/statuses/{sha}","languages_url":"https://api.github.com/repos/apache/spark/languages","stargazers_url":"https://api.github.com/repos/apache/spark/stargazers","contributors_url":"https://api.github.com/repos/apache/spark/contributors","subscribers_url":"https://api.github.com/repos/apache/spark/subscribers","subscription_url":"https://api.github.com/repos/apache/spark/subscription","commits_url":"https://api.github.com/repos/apache/spark/commits{/sha}","git_commits_url":"https://api.github.com/repos/apache/spark/git/commits{/sha}","comments_url":"https://api.github.com/repos/apache/spark/comments{/number}","issue_comment_url":"https://api.github.com/repos/apache/spark/issues/comments{/number}","contents_url":"https://api.github.com/repos/apache/spark/contents/{+path}","compare_url":"https://api.github.com/repos/apache/spark/compare/{base}...{head}","merges_url":"https://api.github.com/repos/apache/spark/merges","archive_url":"https://api.github.com/repos/apache/spark/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/apache/spark/downloads","issues_url":"https://api.github.com/repos/apache/spark/issues{/number}","pulls_url":"https://api.github.com/repos/apache/spark/pulls{/number}","milestones_url":"https://api.github.com/repos/apache/spark/milestones{/number}","notifications_url":"https://api.github.com/repos/apache/spark/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/apache/spark/labels{/name}","releases_url":"https://api.github.com/repos/apache/spark/releases{/id}","deployments_url":"https://api.github.com/repos/apache/spark/deployments","created_at":"2014-02-25T08:00:08Z","updated_at":"2018-08-08T16:13:09Z","pushed_at":"2018-08-08T16:59:19Z","git_url":"git://github.com/apache/spark.git","ssh_url":"git@github.com:apache/spark.git","clone_url":"https://github.com/apache/spark.git","svn_url":"https://github.com/apache/spark","homepage":null,"size":279015,"stargazers_count":18347,"watchers_count":18347,"language":"Scala","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":16611,"mirror_url":null,"archived":false,"open_issues_count":488,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":16611,"open_issues":488,"watchers":18347,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/22009"},"html":{"href":"https://github.com/apache/spark/pull/22009"},"issue":{"href":"https://api.github.com/repos/apache/spark/issues/22009"},"comments":{"href":"https://api.github.com/repos/apache/spark/issues/22009/comments"},"review_comments":{"href":"https://api.github.com/repos/apache/spark/pulls/22009/comments"},"review_comment":{"href":"https://api.github.com/repos/apache/spark/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/apache/spark/pulls/22009/commits"},"statuses":{"href":"https://api.github.com/repos/apache/spark/statuses/c22499964ac759670c3629c690f77018bc79a7c1"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"cloud-fan"}}