You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yangw1234 <gi...@git.apache.org> on 2016/06/10 14:35:40 UTC
[GitHub] spark pull request #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonE...
GitHub user yangw1234 opened a pull request:
https://github.com/apache/spark/pull/13601
[SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty instead of Seq.length == 0 and Seq.length > 0
## What changes were proposed in this pull request?
In scala, immutable.List.length is an expensive operation so we should
avoid using Seq.length == 0 and Seq.lenth > 0 and use Seq.isEmpty and Seq.nonEmpty instead.
## How was this patch tested?
existing tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yangw1234/spark isEmpty
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13601.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 #13601
----
commit 3fa193942fabd03e2ac35f9fb27dc862931a5690
Author: wangyang <wa...@haizhi.com>
Date: 2016-06-10T14:25:22Z
avoid using seq.length
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13601
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonE...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/13601
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13601
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonE...
Posted by yangw1234 <gi...@git.apache.org>.
Github user yangw1234 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13601#discussion_r66630258
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1656,7 +1656,7 @@ class Analyzer(
// We do a final check and see if we only have a single Window Spec defined in an
// expressions.
- if (distinctWindowSpec.length == 0 ) {
+ if (distinctWindowSpec.isEmpty ) {
--- End diff --
@srowen fixed it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13601
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60301/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the issue:
https://github.com/apache/spark/pull/13601
Jenkins, this is ok to test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonE...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13601#discussion_r66624159
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1656,7 +1656,7 @@ class Analyzer(
// We do a final check and see if we only have a single Window Spec defined in an
// expressions.
- if (distinctWindowSpec.length == 0 ) {
+ if (distinctWindowSpec.isEmpty ) {
--- End diff --
LGTM. Tiny nit, if you have to make another edit: remove space inside paren
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13601
**[Test build #60301 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60301/consoleFull)** for PR 13601 at commit [`f6b331c`](https://github.com/apache/spark/commit/f6b331c5154b104a1c5f3b218644789cc33ea4fe).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13601
**[Test build #60301 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60301/consoleFull)** for PR 13601 at commit [`f6b331c`](https://github.com/apache/spark/commit/f6b331c5154b104a1c5f3b218644789cc33ea4fe).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13601: [SPARK-15875] Try to use Seq.isEmpty and Seq.nonEmpty in...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/13601
Merging in master/2.0. I'm not sure whether any of these cases are actually perf sensitive though. If they are, they probably shouldn't be using Seq anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org