[SPARK-25029][BUILD][CORE] Janino "Two non-abstract methods ..." errors #22203
Conversation
|
Test build #95158 has finished for PR 22203 at commit
|
|
Test build #95159 has finished for PR 22203 at commit
|
| @@ -98,7 +98,7 @@ jackson-module-jaxb-annotations-2.6.7.jar | |||
| jackson-module-paranamer-2.7.9.jar | |||
| jackson-module-scala_2.11-2.6.7.1.jar | |||
| jackson-xc-1.9.13.jar | |||
| janino-3.0.8.jar | |||
| janino-3.0.9.jar | |||
There was a problem hiding this comment.
https://janino-compiler.github.io/janino/changelog.html
cc @kiszk @rednaxelafx Please check the changes made in 3.0.9
There was a problem hiding this comment.
It includes the fix of your issue janino-compiler/janino#46
There was a problem hiding this comment.
Yep my primary reason is pulling in this fix for Scala 2.12: janino-compiler/janino#54 The other changes I can see appear to be small bug fixes, some for Java 8 / 9 support, which is probably useful for us.
There was a problem hiding this comment.
Sorry for being late. I confirmed all of changes in the changelog. Two issues janino-compiler/janino#54 and janino-compiler/janino#46 are for Java 8 and later support. Others are bug fixes.
LGTM.
|
LGTM |
|
I'm wondering, though, either in the same PR or in a separate PR, could we add a test case in Spark that would trigger the "two non-abstract method" error so that we'd show an example scenario in Spark that'd trigger the issue and also show that Janino 3.0.9 has fixed it? |
|
Good point @rednaxelafx though I know some existing tests already definitely failed without this change. They're in the JIRA. It's pervasive enough that we'd see a number of failures due to this issue -- or at least I did when running this in Scala 2.12. |
|
Thanks @srowen ! In that case could we add a few mentions of such test cases that failed into the PR description itself, perhaps a few lines under the "How was this patch tested?" section to give a few examples of which tests used to fail and are now fixed? |
|
Done, I added one example test failure to the description here. |
|
Thanks! Merged to master |
Update to janino 3.0.9 to address Java 8 + Scala 2.12 incompatibility. The error manifests as test failures like this in `ExpressionEncoderSuite`: ``` - encode/decode for seq of string: List(abc, xyz) *** FAILED *** java.lang.RuntimeException: Error while encoding: org.codehaus.janino.InternalCompilerException: failed to compile: org.codehaus.janino.InternalCompilerException: Compiling "GeneratedClass": Two non-abstract methods "public int scala.collection.TraversableOnce.size()" have the same parameter types, declaring type and return type ``` It comes up pretty immediately in any generated code that references Scala collections, and virtually always concerning the `size()` method. Existing tests Closes apache#22203 from srowen/SPARK-25029. Authored-by: Sean Owen <sean.owen@databricks.com> Signed-off-by: Xiao Li <gatorsmile@gmail.com> (cherry picked from commit 9b6baeb)
Update to janino 3.0.9 to address Java 8 + Scala 2.12 incompatibility. The error manifests as test failures like this in `ExpressionEncoderSuite`: ``` - encode/decode for seq of string: List(abc, xyz) *** FAILED *** java.lang.RuntimeException: Error while encoding: org.codehaus.janino.InternalCompilerException: failed to compile: org.codehaus.janino.InternalCompilerException: Compiling "GeneratedClass": Two non-abstract methods "public int scala.collection.TraversableOnce.size()" have the same parameter types, declaring type and return type ``` It comes up pretty immediately in any generated code that references Scala collections, and virtually always concerning the `size()` method. Existing tests Closes apache#22203 from srowen/SPARK-25029. Authored-by: Sean Owen <sean.owen@databricks.com> Signed-off-by: Xiao Li <gatorsmile@gmail.com> (cherry picked from commit 9b6baeb)
What changes were proposed in this pull request?
Update to janino 3.0.9 to address Java 8 + Scala 2.12 incompatibility. The error manifests as test failures like this in
ExpressionEncoderSuite:It comes up pretty immediately in any generated code that references Scala collections, and virtually always concerning the
size()method.How was this patch tested?
Existing tests