[GitHub] [spark] rednaxelafx commented on pull request #29495: [SPARK-32640][SQL] Downgrade Janino to fix a correctness bug

[GitBox <gi...@apache.org>]

rednaxelafx commented on pull request #29495:
URL: https://github.com/apache/spark/pull/29495#issuecomment-677864868


   Right, I had hit another bug in Janino 3.1.2 elsewhere with this code pattern:
   ```
   int x;
   if (cond) {
     x = ...
   } else {
     long y = ...
     x = ...
   }
   ```
   (the actual code to trigger the problem is slightly more complicate but this example shows the gist)
   This Janino bug is specific to the new stack map calculation logic, which is new in Janino 3.1.x. Going back to the 3..0.x releases gets around the problem.
   
   With that issue and the issue mentioned in this PR, it looks like the conditional branch handling in Janino 3.1.x has degraded quite a bit. Let's avoid Janino 3.1.x until we can have more confidence in it.
   
   I'd like to also suggest adding some codegen test cases specifically to test the underlying Java compiler we use at runtime, be it Janino or any other alternatives (e.g. javac). WDYT?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org