You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by scwf <gi...@git.apache.org> on 2015/01/22 16:34:00 UTC
[GitHub] spark pull request: [SPARK-5367][SQL] Support star expression in u...
GitHub user scwf opened a pull request:
https://github.com/apache/spark/pull/4163
[SPARK-5367][SQL] Support star expression in udf
now spark sql does not support star expression in udf, run the following sql by spark-sql will get error
```
select concat( * ) from src
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/scwf/spark udf-star
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4163.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 #4163
----
commit eb93c1628247db169caa2581a61ac2a82cf5298d
Author: wangfei <wa...@huawei.com>
Date: 2015-01-22T09:26:35Z
fix star resolve issue in udf
commit 587bf7e951e0568f705200d452c7aa862ec6ef9d
Author: wangfei <wa...@huawei.com>
Date: 2015-01-22T09:28:21Z
compile fix
commit f87b5f90d53b93a8a6836ac9b007b8e24b1b5745
Author: scwf <wa...@huawei.com>
Date: 2015-01-22T15:32:42Z
added test case
commit da1da09f50b5adfed2965c892e9a00f792a63148
Author: scwf <wa...@huawei.com>
Date: 2015-01-22T15:33:43Z
minor fix
----
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71416813
@liancheng can you help review this?
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71039264
[Test build #25969 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25969/consoleFull) for PR 4163 at commit [`da1da09`](https://github.com/apache/spark/commit/da1da09f50b5adfed2965c892e9a00f792a63148).
* This patch merges cleanly.
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23516109
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -273,10 +279,9 @@ class Analyzer(catalog: Catalog,
case q: LogicalPlan =>
logTrace(s"Attempting to resolve ${q.simpleString}")
q transformExpressions {
- case u @ UnresolvedAttribute(name)
- if resolver(name, VirtualColumn.groupingIdName) &&
- q.isInstanceOf[GroupingAnalytics] =>
- // Resolve the virtual column GROUPING__ID for the operator GroupingAnalytics
+ case u @ UnresolvedAttribute(name) if resolver(name, VirtualColumn.groupingIdName) &&
+ q.isInstanceOf[GroupingAnalytics] =>
+ // Resolve the virtual column GROUPING__ID for the operator GroupingAnalytics
--- End diff --
yes, here just a style fix, there are 4 Indentation before if
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23515333
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -250,6 +250,12 @@ class Analyzer(catalog: Catalog,
Project(
projectList.flatMap {
case s: Star => s.expand(child.output, resolver)
+ case Alias(UnresolvedFunction(name1, clds), name2) if containsStar(clds) =>
+ val newClds = clds.flatMap {
+ case s: Star => s.expand(child.output, resolver)
+ case o => o :: Nil
+ }
+ Alias(UnresolvedFunction(name1, newClds), name2)() :: Nil
--- End diff --
How about refactoring this branch to this:
```scala
case a @ Alias(f @ UnresolvedFunction(_, args), _) if containsStar(args) =>
val expandedArgs = args.flatMap {
case s: Star => s.expand(child.output, resolver)
case o => o :: Nil
}
a.copy(child = f.copy(children = expandedArgs))() :: Nil
```
(Also, please avoid using confusing variable names like `clds`, `name1`, or `name2`).
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23515370
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -273,10 +279,9 @@ class Analyzer(catalog: Catalog,
case q: LogicalPlan =>
logTrace(s"Attempting to resolve ${q.simpleString}")
q transformExpressions {
- case u @ UnresolvedAttribute(name)
- if resolver(name, VirtualColumn.groupingIdName) &&
- q.isInstanceOf[GroupingAnalytics] =>
- // Resolve the virtual column GROUPING__ID for the operator GroupingAnalytics
+ case u @ UnresolvedAttribute(name) if resolver(name, VirtualColumn.groupingIdName) &&
+ q.isInstanceOf[GroupingAnalytics] =>
+ // Resolve the virtual column GROUPING__ID for the operator GroupingAnalytics
--- End diff --
This change seems to be unnecessary?
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71051906
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25969/
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: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71423701
Verified that Hive 0.13.1 does support using `*` in UDFs. Left some comments about styling. Thanks for fixing this!
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71535547
LGTM
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71435436
[Test build #26089 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26089/consoleFull) for PR 4163 at commit [`9db7b39`](https://github.com/apache/spark/commit/9db7b39cd3ae1e13a3911a7cbf9497230f7f5a31).
* 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 pull request: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23515282
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -299,7 +304,10 @@ class Analyzer(catalog: Catalog,
* Returns true if `exprs` contains a [[Star]].
*/
protected def containsStar(exprs: Seq[Expression]): Boolean =
- exprs.collect { case _: Star => true}.nonEmpty
+ exprs.flatMap { _ collect {
+ case s: Star => true
+ }
+ }.nonEmpty
--- End diff --
This can be both simpler and more efficient:
```scala
exprs.exists(_.collect { case _: Star => true }.nonEmpty)
```
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71051891
[Test build #25969 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25969/consoleFull) for PR 4163 at commit [`da1da09`](https://github.com/apache/spark/commit/da1da09f50b5adfed2965c892e9a00f792a63148).
* 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 pull request: [SPARK-5367][SQL] Support star expression in u...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71435443
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26089/
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: [SPARK-5367][SQL] Support star expression in u...
Posted by scwf <gi...@git.apache.org>.
Github user scwf commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23516180
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala ---
@@ -508,6 +508,11 @@ class HiveQuerySuite extends HiveComparisonTest with BeforeAndAfter {
assert(sql("select key from src having key > 490").collect().size < 100)
}
+ test("SPARK-5367: resolve star expression in udf") {
+ assert(sql("select concat(*) from src limit 5").collect().size == 5)
+ assert(sql("select array(*) from src limit 5").collect().size == 5)
--- End diff --
Actually in HiveQuerySuite we can not access checkAnswer, so just refer to other test case to use assert here.
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/4163#discussion_r23515293
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala ---
@@ -508,6 +508,11 @@ class HiveQuerySuite extends HiveComparisonTest with BeforeAndAfter {
assert(sql("select key from src having key > 490").collect().size < 100)
}
+ test("SPARK-5367: resolve star expression in udf") {
+ assert(sql("select concat(*) from src limit 5").collect().size == 5)
+ assert(sql("select array(*) from src limit 5").collect().size == 5)
--- End diff --
Would be better to check the answers with `checkAnswer`.
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-71428289
[Test build #26089 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26089/consoleFull) for PR 4163 at commit [`9db7b39`](https://github.com/apache/spark/commit/9db7b39cd3ae1e13a3911a7cbf9497230f7f5a31).
* This patch merges cleanly.
---
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: [SPARK-5367][SQL] Support star expression in u...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/4163#issuecomment-72127801
Thanks! Merged to master.
---
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