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/11/19 17:54:47 UTC
[GitHub] [spark] prakharjain09 opened a new pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
prakharjain09 opened a new pull request #30430:
URL: https://github.com/apache/spark/pull/30430
### What changes were proposed in this pull request?
This is a followup of #30302 . As part of this PR, sameOrderExpressions set is made part of children of SortOrder node - so that they don't need any special handling as done in #30302 .
### Why are the changes needed?
sameOrderExpressions should get same treatment as child. So making them part of children helps in transforming them easily.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing UTs
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734880395
**[Test build #131886 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131886/testReport)** for PR 30430 at commit [`815396e`](https://github.com/apache/spark/commit/815396e21bbe5f371c7603a46a8a59c764d6cefd).
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730541429
Can one of the admins verify this patch?
----------------------------------------------------------------
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] prakharjain09 commented on pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730540024
@cloud-fan @maropu I have made the changes as suggested in https://github.com/apache/spark/pull/30302#discussion_r526104333 .
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730826482
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35990/
Test FAILed.
----------------------------------------------------------------
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] HyukjinKwon commented on pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730767712
ok to test
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734957534
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734956857
**[Test build #131886 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131886/testReport)** for PR 30430 at commit [`815396e`](https://github.com/apache/spark/commit/815396e21bbe5f371c7603a46a8a59c764d6cefd).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnixTimestamp(`
* `case class SerdeInfo(`
* `case class FormatClasses(input: String, output: String) `
* `case class CoalesceShufflePartitions(session: SparkSession) extends CustomShuffleReaderRule `
* `trait CustomShuffleReaderRule extends Rule[SparkPlan] `
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r532434090
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -1090,6 +1090,34 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("sort order doesn't have repeated expressions") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1") {
+ withTempView("t2") {
Review comment:
nit: `withTempView("t1", "t2")`
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733801134
**[Test build #131785 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131785/testReport)** for PR 30430 at commit [`1c38e97`](https://github.com/apache/spark/commit/1c38e979a35dbaafbac013254e3bc35befde7cc8).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731188641
**[Test build #131426 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131426/testReport)** for PR 30430 at commit [`9510b5f`](https://github.com/apache/spark/commit/9510b5fcf6a91efccd522460953d4c5f6e191480).
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730826469
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35990/
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730778009
**[Test build #131386 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131386/testReport)** for PR 30430 at commit [`f3a8ded`](https://github.com/apache/spark/commit/f3a8dede84aedae100e27c8344bda4d5ffa7771f).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734860008
**[Test build #131885 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131885/testReport)** for PR 30430 at commit [`b82540e`](https://github.com/apache/spark/commit/b82540e7fbdee19d7b1215a76de09cbdf0b08793).
* This patch **fails Spark 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] maropu edited a comment on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
maropu edited a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-736514472
The GA tests passed, so I merged to master. Thanks!
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731354349
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734957534
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733801134
**[Test build #131785 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131785/testReport)** for PR 30430 at commit [`1c38e97`](https://github.com/apache/spark/commit/1c38e979a35dbaafbac013254e3bc35befde7cc8).
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730826476
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731228694
----------------------------------------------------------------
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] maropu commented on a change in pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r527336218
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
##########
@@ -64,7 +64,9 @@ case class SortOrder(
direction: SortDirection,
nullOrdering: NullOrdering,
sameOrderExpressions: Set[Expression])
- extends UnaryExpression with Unevaluable {
+ extends Expression with Unevaluable {
+
+ override def children: Seq[Expression] = child +: sameOrderExpressions.toSeq
Review comment:
nit:
```
def satisfies(required: SortOrder): Boolean = {
(sameOrderExpressions + child).exists(required.child.semanticEquals) &&
direction == required.direction && nullOrdering == required.nullOrdering
}
```
=>
```
def satisfies(required: SortOrder): Boolean = {
children.exists(required.child.semanticEquals) &&
direction == required.direction && nullOrdering == required.nullOrdering
}
```
?
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730542206
Can one of the admins verify this patch?
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734880395
**[Test build #131886 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131886/testReport)** for PR 30430 at commit [`815396e`](https://github.com/apache/spark/commit/815396e21bbe5f371c7603a46a8a59c764d6cefd).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731353115
**[Test build #131426 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131426/testReport)** for PR 30430 at commit [`9510b5f`](https://github.com/apache/spark/commit/9510b5fcf6a91efccd522460953d4c5f6e191480).
* 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] SparkQA commented on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731228669
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36032/
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733922317
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730541429
Can one of the admins verify this patch?
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730888060
**[Test build #131386 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131386/testReport)** for PR 30430 at commit [`f3a8ded`](https://github.com/apache/spark/commit/f3a8dede84aedae100e27c8344bda4d5ffa7771f).
* 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] SparkQA commented on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734854799
**[Test build #131885 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131885/testReport)** for PR 30430 at commit [`b82540e`](https://github.com/apache/spark/commit/b82540e7fbdee19d7b1215a76de09cbdf0b08793).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734902155
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734860029
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734860029
----------------------------------------------------------------
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] prakharjain09 commented on a change in pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r533380492
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -1090,6 +1090,34 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("sort order doesn't have repeated expressions") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1") {
+ withTempView("t2") {
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] maropu commented on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-736514472
GA test passed, so I merged to master. Thanks!
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733897179
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730895924
----------------------------------------------------------------
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] maropu commented on a change in pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r527335880
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
##########
@@ -64,7 +64,9 @@ case class SortOrder(
direction: SortDirection,
nullOrdering: NullOrdering,
sameOrderExpressions: Set[Expression])
Review comment:
We need to use `Set` for this variable?
----------------------------------------------------------------
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] maropu closed pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
maropu closed pull request #30430:
URL: https://github.com/apache/spark/pull/30430
----------------------------------------------------------------
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] prakharjain09 commented on a change in pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r527716040
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
##########
@@ -64,7 +64,9 @@ case class SortOrder(
direction: SortDirection,
nullOrdering: NullOrdering,
sameOrderExpressions: Set[Expression])
- extends UnaryExpression with Unevaluable {
+ extends Expression with Unevaluable {
+
+ override def children: Seq[Expression] = child +: sameOrderExpressions.toSeq
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731188641
**[Test build #131426 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131426/testReport)** for PR 30430 at commit [`9510b5f`](https://github.com/apache/spark/commit/9510b5fcf6a91efccd522460953d4c5f6e191480).
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730826476
Merged build finished. Test FAILed.
----------------------------------------------------------------
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] prakharjain09 commented on a change in pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r531583861
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/joins/SortMergeJoinExec.scala
##########
@@ -68,9 +68,9 @@ case class SortMergeJoinExec(
val leftKeyOrdering = getKeyOrdering(leftKeys, left.outputOrdering)
val rightKeyOrdering = getKeyOrdering(rightKeys, right.outputOrdering)
leftKeyOrdering.zip(rightKeyOrdering).map { case (lKey, rKey) =>
- // Also add the right key and its `sameOrderExpressions`
- SortOrder(lKey.child, Ascending, lKey.sameOrderExpressions + rKey.child ++ rKey
- .sameOrderExpressions)
+ // Also add expressions from right side sort order
+ val sameOrderExpressions = ExpressionSet(lKey.children ++ rKey.children) - lKey.child
Review comment:
I was thinking if "lKey.child" can be part of rKey.children. But that shouldn't be the case as left and right side of SMJ will always return different expressions ids (even when they work on same tables).
making suggested change 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] AmplabJenkins removed a comment on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733922317
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730819499
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35990/
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730778009
**[Test build #131386 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131386/testReport)** for PR 30430 at commit [`f3a8ded`](https://github.com/apache/spark/commit/f3a8dede84aedae100e27c8344bda4d5ffa7771f).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733916550
**[Test build #131785 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131785/testReport)** for PR 30430 at commit [`1c38e97`](https://github.com/apache/spark/commit/1c38e979a35dbaafbac013254e3bc35befde7cc8).
* This patch **fails PySpark 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] AmplabJenkins removed a comment on pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734884505
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731354349
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734902155
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731228694
----------------------------------------------------------------
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] maropu commented on pull request #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730775197
@prakharjain09 Thanks for working on this. btw, could you assign a new jira ID to this?
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-731214724
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36032/
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730895924
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r530478275
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/joins/SortMergeJoinExec.scala
##########
@@ -68,9 +68,9 @@ case class SortMergeJoinExec(
val leftKeyOrdering = getKeyOrdering(leftKeys, left.outputOrdering)
val rightKeyOrdering = getKeyOrdering(rightKeys, right.outputOrdering)
leftKeyOrdering.zip(rightKeyOrdering).map { case (lKey, rKey) =>
- // Also add the right key and its `sameOrderExpressions`
- SortOrder(lKey.child, Ascending, lKey.sameOrderExpressions + rKey.child ++ rKey
- .sameOrderExpressions)
+ // Also add expressions from right side sort order
+ val sameOrderExpressions = ExpressionSet(lKey.children ++ rKey.children) - lKey.child
Review comment:
why not `ExpressionSet(lKey.sameOrderExpressions ++ rKey.children)`?
----------------------------------------------------------------
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 #30430: [SPARK-33400][SQL][FOLLOWUP] Make sameOrderExpressions part of SortOrder childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-730542206
Can one of the admins verify this patch?
----------------------------------------------------------------
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] maropu commented on a change in pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r529086328
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
##########
@@ -64,7 +64,9 @@ case class SortOrder(
direction: SortDirection,
nullOrdering: NullOrdering,
sameOrderExpressions: Set[Expression])
Review comment:
> We can use ExpressionSet maybe at those 2 places and make it a Seq in constructor here. any thoughts?
Yea, I think it is okay to just deduplicate it before storing it in the class as you said, and then make it `Seq`.
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-733897179
----------------------------------------------------------------
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] prakharjain09 commented on a change in pull request #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30430:
URL: https://github.com/apache/spark/pull/30430#discussion_r527721715
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/SortOrder.scala
##########
@@ -64,7 +64,9 @@ case class SortOrder(
direction: SortDirection,
nullOrdering: NullOrdering,
sameOrderExpressions: Set[Expression])
Review comment:
The set property of this variable is used at couple of places in SortMergeJoin ([here](https://github.com/apache/spark/blob/3fdfce3120f307147244e5eaf46d61419a723d50/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/SortMergeJoinExec.scala#L122), [here](https://github.com/apache/spark/blob/v3.0.0/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/SortMergeJoinExec.scala#L149)).
We can use ExpressionSet maybe at those 2 places and make it a Seq in constructor here. any thoughts?
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734854799
**[Test build #131885 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131885/testReport)** for PR 30430 at commit [`b82540e`](https://github.com/apache/spark/commit/b82540e7fbdee19d7b1215a76de09cbdf0b08793).
----------------------------------------------------------------
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 #30430: [SPARK-33503][SQL] Refactor SortOrder class to allow multiple childrens
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30430:
URL: https://github.com/apache/spark/pull/30430#issuecomment-734884505
----------------------------------------------------------------
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