spark-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ues...@apache.org
Subject spark git commit: [SPARK-25058][SQL] Use Block.isEmpty/nonEmpty to check whether the code is empty or not.
Date Thu, 09 Aug 2018 05:06:35 GMT
Repository: spark
Updated Branches:
  refs/heads/master a40806d2b -> 519e03d82


[SPARK-25058][SQL] Use Block.isEmpty/nonEmpty to check whether the code is empty or not.

## What changes were proposed in this pull request?

We should use `Block.isEmpty/nonEmpty` instead of comparing with empty string to check whether
the code is empty or not.

```
[error] [warn] /.../sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala:278:
org.apache.spark.sql.catalyst.expressions.codegen.Block and String are unrelated: they will
most likely always compare unequal
[error] [warn]       if (ev.code != "" && required.contains(attributes(i))) {
[error] [warn]
[error] [warn] /.../sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala:323:
org.apache.spark.sql.catalyst.expressions.codegen.Block and String are unrelated: they will
most likely never compare equal
[error] [warn]          |  ${buildVars.filter(_.code == "").map(v => s"${v.isNull} = true;").mkString("\n")}
[error] [warn]
```

## How was this patch tested?

Existing tests.

Closes #22041 from ueshin/issues/SPARK-25058/fix_comparison.

Authored-by: Takuya UESHIN <ueshin@databricks.com>
Signed-off-by: Takuya UESHIN <ueshin@databricks.com>


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/519e03d8
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/519e03d8
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/519e03d8

Branch: refs/heads/master
Commit: 519e03d82e52d2d948f3ef25f5b85cc54bb11a75
Parents: a40806d
Author: Takuya UESHIN <ueshin@databricks.com>
Authored: Thu Aug 9 14:06:28 2018 +0900
Committer: Takuya UESHIN <ueshin@databricks.com>
Committed: Thu Aug 9 14:06:28 2018 +0900

----------------------------------------------------------------------
 .../apache/spark/sql/catalyst/expressions/codegen/javaCode.scala | 4 +++-
 .../org/apache/spark/sql/execution/WholeStageCodegenExec.scala   | 2 +-
 .../apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/519e03d8/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
index 2f8c853..558cbfa 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala
@@ -130,7 +130,9 @@ trait Block extends TreeNode[Block] with JavaCode {
 
   def length: Int = toString.length
 
-  def nonEmpty: Boolean = toString.nonEmpty
+  def isEmpty: Boolean = toString.isEmpty
+
+  def nonEmpty: Boolean = !isEmpty
 
   // The leading prefix that should be stripped from each line.
   // By default we strip blanks or control characters followed by '|' from the line.

http://git-wip-us.apache.org/repos/asf/spark/blob/519e03d8/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala
index 372dc3d..80f886e 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala
@@ -275,7 +275,7 @@ trait CodegenSupport extends SparkPlan {
       required: AttributeSet): String = {
     val evaluateVars = new StringBuilder
     variables.zipWithIndex.foreach { case (ev, i) =>
-      if (ev.code != "" && required.contains(attributes(i))) {
+      if (ev.code.nonEmpty && required.contains(attributes(i))) {
         evaluateVars.append(ev.code.toString + "\n")
         ev.code = EmptyBlock
       }

http://git-wip-us.apache.org/repos/asf/spark/blob/519e03d8/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala
index 0da0e86..a6f3ea4 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastHashJoinExec.scala
@@ -320,7 +320,7 @@ case class BroadcastHashJoinExec(
          |if (!$conditionPassed) {
          |  $matched = null;
          |  // reset the variables those are already evaluated.
-         |  ${buildVars.filter(_.code == "").map(v => s"${v.isNull} = true;").mkString("\n")}
+         |  ${buildVars.filter(_.code.isEmpty).map(v => s"${v.isNull} = true;").mkString("\n")}
          |}
          |$numOutput.add(1);
          |${consume(ctx, resultVars)}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org


Mime
View raw message