[GitHub] [calcite] birschick-bq commented on a change in pull request #2449: [CALCITE-3745] Integrate to new version of Janino (3.1.6) to provide class loader.

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

birschick-bq commented on a change in pull request #2449:
URL: https://github.com/apache/calcite/pull/2449#discussion_r679357499



##########
File path: core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableInterpretable.java
##########
@@ -135,7 +135,8 @@ static Bindable getBindable(ClassDeclaration expr, String s, int fieldCount)
       throws CompileException, IOException, ExecutionException {
     ICompilerFactory compilerFactory;
     try {
-      compilerFactory = CompilerFactoryFactory.getDefaultCompilerFactory();
+      compilerFactory = CompilerFactoryFactory.getDefaultCompilerFactory(

Review comment:
       @zabetak Good question. 
   
   The root issue is that using Calcite in an "uber" JAR fails. When Calcite tries to load the Janino compiler using the `CompilerFactoryFactory`, it fails in the previous version of Janino commons library. This is because It was using the `Thread.currentThread().getContextClassLoader()` which was `null` at runtime.
   
   I worked with the team at Janino to find a solution - where Calcite can pass its class loader to the Janino factory class so that it has the working class's class loader.
   
   So we needed the latest version of Janino to get a fix to the problem of using Calcite in an "uber" JAR. 
   
   Please see the origin [JIRA  issue](https://issues.apache.org/jira/browse/CALCITE-3745).




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

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