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 2021/05/28 07:28:25 UTC
[GitHub] [spark] Ngone51 opened a new pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Ngone51 opened a new pull request #32692:
URL: https://github.com/apache/spark/pull/32692
### What changes were proposed in this pull request?
Change the type of `DATASET_ID_TAG` from `Long` to `HashSet[Long]` to allow the logical plan to match multiple datasets.
### Why are the changes needed?
During the transformation from one Dataset to another Dataset, the DATASET_ID_TAG of logical plan won't change if the plan itself doesn't change:
https://github.com/apache/spark/blob/b5241c97b17a1139a4ff719bfce7f68aef094d95/sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala#L234-L237
However, dataset id always changes even if the logical plan doesn't change:
https://github.com/apache/spark/blob/b5241c97b17a1139a4ff719bfce7f68aef094d95/sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala#L207-L208
And this can lead to the mismatch between dataset's id and col's __dataset_id. E.g.,
```scala
test("SPARK-28344: fail ambiguous self join - Dataset.colRegex as column ref") {
// The test can fail if we change it to:
// val df1 = spark.range(3).toDF()
// val df2 = df1.filter($"id" > 0).toDF()
val df1 = spark.range(3)
val df2 = df1.filter($"id" > 0)
withSQLConf(
SQLConf.FAIL_AMBIGUOUS_SELF_JOIN_ENABLED.key -> "true",
SQLConf.CROSS_JOINS_ENABLED.key -> "true") {
assertAmbiguousSelfJoin(df1.join(df2, df1.colRegex("id") > df2.colRegex("id")))
}
}
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added unit 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] SparkQA commented on pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850218998
**[Test build #139051 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139051/testReport)** for PR 32692 at commit [`00d411d`](https://github.com/apache/spark/commit/00d411d7fc5322bd0a1af948e767e895e8ac0bda).
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850363752
**[Test build #139051 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139051/testReport)** for PR 32692 at commit [`00d411d`](https://github.com/apache/spark/commit/00d411d7fc5322bd0a1af948e767e895e8ac0bda).
* 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] AmplabJenkins commented on pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850364930
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139051/
--
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 pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850637890
+1, LGTM. Than you, @Ngone51 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] AmplabJenkins removed a comment on pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850290117
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43571/
--
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] Ngone51 commented on a change in pull request #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #32692:
URL: https://github.com/apache/spark/pull/32692#discussion_r641331217
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/AnalysisHelper.scala
##########
@@ -91,7 +91,9 @@ trait AnalysisHelper extends QueryPlan[LogicalPlan] { self: LogicalPlan =>
}
} else {
CurrentOrigin.withOrigin(origin) {
- rule.applyOrElse(afterRuleOnChildren, identity[LogicalPlan])
+ val afterRule = rule.applyOrElse(afterRuleOnChildren, identity[LogicalPlan])
+ afterRule.copyTagsFrom(self)
Review comment:
@cloud-fan @dongjoon-hyun this fixes the test failure.
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850218998
**[Test build #139051 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139051/testReport)** for PR 32692 at commit [`00d411d`](https://github.com/apache/spark/commit/00d411d7fc5322bd0a1af948e767e895e8ac0bda).
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850364930
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139051/
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32692:
URL: https://github.com/apache/spark/pull/32692
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850265697
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43571/
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850290117
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43571/
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850244526
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43571/
--
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 #32692: [SPARK-35454][SQL][3.1] One LogicalPlan can match multiple dataset ids
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32692:
URL: https://github.com/apache/spark/pull/32692#issuecomment-850404711
thanks, merging to 3.1!
--
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