[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49690466
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,65 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val firstCase = {
+  val cond = branches.head._1.gen(ctx)
+  val res = branches.head._2.gen(ctx)
+  s"""
+${cond.code}
+if (!${cond.isNull} && ${cond.value}) {
+  ${res.code}
+  ${ev.isNull} = ${res.isNull};
+  ${ev.value} = ${res.value};
+}
+  """
+}
 
-val cases = branches.map { case (condition, value) =>
+val otherCases = branches.tail.map { case (condition, value) =>
   val cond = condition.gen(ctx)
   val res = value.gen(ctx)
+
   s"""
-if (!$got) {
+else {
   ${cond.code}
   if (!${cond.isNull} && ${cond.value}) {
-$got = true;
 ${res.code}
 ${ev.isNull} = ${res.isNull};
 ${ev.value} = ${res.value};
   }
-}
   """
-}.mkString("\n")
+}
+
+val cases = firstCase + otherCases.mkString("\n")
 
 val elseCase = {
   if (elseValue.isDefined) {
 val res = elseValue.get.gen(ctx)
 s"""
-if (!$got) {
-  ${res.code}
-  ${ev.isNull} = ${res.isNull};
-  ${ev.value} = ${res.value};
-}
+  else {
+${res.code}
+${ev.isNull} = ${res.isNull};
+${ev.value} = ${res.value};
+  }
 """
   } else {
 ""
   }
 }
 
+val leftBrackets = {
+  if (otherCases.length > 0) {
+(0 until otherCases.length).map("}").mkString("\n")
--- End diff --

this is just 
```
"}\n" * otherCases.length
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171542500
  
**[Test build #49378 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49378/consoleFull)**
 for PR 10737 at commit 
[`7db0b69`](https://github.com/apache/spark/commit/7db0b69b3c29f49fa3c80941d2d6729e5c848e77).
 * This patch **fails Spark unit 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171543150
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171543153
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49378/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
GitHub user rxin opened a pull request:

https://github.com/apache/spark/pull/10755

[SPARK-12771][SQL] Improve CaseWhen codegen by removing "got" variable



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rxin/spark SPARK-12771

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/10755.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 #10755


commit 392b58be10909d6834b37ed09cc49bfddf7a783f
Author: Reynold Xin 
Date:   2016-01-14T06:15:52Z

[SPARK-12771][SQL] Improve CaseWhen codegen by removing "got" variable with 
"if/else".




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/10755#issuecomment-171544400
  
@viirya I took a stab at 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread viirya
Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49696763
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,65 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val firstCase = {
+  val cond = branches.head._1.gen(ctx)
+  val res = branches.head._2.gen(ctx)
+  s"""
+${cond.code}
+if (!${cond.isNull} && ${cond.value}) {
+  ${res.code}
+  ${ev.isNull} = ${res.isNull};
+  ${ev.value} = ${res.value};
+}
+  """
+}
 
-val cases = branches.map { case (condition, value) =>
+val otherCases = branches.tail.map { case (condition, value) =>
   val cond = condition.gen(ctx)
   val res = value.gen(ctx)
+
   s"""
-if (!$got) {
+else {
   ${cond.code}
   if (!${cond.isNull} && ${cond.value}) {
-$got = true;
 ${res.code}
 ${ev.isNull} = ${res.isNull};
 ${ev.value} = ${res.value};
   }
-}
   """
-}.mkString("\n")
+}
+
+val cases = firstCase + otherCases.mkString("\n")
 
 val elseCase = {
   if (elseValue.isDefined) {
 val res = elseValue.get.gen(ctx)
 s"""
-if (!$got) {
-  ${res.code}
-  ${ev.isNull} = ${res.isNull};
-  ${ev.value} = ${res.value};
-}
+  else {
+${res.code}
+${ev.isNull} = ${res.isNull};
+${ev.value} = ${res.value};
+  }
 """
   } else {
 ""
   }
 }
 
+val leftBrackets = {
+  if (otherCases.length > 0) {
+(0 until otherCases.length).map("}").mkString("\n")
--- End diff --

ha. indeed. thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171570130
  
**[Test build #49393 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49393/consoleFull)**
 for PR 10737 at commit 
[`33b26ca`](https://github.com/apache/spark/commit/33b26cad95ede442aea4ef5c7f4405c859880546).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171592341
  
**[Test build #49393 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49393/consoleFull)**
 for PR 10737 at commit 
[`33b26ca`](https://github.com/apache/spark/commit/33b26cad95ede442aea4ef5c7f4405c859880546).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171592802
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171592803
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49393/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-14 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/10737


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171211213
  
**[Test build #49304 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49304/consoleFull)**
 for PR 10737 at commit 
[`d6f617c`](https://github.com/apache/spark/commit/d6f617c2a21ef1f353219640901d4235f45adda6).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171209982
  
Can you update this once https://github.com/apache/spark/pull/10734 is 
merged?



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread viirya
Github user viirya commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171214987
  
@rxin No problem. I will update 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171234319
  
**[Test build #49304 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49304/consoleFull)**
 for PR 10737 at commit 
[`d6f617c`](https://github.com/apache/spark/commit/d6f617c2a21ef1f353219640901d4235f45adda6).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171234698
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171234700
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49304/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171427069
  
I've merged the other one.



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread viirya
Github user viirya commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171454221
  
Ok. I will update 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171476415
  
**[Test build #49352 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49352/consoleFull)**
 for PR 10737 at commit 
[`d12df3b`](https://github.com/apache/spark/commit/d12df3b088653a5ce8b50cbeb510db60bb6414ca).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread rxin
Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49672588
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,50 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val len = branches.length
 
-val cases = branches.map { case (condition, value) =>
-  val cond = condition.gen(ctx)
-  val res = value.gen(ctx)
-  s"""
-if (!$got) {
+def genBranch(branchIndex: Int): String =
--- End diff --

this code is pretty confusing. do you think you can simplify the flow a bit?

maybe it'd help to not do this with recursion, and also have the else case 
outside (if you want to have it inside, it should be at the end rather than in 
the beginning because else is conceptually the last case, not the first)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49674518
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,50 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val len = branches.length
 
-val cases = branches.map { case (condition, value) =>
-  val cond = condition.gen(ctx)
-  val res = value.gen(ctx)
-  s"""
-if (!$got) {
+def genBranch(branchIndex: Int): String =
+  if (branchIndex >= len) {
+if (elseValue.isDefined) {
+  val res = elseValue.get.gen(ctx)
+  s"""
+${res.code}
+${ev.isNull} = ${res.isNull};
+${ev.value} = ${res.value};
+  """
+} else {
+  ""
+}
+  } else {
+val cond = branches(branchIndex)._1.gen(ctx)
+val res = branches(branchIndex)._2.gen(ctx)
+
+val currentBranch = s"""
   ${cond.code}
   if (!${cond.isNull} && ${cond.value}) {
-$got = true;
 ${res.code}
 ${ev.isNull} = ${res.isNull};
 ${ev.value} = ${res.value};
   }
-}
-  """
-}.mkString("\n")
-
-val elseCase = {
-  if (elseValue.isDefined) {
-val res = elseValue.get.gen(ctx)
-s"""
-if (!$got) {
-  ${res.code}
-  ${ev.isNull} = ${res.isNull};
-  ${ev.value} = ${res.value};
-}
 """
-  } else {
-""
+
+val moreBranch = genBranch(branchIndex + 1)
+if (moreBranch != "") {
+  s"""
+$currentBranch
+else {
+  $moreBranch
--- End diff --

is this more efficient? We will generate deep `if/else` branch then.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171494299
  
**[Test build #49352 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49352/consoleFull)**
 for PR 10737 at commit 
[`d12df3b`](https://github.com/apache/spark/commit/d12df3b088653a5ce8b50cbeb510db60bb6414ca).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:
  * `abstract class Covariance(left: Expression, right: Expression) extends 
ImperativeAggregate`
  * `case class CovSample(`
  * `case class CovPopulation(`
  * `case class CaseWhen(branches: Seq[(Expression, Expression)], 
elseValue: Option[Expression] = None)`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171494764
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171494765
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49352/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread viirya
Github user viirya commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49679817
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,50 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val len = branches.length
 
-val cases = branches.map { case (condition, value) =>
-  val cond = condition.gen(ctx)
-  val res = value.gen(ctx)
-  s"""
-if (!$got) {
+def genBranch(branchIndex: Int): String =
+  if (branchIndex >= len) {
+if (elseValue.isDefined) {
+  val res = elseValue.get.gen(ctx)
+  s"""
+${res.code}
+${ev.isNull} = ${res.isNull};
+${ev.value} = ${res.value};
+  """
+} else {
+  ""
+}
+  } else {
+val cond = branches(branchIndex)._1.gen(ctx)
+val res = branches(branchIndex)._2.gen(ctx)
+
+val currentBranch = s"""
   ${cond.code}
   if (!${cond.isNull} && ${cond.value}) {
-$got = true;
 ${res.code}
 ${ev.isNull} = ${res.isNull};
 ${ev.value} = ${res.value};
   }
-}
-  """
-}.mkString("\n")
-
-val elseCase = {
-  if (elseValue.isDefined) {
-val res = elseValue.get.gen(ctx)
-s"""
-if (!$got) {
-  ${res.code}
-  ${ev.isNull} = ${res.isNull};
-  ${ev.value} = ${res.value};
-}
 """
-  } else {
-""
+
+val moreBranch = genBranch(branchIndex + 1)
+if (moreBranch != "") {
+  s"""
+$currentBranch
+else {
+  $moreBranch
--- End diff --

It will be slightly more efficient when we succeeds in front conditions and 
can skip checking of `got` variable for late branches. Before that we still 
need to check `got` content many times even first condition is already 
satisfied.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171507693
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171507696
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49366/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread viirya
Github user viirya commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171508876
  
retest this please.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171512219
  
**[Test build #49368 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49368/consoleFull)**
 for PR 10737 at commit 
[`82de80e`](https://github.com/apache/spark/commit/82de80e586810bb5ba95dd3c4699c921a62d9401).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171522126
  
**[Test build #49368 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49368/consoleFull)**
 for PR 10737 at commit 
[`82de80e`](https://github.com/apache/spark/commit/82de80e586810bb5ba95dd3c4699c921a62d9401).
 * This patch **fails Spark unit 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171522231
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171522232
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49368/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10737#discussion_r49687059
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -137,45 +137,50 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
   }
 
   override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): 
String = {
-val got = ctx.freshName("got")
+val len = branches.length
 
-val cases = branches.map { case (condition, value) =>
-  val cond = condition.gen(ctx)
-  val res = value.gen(ctx)
-  s"""
-if (!$got) {
+def genBranch(branchIndex: Int): String =
+  if (branchIndex >= len) {
+if (elseValue.isDefined) {
+  val res = elseValue.get.gen(ctx)
+  s"""
+${res.code}
+${ev.isNull} = ${res.isNull};
+${ev.value} = ${res.value};
+  """
+} else {
+  ""
+}
+  } else {
+val cond = branches(branchIndex)._1.gen(ctx)
+val res = branches(branchIndex)._2.gen(ctx)
+
+val currentBranch = s"""
   ${cond.code}
   if (!${cond.isNull} && ${cond.value}) {
-$got = true;
 ${res.code}
 ${ev.isNull} = ${res.isNull};
 ${ev.value} = ${res.value};
   }
-}
-  """
-}.mkString("\n")
-
-val elseCase = {
-  if (elseValue.isDefined) {
-val res = elseValue.get.gen(ctx)
-s"""
-if (!$got) {
-  ${res.code}
-  ${ev.isNull} = ${res.isNull};
-  ${ev.value} = ${res.value};
-}
 """
-  } else {
-""
+
+val moreBranch = genBranch(branchIndex + 1)
+if (moreBranch != "") {
+  s"""
+$currentBranch
+else {
+  $moreBranch
--- End diff --

ah makes sense, thx!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-12771][SQL] Improve CaseWhen codegen by...

2016-01-13 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10737#issuecomment-171528969
  
**[Test build #49378 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49378/consoleFull)**
 for PR 10737 at commit 
[`7db0b69`](https://github.com/apache/spark/commit/7db0b69b3c29f49fa3c80941d2d6729e5c848e77).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org