[GitHub] spark pull request: [SPARK-13242] [SQL] codegen fallback in case-w...

[davies]

Github user davies closed the pull request at:

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


---
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-13242] [SQL] codegen fallback in case-w...

[davies]

Github user davies commented on the pull request:

https://github.com/apache/spark/pull/11606#issuecomment-194481484
  
Merging this into 1.6 branch.


---
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11606#issuecomment-194470546
  
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11606#issuecomment-194470548
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52755/
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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11606#issuecomment-194470241
  
**[Test build #52755 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52755/consoleFull)**
 for PR 11606 at commit 
[`522e7bf`](https://github.com/apache/spark/commit/522e7bfbf7e0d3ee4e1f6e89334c62127d880ccc).
 * 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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11606#issuecomment-194428830
  
**[Test build #52755 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52755/consoleFull)**
 for PR 11606 at commit 
[`522e7bf`](https://github.com/apache/spark/commit/522e7bfbf7e0d3ee4e1f6e89334c62127d880ccc).


---
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-13242] [SQL] codegen fallback in case-w...

[davies]

GitHub user davies opened a pull request:

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

[SPARK-13242] [SQL] codegen fallback in case-when if there many branches

## What changes were proposed in this pull request?

If there are many branches in a CaseWhen expression, the generated code 
could go above the 64K limit for single java method, will fail to compile. This 
PR change it to fallback to interpret mode if there are more than 20 branches.

## How was this patch tested?

Add tests



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

$ git pull https://github.com/davies/spark fix_when_16

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

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


commit 522e7bfbf7e0d3ee4e1f6e89334c62127d880ccc
Author: Davies Liu 
Date:   2016-03-09T17:58:13Z

fallback for CaseWhen and CaseKeyWhen




---
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-13242] [SQL] codegen fallback in case-w...

[davies]

Github user davies commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194415975
  
Merged into master, will create another PR for 1.6 branch


---
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-13242] [SQL] codegen fallback in case-w...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194215562
  
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194215564
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52736/
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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194215281
  
**[Test build #52736 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52736/consoleFull)**
 for PR 11592 at commit 
[`330c002`](https://github.com/apache/spark/commit/330c00240bd7b807edb645e3d5dc627db67939aa).
 * 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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194181110
  
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 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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194179909
  
**[Test build #52736 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52736/consoleFull)**
 for PR 11592 at commit 
[`330c002`](https://github.com/apache/spark/commit/330c00240bd7b807edb645e3d5dc627db67939aa).


---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194112069
  
This looks pretty good. Just some minor comments on naming. It'd be great 
to be more consistent among "branch", "case", and "switch".



---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/11592#discussion_r55471524
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
 ---
@@ -58,6 +58,27 @@ class CodeGenerationSuite extends SparkFunSuite with 
ExpressionEvalHelper {
 }
   }
 
+  test("SPARK-13242: complicated case-when expressions") {
--- End diff --

SPARK-13242: case-when expression with large number of branches (or cases)?


---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/11592#discussion_r55471486
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -136,7 +136,16 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
 }
   }
 
+  def shouldCodegen: Boolean = {
+branches.length < CaseWhen.MAX_NUMBER_OF_SWITCHES
+  }
+
   override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+if (!shouldCodegen) {
+  // Fallback to interpreted mode if there are too many branches, or 
it may reach the
+  // 64K limit (number of bytecode for single Java method).
+  return super.genCode(ctx, ev)
--- End diff --

this super is slightly brittle. Can you see if there is a way in Scala to 
explicitly call genCode in CodegenFallback?


---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/11592#discussion_r55471461
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -136,7 +136,16 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
 }
   }
 
+  def shouldCodegen: Boolean = {
+branches.length < CaseWhen.MAX_NUMBER_OF_SWITCHES
+  }
+
   override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+if (!shouldCodegen) {
+  // Fallback to interpreted mode if there are too many branches, or 
it may reach the
+  // 64K limit (number of bytecode for single Java method).
--- End diff --

number of bytecode for single Java method -> limit on bytecode size for a 
single function


---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/11592#discussion_r55471445
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -136,7 +136,16 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
 }
   }
 
+  def shouldCodegen: Boolean = {
+branches.length < CaseWhen.MAX_NUMBER_OF_SWITCHES
+  }
+
   override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+if (!shouldCodegen) {
+  // Fallback to interpreted mode if there are too many branches, or 
it may reach the
--- End diff --

or -> as


---
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-13242] [SQL] codegen fallback in case-w...

[rxin]

Github user rxin commented on a diff in the pull request:

https://github.com/apache/spark/pull/11592#discussion_r55471424
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
 ---
@@ -205,6 +214,9 @@ case class CaseWhen(branches: Seq[(Expression, 
Expression)], elseValue: Option[E
 /** Factory methods for CaseWhen. */
 object CaseWhen {
 
+  // The maxium number of switches supported with codegen.
+  val MAX_NUMBER_OF_SWITCHES = 20
--- End diff --

MAX_NUM_CASES_FOR_CODEGEN


---
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194062774
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52706/
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-13242] [SQL] codegen fallback in case-w...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194062773
  
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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194062511
  
**[Test build #52706 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52706/consoleFull)**
 for PR 11592 at commit 
[`aace0a3`](https://github.com/apache/spark/commit/aace0a3a0006c2b40a3b678a5d51ca0001ecda1f).
 * 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-13242] [SQL] codegen fallback in case-w...

[davies]

Github user davies commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194032877
  
cc @joehalliwell


---
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-13242] [SQL] codegen fallback in case-w...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11592#issuecomment-194032743
  
**[Test build #52706 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52706/consoleFull)**
 for PR 11592 at commit 
[`aace0a3`](https://github.com/apache/spark/commit/aace0a3a0006c2b40a3b678a5d51ca0001ecda1f).


---
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-13242] [SQL] codegen fallback in case-w...

[davies]

GitHub user davies opened a pull request:

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

[SPARK-13242] [SQL] codegen fallback in case-when if there many branches

## What changes were proposed in this pull request?

If there are many branches in a CaseWhen expression, the generated code 
could go above the 64K limit for single java method, will fail to compile. This 
PR change it to fallback to interpret mode if there are more than 20 branches. 

## How was this patch tested?

Add a test with 50 branches.

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

$ git pull https://github.com/davies/spark fix_when

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

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


commit aace0a3a0006c2b40a3b678a5d51ca0001ecda1f
Author: Davies Liu 
Date:   2016-03-09T00:06:09Z

fallback in case-when




---
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