diff options
author | Yin Huai <yhuai@databricks.com> | 2016-05-31 12:30:34 -0700 |
---|---|---|
committer | Yin Huai <yhuai@databricks.com> | 2016-05-31 12:30:34 -0700 |
commit | c6de5832bfad423e7d6f7e0a92a48170265f25cd (patch) | |
tree | 34160fcacac315147a95435c356442a1ac81dd89 | |
parent | 2bfed1a0c5be7d0718fd574a4dad90f4f6b44be7 (diff) | |
download | spark-c6de5832bfad423e7d6f7e0a92a48170265f25cd.tar.gz spark-c6de5832bfad423e7d6f7e0a92a48170265f25cd.tar.bz2 spark-c6de5832bfad423e7d6f7e0a92a48170265f25cd.zip |
[SPARK-15622][SQL] Wrap the parent classloader of Janino's classloader in the ParentClassLoader.
## What changes were proposed in this pull request?
At https://github.com/aunkrig/janino/blob/janino_2.7.8/janino/src/org/codehaus/janino/ClassLoaderIClassLoader.java#L80-L85, Janino's classloader throws the exception when its parent throws a ClassNotFoundException with a cause set. However, it does not throw the exception when there is no cause set. Seems we need to use a special ClassLoader to wrap the actual parent classloader set to Janino handle this behavior.
## How was this patch tested?
I have reverted the workaround made by https://issues.apache.org/jira/browse/SPARK-11636 ( https://github.com/apache/spark/compare/master...yhuai:SPARK-15622?expand=1#diff-bb538fda94224dd0af01d0fd7e1b4ea0R81) and `test-only *ReplSuite -- -z "SPARK-2576 importing implicits"` still passes the test (without the change in `CodeGenerator`, this test does not pass with the change in `ExecutorClassLoader `).
Author: Yin Huai <yhuai@databricks.com>
Closes #13366 from yhuai/SPARK-15622.
-rw-r--r-- | repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala | 8 | ||||
-rw-r--r-- | sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala | 16 |
2 files changed, 14 insertions, 10 deletions
diff --git a/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala b/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala index 4a15d52b57..2f07395edf 100644 --- a/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala +++ b/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala @@ -79,13 +79,7 @@ class ExecutorClassLoader( case e: ClassNotFoundException => val classOption = findClassLocally(name) classOption match { - case None => - // If this class has a cause, it will break the internal assumption of Janino - // (the compiler used for Spark SQL code-gen). - // See org.codehaus.janino.ClassLoaderIClassLoader's findIClass, you will see - // its behavior will be changed if there is a cause and the compilation - // of generated class will fail. - throw new ClassNotFoundException(name) + case None => throw new ClassNotFoundException(name, e) case Some(a) => a } } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala index 2706c38d9e..86883d7593 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala @@ -19,10 +19,10 @@ package org.apache.spark.sql.catalyst.expressions.codegen import scala.collection.mutable import scala.collection.mutable.ArrayBuffer -import scala.language.existentials import com.google.common.cache.{CacheBuilder, CacheLoader} import org.codehaus.janino.ClassBodyEvaluator +import scala.language.existentials import org.apache.spark.internal.Logging import org.apache.spark.sql.catalyst.InternalRow @@ -31,7 +31,7 @@ import org.apache.spark.sql.catalyst.util.{ArrayData, MapData} import org.apache.spark.sql.types._ import org.apache.spark.unsafe.Platform import org.apache.spark.unsafe.types._ -import org.apache.spark.util.Utils +import org.apache.spark.util.{ParentClassLoader, Utils} /** * Java source for evaluating an [[Expression]] given a [[InternalRow]] of input. @@ -806,7 +806,17 @@ object CodeGenerator extends Logging { */ private[this] def doCompile(code: CodeAndComment): GeneratedClass = { val evaluator = new ClassBodyEvaluator() - evaluator.setParentClassLoader(Utils.getContextOrSparkClassLoader) + + // A special classloader used to wrap the actual parent classloader of + // [[org.codehaus.janino.ClassBodyEvaluator]] (see CodeGenerator.doCompile). This classloader + // does not throw a ClassNotFoundException with a cause set (i.e. exception.getCause returns + // a null). This classloader is needed because janino will throw the exception directly if + // the parent classloader throws a ClassNotFoundException with cause set instead of trying to + // find other possible classes (see org.codehaus.janinoClassLoaderIClassLoader's + // findIClass method). Please also see https://issues.apache.org/jira/browse/SPARK-15622 and + // https://issues.apache.org/jira/browse/SPARK-11636. + val parentClassLoader = new ParentClassLoader(Utils.getContextOrSparkClassLoader) + evaluator.setParentClassLoader(parentClassLoader) // Cannot be under package codegen, or fail with java.lang.InstantiationException evaluator.setClassName("org.apache.spark.sql.catalyst.expressions.GeneratedClass") evaluator.setDefaultImports(Array( |