{"action":"created","comment":{"url":"https://api.github.com/repos/apache/spark/pulls/comments/166030958","pull_request_review_id":94056781,"id":166030958,"diff_hunk":"@@ -17,17 +17,151 @@\n \n package org.apache.spark.sql.execution.datasources.v2\n \n+import java.util.UUID\n+\n+import scala.collection.JavaConverters._\n+import scala.collection.mutable\n+\n+import org.apache.spark.sql.{AnalysisException, SaveMode}\n+import org.apache.spark.sql.catalyst.TableIdentifier\n import org.apache.spark.sql.catalyst.analysis.MultiInstanceRelation\n-import org.apache.spark.sql.catalyst.expressions.AttributeReference\n-import org.apache.spark.sql.catalyst.plans.logical.{LeafNode, Statistics}\n-import org.apache.spark.sql.sources.v2.reader._\n+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, Expression}\n+import org.apache.spark.sql.catalyst.plans.logical.{LeafNode, LogicalPlan, Statistics}\n+import org.apache.spark.sql.execution.datasources.DataSourceStrategy\n+import org.apache.spark.sql.sources.{DataSourceRegister, Filter}\n+import org.apache.spark.sql.sources.v2.{DataSourceOptions, DataSourceV2, ReadSupport, ReadSupportWithSchema, WriteSupport}\n+import org.apache.spark.sql.sources.v2.reader.{DataSourceReader, SupportsPushDownCatalystFilters, SupportsPushDownFilters, SupportsPushDownRequiredColumns, SupportsReportStatistics}\n+import org.apache.spark.sql.sources.v2.writer.DataSourceWriter\n+import org.apache.spark.sql.types.StructType\n \n case class DataSourceV2Relation(\n-    fullOutput: Seq[AttributeReference],\n-    reader: DataSourceReader)\n-  extends LeafNode with MultiInstanceRelation with DataSourceReaderHolder {\n+    source: DataSourceV2,\n+    options: Map[String, String],\n+    path: Option[String] = None,\n+    table: Option[TableIdentifier] = None,","path":"sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala","position":31,"original_position":31,"commit_id":"f1d9872a2699cdbd5c87b02e702dc8103335131d","original_commit_id":"f1d9872a2699cdbd5c87b02e702dc8103335131d","user":{"login":"rdblue","id":87915,"avatar_url":"https://avatars1.githubusercontent.com/u/87915?v=4","gravatar_id":"","url":"https://api.github.com/users/rdblue","html_url":"https://github.com/rdblue","followers_url":"https://api.github.com/users/rdblue/followers","following_url":"https://api.github.com/users/rdblue/following{/other_user}","gists_url":"https://api.github.com/users/rdblue/gists{/gist_id}","starred_url":"https://api.github.com/users/rdblue/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rdblue/subscriptions","organizations_url":"https://api.github.com/users/rdblue/orgs","repos_url":"https://api.github.com/users/rdblue/repos","events_url":"https://api.github.com/users/rdblue/events{/privacy}","received_events_url":"https://api.github.com/users/rdblue/received_events","type":"User","site_admin":false},"body":"@cloud-fan, sorry if it was not clear: Yes, I have considered it and I think it is a bad idea. I sent a note to the dev list about this issue, as well if you want more context. There are two main reasons:\r\n\r\n1. Your proposal creates more places that are responsible for creating a `DataSourceOptions` with the right property names. All of the places where we have a `TableIdentifier` and want to convert to a `DataSourceV2Relation` need to copy the same logic and worry about using the same properties.\r\n    What you propose is hard to maintain and error prone: what happens if we decide not to pass the database if it is `None` in a `TableIdentifier`? We would have to validate every place that creates a v2 relation. On the other hand, if we pass `TableIdentifier` here, we have one code path that converts. It is also easier for us to pass `TableIdentifier` to the data sources if we choose to update the API.\r\n2. There is no reason to use `DataSourceOptions` outside of v2 at this point. This PR doesn't expose the v2-specific options class to other places in the codebase. Instead, it uses a map for generic options and classes that can be used in pattern matching where possible. And again, this has fewer places that create v2 internal classes, which is easier for maintenance.\r\n\r\nIf you want to add those methods to the options class so that implementations can easily access path and table name, then we can do that in a follow-up PR.","created_at":"2018-02-05T16:39:57Z","updated_at":"2018-02-05T16:39:57Z","html_url":"https://github.com/apache/spark/pull/20387#discussion_r166030958","pull_request_url":"https://api.github.com/repos/apache/spark/pulls/20387","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/comments/166030958"},"html":{"href":"https://github.com/apache/spark/pull/20387#discussion_r166030958"},"pull_request":{"href":"https://api.github.com/repos/apache/spark/pulls/20387"}},"in_reply_to_id":165981421},"pull_request":{"url":"https://api.github.com/repos/apache/spark/pulls/20387","id":164934304,"html_url":"https://github.com/apache/spark/pull/20387","diff_url":"https://github.com/apache/spark/pull/20387.diff","patch_url":"https://github.com/apache/spark/pull/20387.patch","issue_url":"https://api.github.com/repos/apache/spark/issues/20387","number":20387,"state":"open","locked":false,"title":"[SPARK-23203][SQL]: DataSourceV2: Use immutable logical plans.","user":{"login":"rdblue","id":87915,"avatar_url":"https://avatars1.githubusercontent.com/u/87915?v=4","gravatar_id":"","url":"https://api.github.com/users/rdblue","html_url":"https://github.com/rdblue","followers_url":"https://api.github.com/users/rdblue/followers","following_url":"https://api.github.com/users/rdblue/following{/other_user}","gists_url":"https://api.github.com/users/rdblue/gists{/gist_id}","starred_url":"https://api.github.com/users/rdblue/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rdblue/subscriptions","organizations_url":"https://api.github.com/users/rdblue/orgs","repos_url":"https://api.github.com/users/rdblue/repos","events_url":"https://api.github.com/users/rdblue/events{/privacy}","received_events_url":"https://api.github.com/users/rdblue/received_events","type":"User","site_admin":false},"body":"## What changes were proposed in this pull request?\r\n\r\nSPARK-23203: DataSourceV2 should use immutable catalyst trees instead of wrapping a mutable DataSourceV2Reader. This commit updates DataSourceV2Relation and consolidates much of the DataSourceV2 API requirements for the read path in it. Instead of wrapping a reader that changes, the relation lazily produces a reader from its configuration.\r\n\r\nThis commit also updates the predicate and projection push-down. Instead of the implementation from SPARK-22197, this reuses the rule matching from the Hive and DataSource read paths (using `PhysicalOperation`) and copies most of the implementation of `SparkPlanner.pruneFilterProject`, with updates for DataSourceV2. By reusing the implementation from other read paths, this should have fewer regressions from other read paths and is less code to maintain.\r\n\r\nThe new push-down rules also supports the following edge cases:\r\n\r\n* The output of DataSourceV2Relation should be what is returned by the reader, in case the reader can only partially satisfy the requested schema projection\r\n* The requested projection passed to the DataSourceV2Reader should include filter columns\r\n* The push-down rule may be run more than once if filters are not pushed through projections\r\n\r\n## How was this patch tested?\r\n\r\nExisting push-down and read tests.","created_at":"2018-01-24T19:00:19Z","updated_at":"2018-02-05T16:39:57Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f66f85b2e0bea132a03385bf2a03d03ff320c0e8","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"milestone":null,"commits_url":"https://api.github.com/repos/apache/spark/pulls/20387/commits","review_comments_url":"https://api.github.com/repos/apache/spark/pulls/20387/comments","review_comment_url":"https://api.github.com/repos/apache/spark/pulls/comments{/number}","comments_url":"https://api.github.com/repos/apache/spark/issues/20387/comments","statuses_url":"https://api.github.com/repos/apache/spark/statuses/f1d9872a2699cdbd5c87b02e702dc8103335131d","head":{"label":"rdblue:SPARK-22386-push-down-immutable-trees","ref":"SPARK-22386-push-down-immutable-trees","sha":"f1d9872a2699cdbd5c87b02e702dc8103335131d","user":{"login":"rdblue","id":87915,"avatar_url":"https://avatars1.githubusercontent.com/u/87915?v=4","gravatar_id":"","url":"https://api.github.com/users/rdblue","html_url":"https://github.com/rdblue","followers_url":"https://api.github.com/users/rdblue/followers","following_url":"https://api.github.com/users/rdblue/following{/other_user}","gists_url":"https://api.github.com/users/rdblue/gists{/gist_id}","starred_url":"https://api.github.com/users/rdblue/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rdblue/subscriptions","organizations_url":"https://api.github.com/users/rdblue/orgs","repos_url":"https://api.github.com/users/rdblue/repos","events_url":"https://api.github.com/users/rdblue/events{/privacy}","received_events_url":"https://api.github.com/users/rdblue/received_events","type":"User","site_admin":false},"repo":{"id":26284539,"name":"spark","full_name":"rdblue/spark","owner":{"login":"rdblue","id":87915,"avatar_url":"https://avatars1.githubusercontent.com/u/87915?v=4","gravatar_id":"","url":"https://api.github.com/users/rdblue","html_url":"https://github.com/rdblue","followers_url":"https://api.github.com/users/rdblue/followers","following_url":"https://api.github.com/users/rdblue/following{/other_user}","gists_url":"https://api.github.com/users/rdblue/gists{/gist_id}","starred_url":"https://api.github.com/users/rdblue/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rdblue/subscriptions","organizations_url":"https://api.github.com/users/rdblue/orgs","repos_url":"https://api.github.com/users/rdblue/repos","events_url":"https://api.github.com/users/rdblue/events{/privacy}","received_events_url":"https://api.github.com/users/rdblue/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rdblue/spark","description":"Mirror of Apache Spark","fork":true,"url":"https://api.github.com/repos/rdblue/spark","forks_url":"https://api.github.com/repos/rdblue/spark/forks","keys_url":"https://api.github.com/repos/rdblue/spark/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rdblue/spark/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rdblue/spark/teams","hooks_url":"https://api.github.com/repos/rdblue/spark/hooks","issue_events_url":"https://api.github.com/repos/rdblue/spark/issues/events{/number}","events_url":"https://api.github.com/repos/rdblue/spark/events","assignees_url":"https://api.github.com/repos/rdblue/spark/assignees{/user}","branches_url":"https://api.github.com/repos/rdblue/spark/branches{/branch}","tags_url":"https://api.github.com/repos/rdblue/spark/tags","blobs_url":"https://api.github.com/repos/rdblue/spark/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rdblue/spark/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rdblue/spark/git/refs{/sha}","trees_url":"https://api.github.com/repos/rdblue/spark/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rdblue/spark/statuses/{sha}","languages_url":"https://api.github.com/repos/rdblue/spark/languages","stargazers_url":"https://api.github.com/repos/rdblue/spark/stargazers","contributors_url":"https://api.github.com/repos/rdblue/spark/contributors","subscribers_url":"https://api.github.com/repos/rdblue/spark/subscribers","subscription_url":"https://api.github.com/repos/rdblue/spark/subscription","commits_url":"https://api.github.com/repos/rdblue/spark/commits{/sha}","git_commits_url":"https://api.github.com/repos/rdblue/spark/git/commits{/sha}","comments_url":"https://api.github.com/repos/rdblue/spark/comments{/number}","issue_comment_url":"https://api.github.com/repos/rdblue/spark/issues/comments{/number}","contents_url":"https://api.github.com/repos/rdblue/spark/contents/{+path}","compare_url":"https://api.github.com/repos/rdblue/spark/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rdblue/spark/merges","archive_url":"https://api.github.com/repos/rdblue/spark/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rdblue/spark/downloads","issues_url":"https://api.github.com/repos/rdblue/spark/issues{/number}","pulls_url":"https://api.github.com/repos/rdblue/spark/pulls{/number}","milestones_url":"https://api.github.com/repos/rdblue/spark/milestones{/number}","notifications_url":"https://api.github.com/repos/rdblue/spark/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rdblue/spark/labels{/name}","releases_url":"https://api.github.com/repos/rdblue/spark/releases{/id}","deployments_url":"https://api.github.com/repos/rdblue/spark/deployments","created_at":"2014-11-06T18:50:45Z","updated_at":"2017-09-13T03:31:54Z","pushed_at":"2018-02-02T22:22:42Z","git_url":"git://github.com/rdblue/spark.git","ssh_url":"git@github.com:rdblue/spark.git","clone_url":"https://github.com/rdblue/spark.git","svn_url":"https://github.com/rdblue/spark","homepage":null,"size":245465,"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":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"apache:master","ref":"master","sha":"eefec93d193d43d5b71b8f8a4b1060286da971dd","user":{"login":"apache","id":47359,"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,"name":"spark","full_name":"apache/spark","owner":{"login":"apache","id":47359,"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-02-05T16:22:28Z","pushed_at":"2018-02-05T14:35:28Z","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":267463,"stargazers_count":16062,"watchers_count":16062,"language":"Scala","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":14871,"mirror_url":null,"archived":false,"open_issues_count":415,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":14871,"open_issues":415,"watchers":16062,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/20387"},"html":{"href":"https://github.com/apache/spark/pull/20387"},"issue":{"href":"https://api.github.com/repos/apache/spark/issues/20387"},"comments":{"href":"https://api.github.com/repos/apache/spark/issues/20387/comments"},"review_comments":{"href":"https://api.github.com/repos/apache/spark/pulls/20387/comments"},"review_comment":{"href":"https://api.github.com/repos/apache/spark/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/apache/spark/pulls/20387/commits"},"statuses":{"href":"https://api.github.com/repos/apache/spark/statuses/f1d9872a2699cdbd5c87b02e702dc8103335131d"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"rdblue"}}