[SPARK-22550][SQL] Fix 64KB JVM bytecode limit problem with elt #19778
Conversation
|
Test build #83979 has finished for PR 19778 at commit
|
|
Test build #83980 has finished for PR 19778 at commit
|
|
|
||
| ev.copy(index.code + "\n" + | ||
| s""" | ||
| final int $indexVal = ${index.value}; |
There was a problem hiding this comment.
nit:
${index.code}
final int $indexVal = ${index.value};
...
| """ | ||
| } else { | ||
| var fullFuncName = "" | ||
| cases.reverse.zipWithIndex.map { case (s, index) => |
There was a problem hiding this comment.
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);"
|
Test build #84032 has finished for PR 19778 at commit
|
| } | ||
| } | ||
|
|
||
| def splitCodes(expressions: Seq[String]): Seq[String] = { |
There was a problem hiding this comment.
actually split is not accurate here, how about groupCodes?
There was a problem hiding this comment.
nit: private. We also need to write a comment.
How about buildCodeBlocks?
There was a problem hiding this comment.
Sure, but it is not private. It is used in stringExpressions.
|
LGTM except one minor comment |
|
LGTM pending jenkins |
|
Test build #84052 has finished for PR 19778 at commit
|
|
Test build #84050 has finished for PR 19778 at commit
|
|
retest this please |
|
Test build #84060 has finished for PR 19778 at commit
|
|
thanks, merging to master/2.2 |
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 Added new test cases into `StringExpressionsSuite` Author: Kazuaki Ishizaki <[email protected]> Closes #19778 from kiszk/SPARK-22550. (cherry picked from commit 9bdff0b) Signed-off-by: Wenchen Fan <[email protected]>
| for (c <- cases.reverse) { | ||
| val funcName = ctx.freshName("eltFunc") | ||
| val funcBody = s""" | ||
| private UTF8String $funcName(InternalRow ${ctx.INPUT_ROW}, int $indexVal) { |
There was a problem hiding this comment.
Looks like this splitting doesn't prevent the case in wholestage codegen?
There was a problem hiding this comment.
ah good catch! we should fix it with splitExpressionsWithCurrentInputs
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 Added new test cases into `StringExpressionsSuite` Author: Kazuaki Ishizaki <[email protected]> Closes apache#19778 from kiszk/SPARK-22550. (cherry picked from commit 9bdff0b) Signed-off-by: Wenchen Fan <[email protected]>
5 participants
What changes were proposed in this pull request?
This PR changes
eltcode generation to place generated code for expression for arguments into separated methods if these size could be large.This PR resolved the case of
eltwith a lot of argumentHow was this patch tested?
Added new test cases into
StringExpressionsSuite