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 2017/11/21 05:53:49 UTC
[GitHub] spark pull request #19790: [SPARK-22569] [SQL] Clean usage of addMutableStat...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/19790
[SPARK-22569] [SQL] Clean usage of addMutableState and splitExpressions
## What changes were proposed in this pull request?
This PR is to clean the usage of addMutableState and splitExpressions
## How was this patch tested?
The existing test cases
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark codeClean
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19790.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 #19790
----
commit a0272d95ab51aca685de8cdd6320a29a9a595139
Author: gatorsmile <ga...@gmail.com>
Date: 2017-11-21T05:46:17Z
clean
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19790
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19790
LGTM, can you improve the PR description to describe the major change? e.g.
```
1. replace hardcoded type string to ctx.JAVA_BOOLEAN etc.
2. create a default value of the initCode for ctx.addMutableStats
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19790
**[Test build #84045 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84045/testReport)** for PR 19790 at commit [`a0272d9`](https://github.com/apache/spark/commit/a0272d95ab51aca685de8cdd6320a29a9a595139).
* This patch **fails Spark unit 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 #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19790
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84057/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19790
**[Test build #84045 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84045/testReport)** for PR 19790 at commit [`a0272d9`](https://github.com/apache/spark/commit/a0272d95ab51aca685de8cdd6320a29a9a595139).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19790
thanks, merging to master!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19790
**[Test build #84057 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84057/testReport)** for PR 19790 at commit [`a5ee80b`](https://github.com/apache/spark/commit/a5ee80bc9abf7d4ab84c0a389d06f4685d917754).
* 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 pull request #19790: [SPARK-22569] [SQL] Clean usage of addMutableStat...
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/19790#discussion_r152195602
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -205,27 +209,32 @@ case class ConcatWs(children: Seq[Expression])
}.unzip
val codes = ctx.splitExpressions(ctx.INPUT_ROW, evals.map(_.code))
- val varargCounts = ctx.splitExpressions(varargCount, "varargCountsConcatWs",
- ("InternalRow", ctx.INPUT_ROW) :: Nil,
- "int",
- { body =>
+ val varargCounts = ctx.splitExpressions(
+ expressions = varargCount,
+ funcName = "varargCountsConcatWs",
+ arguments = ("InternalRow", ctx.INPUT_ROW) :: Nil,
+ returnType = "int",
+ makeSplitFunction = { body =>
s"""
int $varargNum = 0;
$body
return $varargNum;
"""
},
- _.mkString(s"$varargNum += ", s";\n$varargNum += ", ";"))
- val varargBuilds = ctx.splitExpressions(varargBuild, "varargBuildsConcatWs",
- ("InternalRow", ctx.INPUT_ROW) :: ("UTF8String []", array) :: ("int", idxInVararg) :: Nil,
- "int",
- { body =>
+ foldFunctions = _.mkString(s"$varargNum += ", s";\n$varargNum += ", ";"))
+ val varargBuilds = ctx.splitExpressions(
+ expressions = varargBuild,
+ funcName = "varargBuildsConcatWs",
+ arguments =
+ ("InternalRow", ctx.INPUT_ROW) :: ("UTF8String []", array) :: ("int", idxInVararg) :: Nil,
+ returnType = "int",
+ makeSplitFunction = { body =>
--- End diff --
nit: is it possible to eliminate the `{ }`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19790: [SPARK-22569] [SQL] Clean usage of addMutableStat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19790#discussion_r152200760
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -205,27 +209,32 @@ case class ConcatWs(children: Seq[Expression])
}.unzip
val codes = ctx.splitExpressions(ctx.INPUT_ROW, evals.map(_.code))
- val varargCounts = ctx.splitExpressions(varargCount, "varargCountsConcatWs",
- ("InternalRow", ctx.INPUT_ROW) :: Nil,
- "int",
- { body =>
+ val varargCounts = ctx.splitExpressions(
+ expressions = varargCount,
+ funcName = "varargCountsConcatWs",
+ arguments = ("InternalRow", ctx.INPUT_ROW) :: Nil,
+ returnType = "int",
+ makeSplitFunction = { body =>
s"""
int $varargNum = 0;
$body
return $varargNum;
"""
},
- _.mkString(s"$varargNum += ", s";\n$varargNum += ", ";"))
- val varargBuilds = ctx.splitExpressions(varargBuild, "varargBuildsConcatWs",
- ("InternalRow", ctx.INPUT_ROW) :: ("UTF8String []", array) :: ("int", idxInVararg) :: Nil,
- "int",
- { body =>
+ foldFunctions = _.mkString(s"$varargNum += ", s";\n$varargNum += ", ";"))
+ val varargBuilds = ctx.splitExpressions(
+ expressions = varargBuild,
+ funcName = "varargBuildsConcatWs",
+ arguments =
+ ("InternalRow", ctx.INPUT_ROW) :: ("UTF8String []", array) :: ("int", idxInVararg) :: Nil,
+ returnType = "int",
+ makeSplitFunction = { body =>
--- End diff --
done.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19790
**[Test build #84057 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84057/testReport)** for PR 19790 at commit [`a5ee80b`](https://github.com/apache/spark/commit/a5ee80bc9abf7d4ab84c0a389d06f4685d917754).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19790
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84045/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19790: [SPARK-22569] [SQL] Clean usage of addMutableStat...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19790
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19790
cc @cloud-fan @kiszk
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19790: [SPARK-22569] [SQL] Clean usage of addMutableState and s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19790
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org