Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Mar 9, 2016

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.

This PR is based on #11243 and #11221, thanks to @joehalliwell

Closes #11243
Closes #11221

How was this patch tested?

Add a test with 50 branches.

@davies
Copy link
Contributor Author

davies commented Mar 9, 2016

cc @joehalliwell

@SparkQA
Copy link

SparkQA commented Mar 9, 2016

Test build #52706 has finished for PR 11592 at commit aace0a3.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

object CaseWhen {

// The maxium number of switches supported with codegen.
val MAX_NUMBER_OF_SWITCHES = 20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAX_NUM_CASES_FOR_CODEGEN

@rxin
Copy link
Contributor

rxin commented Mar 9, 2016

This looks pretty good. Just some minor comments on naming. It'd be great to be more consistent among "branch", "case", and "switch".

@rxin
Copy link
Contributor

rxin commented Mar 9, 2016

LGTM

@SparkQA
Copy link

SparkQA commented Mar 9, 2016

Test build #52736 has finished for PR 11592 at commit 330c002.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@asfgit asfgit closed this in 9634e17 Mar 9, 2016
@davies
Copy link
Contributor Author

davies commented Mar 9, 2016

Merged into master, will create another PR for 1.6 branch

roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
## 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.

This PR is based on apache#11243 and apache#11221, thanks to joehalliwell

Closes apache#11243
Closes apache#11221

## How was this patch tested?

Add a test with 50 branches.

Author: Davies Liu <[email protected]>

Closes apache#11592 from davies/fix_when.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants