You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2018/10/13 18:59:04 UTC
[GitHub] spark pull request #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to ot...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/22715
[SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopyArgs
## What changes were proposed in this pull request?
Add `outputOrdering ` to `otherCopyArgs` so that this field will be copied when we doing the tree transformation.
## How was this patch tested?
N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark copyArgs1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22715.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22715
----
commit 1d30cc53850e1455cef8586bbc83ab4257d47f89
Author: gatorsmile <ga...@...>
Date: 2018-10-13T18:55:56Z
fix
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22715
**[Test build #97346 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97346/testReport)** for PR 22715 at commit [`1d30cc5`](https://github.com/apache/spark/commit/1d30cc53850e1455cef8586bbc83ab4257d47f89).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22715
thanks for this fix @gatorsmile. Can we add a UT for this? Moreover, shall we add it also to `LogicalRDD `?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22715
**[Test build #97346 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97346/testReport)** for PR 22715 at commit [`1d30cc5`](https://github.com/apache/spark/commit/1d30cc53850e1455cef8586bbc83ab4257d47f89).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22715
**[Test build #97350 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97350/testReport)** for PR 22715 at commit [`cff78e9`](https://github.com/apache/spark/commit/cff78e992b91ae81f3b08f421919d81c45c244cc).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3948/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22715
@mgaido91 Anything is missing in LogicalRDD?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22715: [SPARK-25727][SQL] Add outputOrdering to otherCop...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22715
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22715: [SPARK-25727][SQL] Add outputOrdering to otherCop...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/22715#discussion_r225024084
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala ---
@@ -206,7 +206,7 @@ case class InMemoryRelation(
outputOrdering).asInstanceOf[this.type]
}
- override protected def otherCopyArgs: Seq[AnyRef] = Seq(statsOfPlanToCache)
+ override protected def otherCopyArgs: Seq[AnyRef] = Seq(statsOfPlanToCache, outputOrdering)
--- End diff --
The thing I don't understand is why we put the `outputOrdering` in the curry constructor at the first place...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22715
Merged to master/branch-2.4.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22715
**[Test build #97350 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97350/testReport)** for PR 22715 at commit [`cff78e9`](https://github.com/apache/spark/commit/cff78e992b91ae81f3b08f421919d81c45c244cc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/22715
Thank you, @gatorsmile and @mgaido91 .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3951/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22715
sorry @gatorsmile that is fine, my bad. A late LGTM to this, despite probably @cloud-fan 's comment make, we probably should have just put it as a part of the main constructor...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97346/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-23375][FOLLOW-UP] Add outputOrdering to otherCopy...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22715
Hi @mgaido91 , since you are the major author of this part, do you have time to open a PR and move `outputOrdering` to the main constructor? Thanks in advance!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22715
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97350/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22715: [SPARK-25727][SQL] Add outputOrdering to otherCopyArgs i...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22715
@cloud-fan , sure, I'll submit a follow-up PR for this. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org