Skip to content

Commit

Permalink
KE-11534 Upgrade janino and fix issues for Calcite 1.30 (apache#735)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
gleonSun authored Mar 8, 2024
1 parent c7ed0dc commit fa8f382
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 33 deletions.
4 changes: 2 additions & 2 deletions dev/deps/spark-deps-hadoop-2-hive-2.3
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions dev/deps/spark-deps-hadoop-3-hive-2.3
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
<commons-pool2.version>2.11.1</commons-pool2.version>
<datanucleus-core.version>4.1.17</datanucleus-core.version>
<guava.version>14.0.1</guava.version>
<janino.version>3.0.16</janino.version>
<janino.version>3.1.6</janino.version>
<jersey.version>2.35</jersey.version>
<joda.version>2.10.10</joda.version>
<jodd.version>5.3.0</jodd.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand All @@ -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}
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit fa8f382

Please sign in to comment.