You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kiszk <gi...@git.apache.org> on 2017/11/18 03:19:22 UTC
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
GitHub user kiszk opened a pull request:
https://github.com/apache/spark/pull/19778
[SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem with elt
## What changes were proposed in this pull request?
This PR changes `elt` code generation to place generated code for expression for arguments into separated methods if these size could be large.
This PR resolved the case of `elt` with a lot of argument
## How was this patch tested?
Added new test cases into `StringExpressionsSuite`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kiszk/spark SPARK-22550
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19778.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 #19778
----
commit 06b103e556756d567ae45626a9a62b8ffb777e36
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Date: 2017-11-18T03:07:31Z
initial commit
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84032 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84032/testReport)** for PR 19778 at commit [`b0e9092`](https://github.com/apache/spark/commit/b0e90921099bfe9e7c45e42b84b3153599867da0).
* 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 issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84052 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84052/testReport)** for PR 19778 at commit [`3822864`](https://github.com/apache/spark/commit/38228646a64862a4537d69a9c6575a9870cc625e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19778
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #83980 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83980/testReport)** for PR 19778 at commit [`feecef0`](https://github.com/apache/spark/commit/feecef0995d55a90c3cac8c4fc5c3680319b3def).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19778
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r156569281
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -224,22 +224,52 @@ case class Elt(children: Seq[Expression])
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val index = indexExpr.genCode(ctx)
val strings = stringExprs.map(_.genCode(ctx))
+ val indexVal = ctx.freshName("index")
+ val stringVal = ctx.freshName("stringVal")
val assignStringValue = strings.zipWithIndex.map { case (eval, index) =>
s"""
case ${index + 1}:
- ${ev.value} = ${eval.isNull} ? null : ${eval.value};
+ ${eval.code}
+ $stringVal = ${eval.isNull} ? null : ${eval.value};
break;
"""
- }.mkString("\n")
- val indexVal = ctx.freshName("index")
- val stringArray = ctx.freshName("strings");
+ }
- ev.copy(index.code + "\n" + strings.map(_.code).mkString("\n") + s"""
- final int $indexVal = ${index.value};
- UTF8String ${ev.value} = null;
- switch ($indexVal) {
- $assignStringValue
+ val cases = ctx.buildCodeBlocks(assignStringValue)
+ val codes = if (cases.length == 1) {
+ s"""
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ ${cases.head}
+ }
+ """
+ } else {
+ var prevFunc = "null"
+ for (c <- cases.reverse) {
+ val funcName = ctx.freshName("eltFunc")
+ val funcBody = s"""
+ private UTF8String $funcName(InternalRow ${ctx.INPUT_ROW}, int $indexVal) {
--- End diff --
ah good catch! we should fix it with `splitExpressionsWithCurrentInputs`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19778
LGTM except one minor comment
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #83980 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83980/testReport)** for PR 19778 at commit [`feecef0`](https://github.com/apache/spark/commit/feecef0995d55a90c3cac8c4fc5c3680319b3def).
* 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 issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r152190136
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -841,6 +825,26 @@ class CodegenContext {
}
}
+ def splitCodes(expressions: Seq[String]): Seq[String] = {
--- End diff --
Sure, but it is not `private`. It is used in `stringExpressions`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84060/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #83979 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83979/testReport)** for PR 19778 at commit [`06b103e`](https://github.com/apache/spark/commit/06b103e556756d567ae45626a9a62b8ffb777e36).
* 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 issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83979/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r156624676
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -224,22 +224,52 @@ case class Elt(children: Seq[Expression])
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val index = indexExpr.genCode(ctx)
val strings = stringExprs.map(_.genCode(ctx))
+ val indexVal = ctx.freshName("index")
+ val stringVal = ctx.freshName("stringVal")
val assignStringValue = strings.zipWithIndex.map { case (eval, index) =>
s"""
case ${index + 1}:
- ${ev.value} = ${eval.isNull} ? null : ${eval.value};
+ ${eval.code}
+ $stringVal = ${eval.isNull} ? null : ${eval.value};
break;
"""
- }.mkString("\n")
- val indexVal = ctx.freshName("index")
- val stringArray = ctx.freshName("strings");
+ }
- ev.copy(index.code + "\n" + strings.map(_.code).mkString("\n") + s"""
- final int $indexVal = ${index.value};
- UTF8String ${ev.value} = null;
- switch ($indexVal) {
- $assignStringValue
+ val cases = ctx.buildCodeBlocks(assignStringValue)
+ val codes = if (cases.length == 1) {
+ s"""
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ ${cases.head}
+ }
+ """
+ } else {
+ var prevFunc = "null"
+ for (c <- cases.reverse) {
+ val funcName = ctx.freshName("eltFunc")
+ val funcBody = s"""
+ private UTF8String $funcName(InternalRow ${ctx.INPUT_ROW}, int $indexVal) {
--- End diff --
Proposes a fix in #19964.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19778
LGTM pending jenkins
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #83979 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83979/testReport)** for PR 19778 at commit [`06b103e`](https://github.com/apache/spark/commit/06b103e556756d567ae45626a9a62b8ffb777e36).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84032/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84032 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84032/testReport)** for PR 19778 at commit [`b0e9092`](https://github.com/apache/spark/commit/b0e90921099bfe9e7c45e42b84b3153599867da0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r151958049
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -224,22 +224,55 @@ case class Elt(children: Seq[Expression])
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val index = indexExpr.genCode(ctx)
val strings = stringExprs.map(_.genCode(ctx))
+ val indexVal = ctx.freshName("index")
+ val stringVal = ctx.freshName("stringVal")
val assignStringValue = strings.zipWithIndex.map { case (eval, index) =>
s"""
case ${index + 1}:
- ${ev.value} = ${eval.isNull} ? null : ${eval.value};
+ ${eval.code}
+ $stringVal = ${eval.isNull} ? null : ${eval.value};
break;
"""
- }.mkString("\n")
- val indexVal = ctx.freshName("index")
- val stringArray = ctx.freshName("strings");
+ }
- ev.copy(index.code + "\n" + strings.map(_.code).mkString("\n") + s"""
- final int $indexVal = ${index.value};
- UTF8String ${ev.value} = null;
- switch ($indexVal) {
- $assignStringValue
+ val cases = ctx.splitCodes(assignStringValue)
+ val codes = if (cases.length == 1) {
+ s"""
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ ${cases.head}
+ }
+ """
+ } else {
+ var fullFuncName = ""
+ cases.reverse.zipWithIndex.map { case (s, index) =>
--- End diff --
I'd like to make it more imperative:
```
var prevFunc = "null"
for (case <- cases) {
val funcName = ...
val funcBody = ...
prevFunc = ctx.addNewFunction
}
s"UTF8String $stringVal = $prevFunc(${ctx.INPUT_ROW}, $indexVal);"
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84052/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83980/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r152101922
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -841,6 +825,26 @@ class CodegenContext {
}
}
+ def splitCodes(expressions: Seq[String]): Seq[String] = {
--- End diff --
actually `split` is not accurate here, how about `groupCodes`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84050/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r152186300
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -841,6 +825,26 @@ class CodegenContext {
}
}
+ def splitCodes(expressions: Seq[String]): Seq[String] = {
--- End diff --
nit: private. We also need to write a comment.
How about `buildCodeBlocks`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r151955134
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -224,22 +224,55 @@ case class Elt(children: Seq[Expression])
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val index = indexExpr.genCode(ctx)
val strings = stringExprs.map(_.genCode(ctx))
+ val indexVal = ctx.freshName("index")
+ val stringVal = ctx.freshName("stringVal")
val assignStringValue = strings.zipWithIndex.map { case (eval, index) =>
s"""
case ${index + 1}:
- ${ev.value} = ${eval.isNull} ? null : ${eval.value};
+ ${eval.code}
+ $stringVal = ${eval.isNull} ? null : ${eval.value};
break;
"""
- }.mkString("\n")
- val indexVal = ctx.freshName("index")
- val stringArray = ctx.freshName("strings");
+ }
- ev.copy(index.code + "\n" + strings.map(_.code).mkString("\n") + s"""
- final int $indexVal = ${index.value};
- UTF8String ${ev.value} = null;
- switch ($indexVal) {
- $assignStringValue
+ val cases = ctx.splitCodes(assignStringValue)
+ val codes = if (cases.length == 1) {
+ s"""
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ ${cases.head}
+ }
+ """
+ } else {
+ var fullFuncName = ""
+ cases.reverse.zipWithIndex.map { case (s, index) =>
+ val prevFunc = if (index == 0) {
+ "null"
+ } else {
+ s"$fullFuncName(${ctx.INPUT_ROW}, $indexVal)"
+ }
+ val funcName = ctx.freshName("eltFunc")
+ val funcBody = s"""
+ private UTF8String $funcName(InternalRow ${ctx.INPUT_ROW}, int $indexVal) {
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ $s
+ default:
+ return $prevFunc;
+ }
+ return $stringVal;
+ }
+ """
+ fullFuncName = ctx.addNewFunction(funcName, funcBody)
}
+ s"UTF8String $stringVal = $fullFuncName(${ctx.INPUT_ROW}, ${indexVal});"
+ }
+
+ ev.copy(index.code + "\n" +
+ s"""
+ final int $indexVal = ${index.value};
--- End diff --
nit:
```
${index.code}
final int $indexVal = ${index.value};
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84052 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84052/testReport)** for PR 19778 at commit [`3822864`](https://github.com/apache/spark/commit/38228646a64862a4537d69a9c6575a9870cc625e).
* This patch **fails due to an unknown error code, -9**.
* 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 #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit pr...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/19778#discussion_r156566161
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -224,22 +224,52 @@ case class Elt(children: Seq[Expression])
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val index = indexExpr.genCode(ctx)
val strings = stringExprs.map(_.genCode(ctx))
+ val indexVal = ctx.freshName("index")
+ val stringVal = ctx.freshName("stringVal")
val assignStringValue = strings.zipWithIndex.map { case (eval, index) =>
s"""
case ${index + 1}:
- ${ev.value} = ${eval.isNull} ? null : ${eval.value};
+ ${eval.code}
+ $stringVal = ${eval.isNull} ? null : ${eval.value};
break;
"""
- }.mkString("\n")
- val indexVal = ctx.freshName("index")
- val stringArray = ctx.freshName("strings");
+ }
- ev.copy(index.code + "\n" + strings.map(_.code).mkString("\n") + s"""
- final int $indexVal = ${index.value};
- UTF8String ${ev.value} = null;
- switch ($indexVal) {
- $assignStringValue
+ val cases = ctx.buildCodeBlocks(assignStringValue)
+ val codes = if (cases.length == 1) {
+ s"""
+ UTF8String $stringVal = null;
+ switch ($indexVal) {
+ ${cases.head}
+ }
+ """
+ } else {
+ var prevFunc = "null"
+ for (c <- cases.reverse) {
+ val funcName = ctx.freshName("eltFunc")
+ val funcBody = s"""
+ private UTF8String $funcName(InternalRow ${ctx.INPUT_ROW}, int $indexVal) {
--- End diff --
Looks like this splitting doesn't prevent the case in wholestage codegen?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84060 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84060/testReport)** for PR 19778 at commit [`3822864`](https://github.com/apache/spark/commit/38228646a64862a4537d69a9c6575a9870cc625e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84050 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84050/testReport)** for PR 19778 at commit [`e70e45c`](https://github.com/apache/spark/commit/e70e45c2bb743c930cd2444e12ccbef74047cc5c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19778
thanks, merging to master/2.2
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84060 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84060/testReport)** for PR 19778 at commit [`3822864`](https://github.com/apache/spark/commit/38228646a64862a4537d69a9c6575a9870cc625e).
* 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 issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19778
**[Test build #84050 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84050/testReport)** for PR 19778 at commit [`e70e45c`](https://github.com/apache/spark/commit/e70e45c2bb743c930cd2444e12ccbef74047cc5c).
* This patch **fails due to an unknown error code, -9**.
* 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 issue #19778: [SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem w...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19778
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org