You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/12/16 15:31:58 UTC
[GitHub] [spark] aokolnychyi opened a new pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
aokolnychyi opened a new pull request #30806:
URL: https://github.com/apache/spark/pull/30806
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR adds logic to build logical writes introduced in SPARK-33779.
Note: This PR contains a subset of changes discussed in PR #29066.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
These changes are the next step as discussed in the [design doc](https://docs.google.com/document/d/1X0NsQSryvNmXBY9kcvfINeYyKC-AahZarUqg3nS1GQs/edit#) for SPARK-23889.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Existing tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746492358
cc @dongjoon-hyun @dbtsai @sunchao @rdblue @viirya @cloud-fan @HyukjinKwon @HeartSaVioR @holdenk
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747118795
**[Test build #132900 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132900/testReport)** for PR 30806 at commit [`db6abdb`](https://github.com/apache/spark/commit/db6abdb6783758f11d216034992ca8bfcf3c310e).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546767165
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -65,7 +66,8 @@ case class AppendData(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
override def withNewQuery(newQuery: LogicalPlan): AppendData = copy(query = newQuery)
override def withNewTable(newTable: NamedRelation): AppendData = copy(table = newTable)
Review comment:
Sounds like a good idea but we actually construct the `Write` object in the optimizer after the operator optimization is done to ensure we operate on optimal expressions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749272123
**[Test build #133186 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133186/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748926424
Retest this, please.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544469508
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
I'm interested to hear what @dongjoon-hyun thinks about this.
I think we should have a different physical node for each write so that the explain plan shows what is happening. Otherwise, the approach to support building the batch write here or building it in the optimizer was mainly to be able to turn this on and off in our environment. I doubt that is needed in other situations.
I think I would be for removing all of the `buildAndRun` methods and always building the write in the optimizer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544401511
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -89,6 +90,21 @@ sealed trait V1FallbackWriters extends V2CommandExec with SupportsV1Write {
def table: SupportsWrite
def writeOptions: CaseInsensitiveStringMap
+ def refreshCache: () => Unit
+ def write: Option[V1Write] = None
Review comment:
Same here: `Option[V1Write]` can become just `V1Write` if we are OK to apply the new logic all the time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545200535
##########
File path: sql/core/src/main/java/org/apache/spark/sql/connector/write/V1WriteBuilder.java
##########
@@ -33,6 +33,11 @@
*/
@Unstable
public interface V1WriteBuilder extends WriteBuilder {
Review comment:
I think that should simplify the V1 fallback API. I'll update it, @cloud-fan.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746560237
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132896/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746483025
I added @rdblue as a co-author of the change as he addressed the initial comments to my change and I included his changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544396081
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -94,7 +96,8 @@ case class OverwriteByExpression(
deleteExpr: Expression,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
Making this optional allows us to reuse the same plan before we construct a write and after. Having `None` here means the logical write hasn't been constructed yet. This allows us to have idempotent rules in the optimizer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545371602
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala
##########
@@ -477,9 +450,10 @@ private[v2] trait TableWriteExecHelper extends V2TableWriteExec with SupportsV1W
writeOptions)
val writeBuilder = table.newWriteBuilder(info)
- val writtenRows = writeBuilder match {
- case v1: V1WriteBuilder => writeWithV1(v1.buildForV1Write())
- case v2 => writeWithV2(v2.buildForBatch())
+ val write = writeBuilder.build()
+ val writtenRows = write match {
Review comment:
I don't feel strongly about this place and can update it. However, I do prefer to split different logical parts into different variables. Here, I've separated building a logical write from actually writing the records. Let me know what are you thoughts, @jzhuge.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala
##########
@@ -477,9 +450,10 @@ private[v2] trait TableWriteExecHelper extends V2TableWriteExec with SupportsV1W
writeOptions)
val writeBuilder = table.newWriteBuilder(info)
- val writtenRows = writeBuilder match {
- case v1: V1WriteBuilder => writeWithV1(v1.buildForV1Write())
- case v2 => writeWithV2(v2.buildForBatch())
+ val write = writeBuilder.build()
+ val writtenRows = write match {
Review comment:
I don't feel strongly about this place and can update it. However, I do prefer to split different logical parts into different variables. Here, I've separated building a logical write from actually writing the records. Let me know what are your thoughts, @jzhuge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746750867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37498/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544398504
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.map(_.asInstanceOf[V1Write])) :: Nil
Review comment:
I think there is one open point we need to discuss: do we want to always apply the new logic or should we expose a feature flag and construct logical writes only if the flag is enabled? I'd vote for always constructing writes using the new logic as it feels quite same and it does not have the burden of maintaining one more config. In addition, this will allow us to simply this PR a bit and get rid of optional writes in exec nodes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749262103
Retest this please.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747682913
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jzhuge commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
jzhuge commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545257760
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
Review comment:
Good idea to add more meaningful exception here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748928097
**[Test build #133150 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133150/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jzhuge commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
jzhuge commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545379871
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala
##########
@@ -477,9 +450,10 @@ private[v2] trait TableWriteExecHelper extends V2TableWriteExec with SupportsV1W
writeOptions)
val writeBuilder = table.newWriteBuilder(info)
- val writtenRows = writeBuilder match {
- case v1: V1WriteBuilder => writeWithV1(v1.buildForV1Write())
- case v2 => writeWithV2(v2.buildForBatch())
+ val write = writeBuilder.build()
+ val writtenRows = write match {
Review comment:
Fine with me
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749318124
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133186/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747682913
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132965/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746937803
**[Test build #132900 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132900/testReport)** for PR 30806 at commit [`db6abdb`](https://github.com/apache/spark/commit/db6abdb6783758f11d216034992ca8bfcf3c310e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544479918
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -115,14 +131,10 @@ sealed trait V1FallbackWriters extends V2CommandExec with SupportsV1Write {
trait SupportsV1Write extends SparkPlan {
def plan: LogicalPlan
- protected def writeWithV1(
- relation: InsertableRelation,
- refreshCache: () => Unit = () => ()): Seq[InternalRow] = {
+ protected def writeWithV1(relation: InsertableRelation): Seq[InternalRow] = {
val session = sqlContext.sparkSession
// The `plan` is already optimized, we should not analyze and optimize it again.
relation.insert(AlreadyOptimized.dataFrame(session, plan), overwrite = false)
- refreshCache()
Review comment:
cc @sunchao
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749261461
**[Test build #133168 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133168/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
* This patch **fails from timeout after a configured wait of `500m`**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746551529
**[Test build #132896 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132896/testReport)** for PR 30806 at commit [`ce334cc`](https://github.com/apache/spark/commit/ce334cc48c8ea615a2465a9111fd0d9f6f606eb2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748933099
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747676591
**[Test build #132965 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132965/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544502180
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
Getting rid of `buildAndRun` would also ensure we don't have to maintain the same logic in two places.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544508365
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
I'm also +1 on getting rid of `buildAndRun`.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -115,14 +131,10 @@ sealed trait V1FallbackWriters extends V2CommandExec with SupportsV1Write {
trait SupportsV1Write extends SparkPlan {
def plan: LogicalPlan
- protected def writeWithV1(
- relation: InsertableRelation,
- refreshCache: () => Unit = () => ()): Seq[InternalRow] = {
+ protected def writeWithV1(relation: InsertableRelation): Seq[InternalRow] = {
val session = sqlContext.sparkSession
// The `plan` is already optimized, we should not analyze and optimize it again.
relation.insert(AlreadyOptimized.dataFrame(session, plan), overwrite = false)
- refreshCache()
Review comment:
lgtm
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747682879
**[Test build #132965 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132965/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
* This patch **fails MiMa tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749271208
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133168/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546737454
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -65,7 +66,8 @@ case class AppendData(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
override def withNewQuery(newQuery: LogicalPlan): AppendData = copy(query = newQuery)
override def withNewTable(newTable: NamedRelation): AppendData = copy(table = newTable)
Review comment:
Shall we add `override lazy val resolved = ... && write.isDefined` in `V2WriteCommand`? It's safer to make sure that the analyzer creates the `Write` object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746560142
**[Test build #132896 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132896/testReport)** for PR 30806 at commit [`ce334cc`](https://github.com/apache/spark/commit/ce334cc48c8ea615a2465a9111fd0d9f6f606eb2).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait V2ExistingTableWriteExec extends V2TableWriteExec `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749032280
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37759/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749099161
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37767/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746750867
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37498/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748975567
**[Test build #133160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133160/testReport)** for PR 30806 at commit [`84241e0`](https://github.com/apache/spark/commit/84241e0ab1c35a5333447202573a8eea8244660e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748933099
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133150/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545372125
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
Review comment:
I've updated this place. Could you take a look, @jzhuge and @cloud-fan?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545372950
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,33 +189,42 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
- r.table.asWritable match {
- case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
- case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
+ _, Some(write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
+ write match {
+ case v1Write: V1Write =>
+ AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r), v1Write) :: Nil
+ case v2Write =>
+ throw new AnalysisException(
+ s"Table ${v1.name} declares ${TableCapability.V1_BATCH_WRITE} capability but " +
+ s"${v2Write.getClass} is not an instance of ${classOf[V1Write]}")
}
- case OverwriteByExpression(r: DataSourceV2Relation, deleteExpr, query, writeOptions, _) =>
- // fail if any filter cannot be converted. correctness depends on removing all matching data.
Review comment:
I removed the filter conversion as it is done earlier.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,33 +189,42 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
- r.table.asWritable match {
- case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
- case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
+ _, Some(write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
+ write match {
+ case v1Write: V1Write =>
+ AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r), v1Write) :: Nil
+ case v2Write =>
+ throw new AnalysisException(
+ s"Table ${v1.name} declares ${TableCapability.V1_BATCH_WRITE} capability but " +
+ s"${v2Write.getClass} is not an instance of ${classOf[V1Write]}")
}
- case OverwriteByExpression(r: DataSourceV2Relation, deleteExpr, query, writeOptions, _) =>
- // fail if any filter cannot be converted. correctness depends on removing all matching data.
Review comment:
I removed the filter conversion as it is done earlier now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545374031
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2Writes.scala
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources.v2
+
+import java.util.UUID
+
+import org.apache.spark.SparkException
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.expressions.PredicateHelper
+import org.apache.spark.sql.catalyst.plans.logical.{AppendData, LogicalPlan, OverwriteByExpression, OverwritePartitionsDynamic}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.connector.catalog.Table
+import org.apache.spark.sql.connector.write.{LogicalWriteInfoImpl, SupportsDynamicOverwrite, SupportsOverwrite, SupportsTruncate, WriteBuilder}
+import org.apache.spark.sql.execution.datasources.DataSourceStrategy
+import org.apache.spark.sql.sources.{AlwaysTrue, Filter}
+
+/**
+ * A rule that constructs logical writes.
+ */
+object V2Writes extends Rule[LogicalPlan] with PredicateHelper {
+
+ import DataSourceV2Implicits._
+
+ override def apply(plan: LogicalPlan): LogicalPlan = plan transformDown {
+ case a @ AppendData(r: DataSourceV2Relation, query, options, _, None) =>
+ val writeBuilder = newWriteBuilder(r.table, query, options)
+ val write = writeBuilder.build()
+ a.copy(write = Some(write))
+
+ case o @ OverwriteByExpression(r: DataSourceV2Relation, deleteExpr, query, options, _, None) =>
+ // fail if any filter cannot be converted. correctness depends on removing all matching data.
+ val filters = splitConjunctivePredicates(deleteExpr).flatMap { pred =>
Review comment:
I kept the old logic but I am not sure whether we should also normalize filters. Thoughts, @cloud-fan @rdblue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749038447
**[Test build #133168 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133168/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746560237
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132896/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546773592
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -195,33 +196,42 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
- r.table.asWritable match {
- case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
- case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
+ _, Some(write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
+ write match {
+ case v1Write: V1Write =>
+ AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r), v1Write) :: Nil
+ case v2Write =>
+ throw new AnalysisException(
+ s"Table ${v1.name} declares ${TableCapability.V1_BATCH_WRITE} capability but " +
+ s"${v2Write.getClass} is not an instance of ${classOf[V1Write]}")
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749413913
thanks, merging to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jzhuge commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
jzhuge commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544832653
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.get.asInstanceOf[V1Write]) :: Nil
Review comment:
Possible to avoid instance cast? See my suggestion above.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
Review comment:
Is `write` guaranteed not be `None`?
How about rewriting this case as follows?
```
case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
_, Some(v1Write: V1Write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r), v1Write) :: Nil
case AppendData(r @ DataSourceV2Relation(v2: SupportsWrite, _, _, _, _),
query, writeOptions, _, Some(write)) =>
AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r), write) :: Nil
```
It is not exactly the same as the existing code. Some unmatched cases (not sure how many or if any) will fall through. Exception will be thrown later, instead of right here upon instance cast or Option.get.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala
##########
@@ -477,9 +450,10 @@ private[v2] trait TableWriteExecHelper extends V2TableWriteExec with SupportsV1W
writeOptions)
val writeBuilder = table.newWriteBuilder(info)
- val writtenRows = writeBuilder match {
- case v1: V1WriteBuilder => writeWithV1(v1.buildForV1Write())
- case v2 => writeWithV2(v2.buildForBatch())
+ val write = writeBuilder.build()
+ val writtenRows = write match {
Review comment:
Nit: merge line 451-454 into:
```
val writtenRows = table.newWriteBuilder(info).build() match {
```
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -115,14 +80,10 @@ sealed trait V1FallbackWriters extends V2CommandExec with SupportsV1Write {
trait SupportsV1Write extends SparkPlan {
def plan: LogicalPlan
- protected def writeWithV1(
- relation: InsertableRelation,
- refreshCache: () => Unit = () => ()): Seq[InternalRow] = {
+ protected def writeWithV1(relation: InsertableRelation): Seq[InternalRow] = {
Review comment:
Nicely simplified
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.get.asInstanceOf[V1Write]) :: Nil
case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ AppendDataExec(
+ v2, writeOptions.asOptions, planLater(query),
+ refreshCache(r), write.get) :: Nil
Review comment:
Possible to avoid Option.get? See my suggestion above.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746551529
**[Test build #132896 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132896/testReport)** for PR 30806 at commit [`ce334cc`](https://github.com/apache/spark/commit/ce334cc48c8ea615a2465a9111fd0d9f6f606eb2).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746643363
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37498/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545059579
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
Review comment:
+1 to this idea. It's guaranteed that the `write` will be `Some` not `None` at the planner, so matching `Some(write)` is better.
It's possible that the implementation declares `V1_BATCH_WRITE` but doesn't return `V1Write`. We should give clear error message if it happens:
```
case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
_, Some(write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
if (!write.isInstanceOf[V1Write]) throw ...
...
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749308839
**[Test build #133186 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133186/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546769770
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2Writes.scala
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources.v2
+
+import java.util.UUID
+
+import org.apache.spark.SparkException
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.expressions.PredicateHelper
+import org.apache.spark.sql.catalyst.plans.logical.{AppendData, LogicalPlan, OverwriteByExpression, OverwritePartitionsDynamic}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.connector.catalog.Table
+import org.apache.spark.sql.connector.write.{LogicalWriteInfoImpl, SupportsDynamicOverwrite, SupportsOverwrite, SupportsTruncate, WriteBuilder}
+import org.apache.spark.sql.execution.datasources.DataSourceStrategy
+import org.apache.spark.sql.sources.{AlwaysTrue, Filter}
+
+/**
+ * A rule that constructs logical writes.
+ */
+object V2Writes extends Rule[LogicalPlan] with PredicateHelper {
+
+ import DataSourceV2Implicits._
+
+ override def apply(plan: LogicalPlan): LogicalPlan = plan transformDown {
+ case a @ AppendData(r: DataSourceV2Relation, query, options, _, None) =>
+ val writeBuilder = newWriteBuilder(r.table, query, options)
+ val write = writeBuilder.build()
+ a.copy(write = Some(write))
+
+ case o @ OverwriteByExpression(r: DataSourceV2Relation, deleteExpr, query, options, _, None) =>
+ // fail if any filter cannot be converted. correctness depends on removing all matching data.
+ val filters = splitConjunctivePredicates(deleteExpr).flatMap { pred =>
Review comment:
I've created SPARK-33868 as a follow-up item. I will keep the old behavior in this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748933035
**[Test build #133150 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133150/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
* This patch **fails MiMa tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748975567
**[Test build #133160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133160/testReport)** for PR 30806 at commit [`84241e0`](https://github.com/apache/spark/commit/84241e0ab1c35a5333447202573a8eea8244660e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747701132
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37568/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749291357
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37784/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747032742
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37502/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi edited a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi edited a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746483025
I added @rdblue as a co-author of the change as he addressed the initial comments to my PR and I included his changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544401805
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -115,14 +131,10 @@ sealed trait V1FallbackWriters extends V2CommandExec with SupportsV1Write {
trait SupportsV1Write extends SparkPlan {
def plan: LogicalPlan
- protected def writeWithV1(
- relation: InsertableRelation,
- refreshCache: () => Unit = () => ()): Seq[InternalRow] = {
+ protected def writeWithV1(relation: InsertableRelation): Seq[InternalRow] = {
val session = sqlContext.sparkSession
// The `plan` is already optimized, we should not analyze and optimize it again.
relation.insert(AlreadyOptimized.dataFrame(session, plan), overwrite = false)
- refreshCache()
Review comment:
Refresh moved to `run`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748997495
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37759/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r547163439
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -132,7 +135,8 @@ case class OverwritePartitionsDynamic(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
Quick question: `DataSourceV2Relation` is also used inside write nodes like `AppendData`. If we add an optional scan, will that mean we will leak a read-specific concept into write plans?
cc @rdblue @cloud-fan
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746563694
Hm, the failure is a bit weird and does not seem related.
```
Setting status of ce334cc48c8ea615a2465a9111fd0d9f6f606eb2 to FAILURE with url https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132896/ and message: 'Build finished. '
FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or the user account does not have write access to the repo.
org.kohsuke.github.GHFileNotFoundException: https://api.github.com/repos/apache/spark/statuses/ce334cc48c8ea615a2465a9111fd0d9f6f606eb2 {"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/repos#create-a-commit-status"}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jzhuge commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
jzhuge commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545383545
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
Review comment:
Looks great! Thanks for taking care of `Overwrite*` cases as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749291373
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37784/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546851543
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -132,7 +135,8 @@ case class OverwritePartitionsDynamic(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
I think that's a good idea.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544544159
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkOptimizer.scala
##########
@@ -39,6 +39,9 @@ class SparkOptimizer(
// TODO: move SchemaPruning into catalyst
SchemaPruning :: V2ScanRelationPushDown :: PruneFileSourcePartitions :: Nil
+ override def dataSourceRewriteRules: Seq[Rule[LogicalPlan]] =
+ V2Writes :: Nil
Review comment:
Ok, it makes sense after thinking more about it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546734400
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -195,33 +196,42 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
- r.table.asWritable match {
- case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
- case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ case AppendData(r @ DataSourceV2Relation(v1: SupportsWrite, _, _, _, _), query, writeOptions,
+ _, Some(write)) if v1.supports(TableCapability.V1_BATCH_WRITE) =>
+ write match {
+ case v1Write: V1Write =>
+ AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r), v1Write) :: Nil
+ case v2Write =>
+ throw new AnalysisException(
+ s"Table ${v1.name} declares ${TableCapability.V1_BATCH_WRITE} capability but " +
+ s"${v2Write.getClass} is not an instance of ${classOf[V1Write]}")
Review comment:
`classOf[V1Write].getName`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746937803
**[Test build #132900 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132900/testReport)** for PR 30806 at commit [`db6abdb`](https://github.com/apache/spark/commit/db6abdb6783758f11d216034992ca8bfcf3c310e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747676591
**[Test build #132965 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132965/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544399834
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.map(_.asInstanceOf[V1Write])) :: Nil
Review comment:
I'd be curious about what everybody thinks here. I will be okay either way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544400986
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
We can get rid of all `buildAndRun` methods if we are OK to apply the new logic all the time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747119458
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132900/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545371860
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.get.asInstanceOf[V1Write]) :: Nil
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748980699
**[Test build #133160 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133160/testReport)** for PR 30806 at commit [`84241e0`](https://github.com/apache/spark/commit/84241e0ab1c35a5333447202573a8eea8244660e).
* This patch **fails MiMa tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749272123
**[Test build #133186 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133186/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r552686257
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -132,7 +135,8 @@ case class OverwritePartitionsDynamic(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
For `AppendData`, we intentionally do not treat the `table` as a child, which means the pushdown rule won't apply for it and the `Scan` object will always be None in `AppendData`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749032280
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37759/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544466723
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkOptimizer.scala
##########
@@ -39,6 +39,9 @@ class SparkOptimizer(
// TODO: move SchemaPruning into catalyst
SchemaPruning :: V2ScanRelationPushDown :: PruneFileSourcePartitions :: Nil
+ override def dataSourceRewriteRules: Seq[Rule[LogicalPlan]] =
+ V2Writes :: Nil
Review comment:
I would probably put this in the early pushdown batch, even though the name doesn't match. The rewrite batch needs to run before this so that writes created by it run `V2Writes` afterward. That's the same reason why early pushdown runs after plan rewrites.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545369598
##########
File path: sql/core/src/main/java/org/apache/spark/sql/connector/write/V1WriteBuilder.java
##########
@@ -33,6 +33,11 @@
*/
@Unstable
public interface V1WriteBuilder extends WriteBuilder {
Review comment:
Got rid of `V1WriteBuilder` and tried to update docs too. Let me know if I missed places, @cloud-fan.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545371765
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -188,15 +189,20 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
orCreate = orCreate) :: Nil
}
- case AppendData(r: DataSourceV2Relation, query, writeOptions, _) =>
+ case AppendData(r: DataSourceV2Relation, query, writeOptions, _, write) =>
r.table.asWritable match {
case v1 if v1.supports(TableCapability.V1_BATCH_WRITE) =>
- AppendDataExecV1(v1, writeOptions.asOptions, query, refreshCache(r)) :: Nil
+ AppendDataExecV1(
+ v1, writeOptions.asOptions, query,
+ refreshCache(r), write.get.asInstanceOf[V1Write]) :: Nil
case v2 =>
- AppendDataExec(v2, writeOptions.asOptions, planLater(query), refreshCache(r)) :: Nil
+ AppendDataExec(
+ v2, writeOptions.asOptions, planLater(query),
+ refreshCache(r), write.get) :: Nil
Review comment:
Got rid of it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747021316
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37502/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r547163439
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -132,7 +135,8 @@ case class OverwritePartitionsDynamic(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
Quick question: `DataSourceV2Relation` is also used inside write nodes like `AppendData`. If we add an optional scan, will that mean we will leak a read-specific concept into write plans?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748980743
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/133160/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749439400
Thanks @cloud-fan @jzhuge @rdblue @sunchao @dongjoon-hyun!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546776855
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -132,7 +135,8 @@ case class OverwritePartitionsDynamic(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
Review comment:
Not related to this PR. I'm thinking that if we should have an optional `Scan` object in `DataSourceV2Relation`, instead of having a new logical plan `DataSourceV2ScanRelation`. It's simpler and consistent with the write logical plans. cc @rdblue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546735790
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2Writes.scala
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources.v2
+
+import java.util.UUID
+
+import org.apache.spark.SparkException
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.expressions.PredicateHelper
+import org.apache.spark.sql.catalyst.plans.logical.{AppendData, LogicalPlan, OverwriteByExpression, OverwritePartitionsDynamic}
+import org.apache.spark.sql.catalyst.rules.Rule
+import org.apache.spark.sql.connector.catalog.Table
+import org.apache.spark.sql.connector.write.{LogicalWriteInfoImpl, SupportsDynamicOverwrite, SupportsOverwrite, SupportsTruncate, WriteBuilder}
+import org.apache.spark.sql.execution.datasources.DataSourceStrategy
+import org.apache.spark.sql.sources.{AlwaysTrue, Filter}
+
+/**
+ * A rule that constructs logical writes.
+ */
+object V2Writes extends Rule[LogicalPlan] with PredicateHelper {
+
+ import DataSourceV2Implicits._
+
+ override def apply(plan: LogicalPlan): LogicalPlan = plan transformDown {
+ case a @ AppendData(r: DataSourceV2Relation, query, options, _, None) =>
+ val writeBuilder = newWriteBuilder(r.table, query, options)
+ val write = writeBuilder.build()
+ a.copy(write = Some(write))
+
+ case o @ OverwriteByExpression(r: DataSourceV2Relation, deleteExpr, query, options, _, None) =>
+ // fail if any filter cannot be converted. correctness depends on removing all matching data.
+ val filters = splitConjunctivePredicates(deleteExpr).flatMap { pred =>
Review comment:
I think we should, to follow DS v1 and file source.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30806:
URL: https://github.com/apache/spark/pull/30806
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r546775417
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -65,7 +66,8 @@ case class AppendData(
table: NamedRelation,
query: LogicalPlan,
writeOptions: Map[String, String],
- isByName: Boolean) extends V2WriteCommand {
+ isByName: Boolean,
+ write: Option[Write] = None) extends V2WriteCommand {
override def withNewQuery(newQuery: LogicalPlan): AppendData = copy(query = newQuery)
override def withNewTable(newTable: NamedRelation): AppendData = copy(table = newTable)
Review comment:
ah I see, let's leave it then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544519633
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
It sounds like an intermediate consensus, I'll update the PR but we can revisit this once we have more input from others.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747119458
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/132900/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748955479
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37750/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747733941
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37568/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747716786
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37568/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-746708530
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37498/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r544485842
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V1FallbackWriters.scala
##########
@@ -38,10 +38,11 @@ case class AppendDataExecV1(
table: SupportsWrite,
writeOptions: CaseInsensitiveStringMap,
plan: LogicalPlan,
- refreshCache: () => Unit) extends V1FallbackWriters {
+ refreshCache: () => Unit,
+ override val write: Option[V1Write] = None) extends V1FallbackWriters {
- override protected def run(): Seq[InternalRow] = {
- writeWithV1(newWriteBuilder().buildForV1Write(), refreshCache = refreshCache)
+ override protected def buildAndRun(): Seq[InternalRow] = {
Review comment:
Does it cause many code change on top of this? If it is not intrusive, it sounds reasonable in that context. I'd give +1 for the direction, @rdblue .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749283631
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37784/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747032815
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37502/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749038447
**[Test build #133168 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133168/testReport)** for PR 30806 at commit [`882e321`](https://github.com/apache/spark/commit/882e3210004c0e27aca914b58819008dafdde92a).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-747032815
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/37502/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748955466
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37750/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748953440
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37750/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] aokolnychyi commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748969298
I think certain checks are expected to fail:
```
[error] spark-sql: Failed binary compatibility check against org.apache.spark:spark-sql_2.12:3.0.0! Found 1 potential problems (filtered 914)
[error] * interface org.apache.spark.sql.connector.write.V1WriteBuilder does not have a correspondent in current version
[error] filter with: ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.sql.connector.write.V1WriteBuilder")
```
Per discussion [here](https://github.com/apache/spark/pull/30806#discussion_r545052456).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30806:
URL: https://github.com/apache/spark/pull/30806#discussion_r545052456
##########
File path: sql/core/src/main/java/org/apache/spark/sql/connector/write/V1WriteBuilder.java
##########
@@ -33,6 +33,11 @@
*/
@Unstable
public interface V1WriteBuilder extends WriteBuilder {
Review comment:
This is an unstable API, can we just remove it and only use `V1Write`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749003647
We need to update `project/MimaExcludes.scala` to fix the mima errors.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749099161
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-748928097
**[Test build #133150 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/133150/testReport)** for PR 30806 at commit [`e6335c0`](https://github.com/apache/spark/commit/e6335c066680a0ce1409d6a926bf0b73422ed0a6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30806: [SPARK-33808][SQL] DataSource V2: Build logical writes in the optimizer
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30806:
URL: https://github.com/apache/spark/pull/30806#issuecomment-749016610
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37759/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org