You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgaido91 <gi...@git.apache.org> on 2017/11/26 13:30:22 UTC
[GitHub] spark pull request #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeEx...
GitHub user mgaido91 opened a pull request:
https://github.com/apache/spark/pull/19822
[SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when possible
## What changes were proposed in this pull request?
There are several places in the code where `CodeGeneration.nullSafeExec` could be used, but it is not. This makes the generated code containing a lot of useless:
```
if (!false) {
// some code here
}
```
This PR use `CodeGeneration.nullSafeExec` where it is possible, also for consistency and coherency of the code.
## How was this patch tested?
Existing UTs
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mgaido91/spark SPARK-22609
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19822.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 #19822
----
commit 830793e75a43ec119be520f20e4565259cd11d71
Author: Marco Gaido <ma...@gmail.com>
Date: 2017-11-26T11:22:42Z
[SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when possible
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeEx...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19822#discussion_r153080840
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ScalaUDF.scala ---
@@ -1050,17 +1050,17 @@ case class ScalaUDF(
throw new org.apache.spark.SparkException($scalaUDF.udfErrorMessage(), e);
}
"""
-
+ val nullSafeCode = ctx.nullSafeExec(nullable, ev.isNull) {
+ s"${ev.value} = $resultTerm;"
+ }
ev.copy(code = s"""
$evalCode
${converters.mkString("\n")}
$callFunc
boolean ${ev.isNull} = $resultTerm == null;
${ctx.javaType(dataType)} ${ev.value} = ${ctx.defaultValue(dataType)};
- if (!${ev.isNull}) {
- ${ev.value} = $resultTerm;
- }""")
+ $nullSafeCode""")
--- End diff --
I do not like this parameter name. It is meaningless.
Maybe you can close this PR? It does not help either code readability or performance
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19822
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84192/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/19822
For such generated code:
```scala
if (!false) {
// some code here
}
```scala
I think the compiler optimization can simplify it. So it should not be a problem.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/19822
@viirya yes, you are right. My goal here is consistency and coherency throughout the code. There is no performance improvement.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19822
**[Test build #84192 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84192/testReport)** for PR 19822 at commit [`830793e`](https://github.com/apache/spark/commit/830793e75a43ec119be520f20e4565259cd11d71).
* 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 #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeEx...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 closed the pull request at:
https://github.com/apache/spark/pull/19822
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19822
**[Test build #84192 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84192/testReport)** for PR 19822 at commit [`830793e`](https://github.com/apache/spark/commit/830793e75a43ec119be520f20e4565259cd11d71).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19822: [SPARK-22609][SQL] Reuse CodeGenerator.nullSafeExec when...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19822
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org