You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by baishuo <gi...@git.apache.org> on 2014/07/07 04:27:46 UTC
[GitHub] spark pull request: Update MultiInstanceRelation.scala
GitHub user baishuo opened a pull request:
https://github.com/apache/spark/pull/1312
Update MultiInstanceRelation.scala
I think if multiAppearance is empty,we can return plan directly
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/baishuo/spark test-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1312.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 #1312
----
commit b779773356ba475085ea425a1c8a23048b13fd4f
Author: baishuo(白硕) <vc...@hotmail.com>
Date: 2014-07-07T02:26:43Z
Update MultiInstanceRelation.scala
I think if multiAppearance is empty,we can return plan directly
----
---
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.
---
[GitHub] spark pull request: [SQL]Update MultiInstanceRelation.scala
Posted by baishuo <gi...@git.apache.org>.
Github user baishuo commented on the pull request:
https://github.com/apache/spark/pull/1312#issuecomment-48352678
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.
---
[GitHub] spark pull request: Update MultiInstanceRelation.scala
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1312#issuecomment-48136093
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.
---
[GitHub] spark pull request: [SQL]Update MultiInstanceRelation.scala
Posted by baishuo <gi...@git.apache.org>.
Github user baishuo commented on the pull request:
https://github.com/apache/spark/pull/1312#issuecomment-48421247
thank you @marmbrus , close this PR
---
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.
---
[GitHub] spark pull request: [SQL]Update MultiInstanceRelation.scala
Posted by baishuo <gi...@git.apache.org>.
Github user baishuo closed the pull request at:
https://github.com/apache/spark/pull/1312
---
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.
---
[GitHub] spark pull request: [SQL]Update MultiInstanceRelation.scala
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/1312#issuecomment-48372461
Also, for future reference there are several stylistic issues:
- Space between `if` and `(`
- Spaces `} else {`
- Space between `)` and `{`
---
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.
---
[GitHub] spark pull request: [SQL]Update MultiInstanceRelation.scala
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/1312#issuecomment-48372204
I think this patch is correct, but I'm not convinced that we need to make these kind of optimizations at the expense of code complexity / length. I'd imagine the performance benefit of this change is negligible and AFAICT it produces the same result as the original code.
---
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.
---