From fa8f38234bd0439e48aa3f4b0bb5fb5d59795894 Mon Sep 17 00:00:00 2001 From: Guoliang Sun Date: Fri, 8 Mar 2024 17:39:31 +0800 Subject: [PATCH] KE-11534 Upgrade janino and fix issues for Calcite 1.30 (#735) * KE-11534 Upgrade Janino to 3.1.6 for Calcite 1.30 * KE-11534 [Follow up]Fix for null vs true/false comparisons in TDVT SQL626 brought by Calcite 1.30 --- dev/deps/spark-deps-hadoop-2-hive-2.3 | 4 +-- dev/deps/spark-deps-hadoop-3-hive-2.3 | 4 +-- pom.xml | 2 +- .../expressions/codegen/CodeGenerator.scala | 16 +++------ .../sql/errors/QueryExecutionErrors.scala | 3 +- .../expressions/CodeGenerationSuite.scala | 1 - .../catalyst/util/V2ExpressionBuilder.scala | 36 +++++++++++-------- 7 files changed, 33 insertions(+), 33 deletions(-) diff --git a/dev/deps/spark-deps-hadoop-2-hive-2.3 b/dev/deps/spark-deps-hadoop-2-hive-2.3 index 39aa63d7e7fdf..ff809fac2578c 100644 --- a/dev/deps/spark-deps-hadoop-2-hive-2.3 +++ b/dev/deps/spark-deps-hadoop-2-hive-2.3 @@ -38,7 +38,7 @@ commons-cli/1.5.0//commons-cli-1.5.0.jar commons-codec/1.15//commons-codec-1.15.jar commons-collections/3.2.2//commons-collections-3.2.2.jar commons-collections4/4.4//commons-collections4-4.4.jar -commons-compiler/3.0.16//commons-compiler-3.0.16.jar +commons-compiler/3.1.6//commons-compiler-3.1.6.jar commons-compress/1.21//commons-compress-1.21.jar commons-configuration/1.6//commons-configuration-1.6.jar commons-crypto/1.1.0//commons-crypto-1.1.0.jar @@ -129,7 +129,7 @@ jakarta.servlet-api/4.0.3//jakarta.servlet-api-4.0.3.jar jakarta.validation-api/2.0.2//jakarta.validation-api-2.0.2.jar jakarta.ws.rs-api/2.1.6//jakarta.ws.rs-api-2.1.6.jar jakarta.xml.bind-api/2.3.2//jakarta.xml.bind-api-2.3.2.jar -janino/3.0.16//janino-3.0.16.jar +janino/3.1.6//janino-3.1.6.jar javassist/3.25.0-GA//javassist-3.25.0-GA.jar javax.inject/1//javax.inject-1.jar javax.jdo/3.2.0-m3//javax.jdo-3.2.0-m3.jar diff --git a/dev/deps/spark-deps-hadoop-3-hive-2.3 b/dev/deps/spark-deps-hadoop-3-hive-2.3 index 160ece7f9a29d..095ad0062320c 100644 --- a/dev/deps/spark-deps-hadoop-3-hive-2.3 +++ b/dev/deps/spark-deps-hadoop-3-hive-2.3 @@ -40,7 +40,7 @@ commons-cli/1.5.0//commons-cli-1.5.0.jar commons-codec/1.15//commons-codec-1.15.jar commons-collections/3.2.2//commons-collections-3.2.2.jar commons-collections4/4.4//commons-collections4-4.4.jar -commons-compiler/3.0.16//commons-compiler-3.0.16.jar +commons-compiler/3.1.6//commons-compiler-3.1.6.jar commons-compress/1.21//commons-compress-1.21.jar commons-crypto/1.1.0//commons-crypto-1.1.0.jar commons-dbcp/1.4//commons-dbcp-1.4.jar @@ -118,7 +118,7 @@ jakarta.servlet-api/4.0.3//jakarta.servlet-api-4.0.3.jar jakarta.validation-api/2.0.2//jakarta.validation-api-2.0.2.jar jakarta.ws.rs-api/2.1.6//jakarta.ws.rs-api-2.1.6.jar jakarta.xml.bind-api/2.3.2//jakarta.xml.bind-api-2.3.2.jar -janino/3.0.16//janino-3.0.16.jar +janino/3.1.6//janino-3.1.6.jar javassist/3.25.0-GA//javassist-3.25.0-GA.jar javax.jdo/3.2.0-m3//javax.jdo-3.2.0-m3.jar javolution/5.5.1//javolution-5.5.1.jar diff --git a/pom.xml b/pom.xml index 7cffbdaae5727..dcfd0f3c0f119 100644 --- a/pom.xml +++ b/pom.xml @@ -188,7 +188,7 @@ 2.11.1 4.1.17 14.0.1 - 3.0.16 + 3.1.6 2.35 2.10.10 5.3.0 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 c982a7b3f9bbd..71fea289a5e23 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 @@ -18,7 +18,6 @@ package org.apache.spark.sql.catalyst.expressions.codegen import java.io.ByteArrayInputStream -import java.util.{Map => JavaMap} import scala.annotation.tailrec import scala.collection.JavaConverters._ @@ -28,8 +27,8 @@ import scala.util.control.NonFatal import com.google.common.cache.{CacheBuilder, CacheLoader} import com.google.common.util.concurrent.{ExecutionError, UncheckedExecutionException} -import org.codehaus.commons.compiler.CompileException -import org.codehaus.janino.{ByteArrayClassLoader, ClassBodyEvaluator, InternalCompilerException, SimpleCompiler} +import org.codehaus.commons.compiler.{CompileException, InternalCompilerException} +import org.codehaus.janino.ClassBodyEvaluator import org.codehaus.janino.util.ClassFile import org.apache.spark.{TaskContext, TaskKilledException} @@ -1521,14 +1520,9 @@ object CodeGenerator extends Logging { */ private def updateAndGetCompilationStats(evaluator: ClassBodyEvaluator): ByteCodeStats = { // First retrieve the generated classes. - val classes = { - val resultField = classOf[SimpleCompiler].getDeclaredField("result") - resultField.setAccessible(true) - val loader = resultField.get(evaluator).asInstanceOf[ByteArrayClassLoader] - val classesField = loader.getClass.getDeclaredField("classes") - classesField.setAccessible(true) - classesField.get(loader).asInstanceOf[JavaMap[String, Array[Byte]]].asScala - } + // Calcite 1.30's janino version upgrade to 3.1.6 + // SimpleCompiler annotates "result", we use ClassBodyEvaluator method instead directly + val classes = evaluator.getBytecodes.asScala // Then walk the classes to get at the method bytecode. val codeAttr = Utils.classForName("org.codehaus.janino.util.ClassFile$CodeAttribute") diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala index 21fe0b9267014..18a02497d9f6c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala @@ -31,8 +31,7 @@ import java.util.concurrent.TimeoutException import com.fasterxml.jackson.core.{JsonParser, JsonToken} import org.apache.hadoop.fs.{FileAlreadyExistsException, FileStatus, Path} import org.apache.hadoop.fs.permission.FsPermission -import org.codehaus.commons.compiler.CompileException -import org.codehaus.janino.InternalCompilerException +import org.codehaus.commons.compiler.{CompileException, InternalCompilerException} import org.apache.spark.{Partition, SparkArithmeticException, SparkArrayIndexOutOfBoundsException, SparkClassNotFoundException, SparkConcurrentModificationException, SparkDateTimeException, SparkException, SparkFileAlreadyExistsException, SparkFileNotFoundException, SparkIllegalArgumentException, SparkIndexOutOfBoundsException, SparkNoSuchElementException, SparkNoSuchMethodException, SparkNumberFormatException, SparkRuntimeException, SparkSecurityException, SparkSQLException, SparkSQLFeatureNotSupportedException, SparkUnsupportedOperationException, SparkUpgradeException} import org.apache.spark.executor.CommitDeniedException diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala index 1e4499a0ee3fe..737fcb1bada0e 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala @@ -568,7 +568,6 @@ class CodeGenerationSuite extends SparkFunSuite with ExpressionEvalHelper { assert(refTerm.contains("scala.math.LowPriorityOrderingImplicits$$anon$")) } - // TODO (SPARK-35579): Fix this bug in janino and upgrade janino in Spark. test("SPARK-35578: final local variable bug in janino") { val code = """ diff --git a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala index 7e15de56e2dec..19373d755714c 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala @@ -137,23 +137,31 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean = false) { case wb: WidthBucket => generateExpressionWithName("WIDTH_BUCKET", wb.children) case and: And => // AND expects predicate - val l = generateExpression(and.left, true) - val r = generateExpression(and.right, true) - if (l.isDefined && r.isDefined) { - assert(l.get.isInstanceOf[V2Predicate] && r.get.isInstanceOf[V2Predicate]) - Some(new V2And(l.get.asInstanceOf[V2Predicate], r.get.asInstanceOf[V2Predicate])) - } else { - None + val l = generateExpression(and.left, isPredicate = true) + val r = generateExpression(and.right, isPredicate = true) + (l, r) match { + case (Some(left: V2Predicate), Some(right: V2Predicate)) => + Some(new V2And(left, right)) + case (Some(left: V2Predicate), _) if r.isEmpty => + Some(left) + case (_, Some(right: V2Predicate)) if l.isEmpty => + Some(right) + case _ => + None } case or: Or => // OR expects predicate - val l = generateExpression(or.left, true) - val r = generateExpression(or.right, true) - if (l.isDefined && r.isDefined) { - assert(l.get.isInstanceOf[V2Predicate] && r.get.isInstanceOf[V2Predicate]) - Some(new V2Or(l.get.asInstanceOf[V2Predicate], r.get.asInstanceOf[V2Predicate])) - } else { - None + val l = generateExpression(or.left, isPredicate = true) + val r = generateExpression(or.right, isPredicate = true) + (l, r) match { + case (Some(left: V2Predicate), Some(right: V2Predicate)) => + Some(new V2Or(left, right)) + case (Some(left: V2Predicate), _) if r.isEmpty => + Some(left) + case (_, Some(right: V2Predicate)) if l.isEmpty => + Some(right) + case _ => + None } case b: BinaryOperator if canTranslate(b) => val l = generateExpression(b.left)