Skip to content

Commit

Permalink
[SPARK-31101][BUILD][2.4] Upgrade Janino to 3.0.16
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?

This PR(SPARK-31101) proposes to upgrade Janino to 3.0.16 which is released recently.

* Merged pull request janino-compiler/janino#114 "Grow the code for relocatables, and do fixup, and relocate".

Please see the commit log.
- https://github.com/janino-compiler/janino/commits/3.0.16

You can see the changelog from the link: http://janino-compiler.github.io/janino/changelog.html / though release note for Janino 3.0.16 is actually incorrect.

### Why are the changes needed?

We got some report on failure on user's query which Janino throws error on compiling generated code. The issue is here: janino-compiler/janino#113 It contains the information of generated code, symptom (error), and analysis of the bug, so please refer the link for more details.
Janino 3.0.16 contains the PR janino-compiler/janino#114 which would enable Janino to succeed to compile user's query properly.

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Existing UTs.

Below test code fails on branch-2.4 and passes with this patch.

(Note that there seems to be the case where another UT affects this UT to not fail - adding this to SQLQuerySuite won't fail this UT, but adding this to DateFunctionsSuite will fail this UT, and if you run this UT solely in SQLQuerySuite via `build/sbt "sql/testOnly *.SQLQuerySuite -- -z SPARK-31115"` then it fails.)

```
  /**
   * NOTE: The test code tries to control the size of for/switch statement in expand_doConsume,
   * as well as the overall size of expand_doConsume, so that the query triggers known Janino
   * bug - janino-compiler/janino#113.
   *
   * The expected exception message from Janino when we use switch statement for "ExpandExec":
   * - "Operand stack inconsistent at offset xxx: Previous size 1, now 0"
   * which will not happen when we use if-else-if statement for "ExpandExec".
   *
   * "The number of fields" and "The number of distinct aggregation functions" are the major
   * factors to increase the size of generated code: while these values should be large enough
   * to trigger the Janino bug, these values should not also too big; otherwise one of below
   * exceptions might be thrown:
   * - "expand_doConsume would be beyond 64KB"
   * - "java.lang.ClassFormatError: Too many arguments in method signature in class file"
   */
  test("SPARK-31115 Lots of columns and distinct aggregations shouldn't break code generation") {
    withSQLConf(
      (SQLConf.WHOLESTAGE_CODEGEN_ENABLED.key, "true"),
      (SQLConf.WHOLESTAGE_MAX_NUM_FIELDS.key, "10000"),
      (SQLConf.CODEGEN_FALLBACK.key, "false"),
      (SQLConf.CODEGEN_LOGGING_MAX_LINES.key, "-1")
    ) {
      var df = Seq(("1", "2", 1), ("1", "2", 2), ("2", "3", 3), ("2", "3", 4)).toDF("a", "b", "c")

      // The value is tested under commit "244405fe57d7737d81c34ba9e8917df6285889eb":
      // the query fails with switch statement, whereas it passes with if-else statement.
      // Note that the value depends on the Spark logic as well - different Spark versions may
      // require different value to ensure the test failing with switch statement.
      val numNewFields = 100

      df = df.withColumns(
        (1 to numNewFields).map { idx => s"a$idx" },
        (1 to numNewFields).map { idx =>
          when(col("c").mod(lit(2)).===(lit(0)), lit(idx)).otherwise(col("c"))
        }
      )

      val aggExprs: Array[Column] = Range(1, numNewFields).map { idx =>
        if (idx % 2 == 0) {
          coalesce(countDistinct(s"a$idx"), lit(0))
        } else {
          coalesce(count(s"a$idx"), lit(0))
        }
      }.toArray

      val aggDf = df
        .groupBy("a", "b")
        .agg(aggExprs.head, aggExprs.tail: _*)

      // We are only interested in whether the code compilation fails or not, so skipping
      // verification on outputs.
      aggDf.collect()
    }
  }
```

Closes #27997 from HeartSaVioR/SPARK-31101-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
  • Loading branch information
HeartSaVioR authored and dongjoon-hyun committed Mar 30, 2020
1 parent f05ac28 commit 4add8ad
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 7 deletions.
4 changes: 2 additions & 2 deletions dev/deps/spark-deps-hadoop-2.6
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ commons-beanutils/1.9.4//commons-beanutils-1.9.4.jar
commons-cli/1.2//commons-cli-1.2.jar
commons-codec/1.10//commons-codec-1.10.jar
commons-collections/3.2.2//commons-collections-3.2.2.jar
commons-compiler/3.0.9//commons-compiler-3.0.9.jar
commons-compiler/3.0.16//commons-compiler-3.0.16.jar
commons-compress/1.8.1//commons-compress-1.8.1.jar
commons-configuration/1.6//commons-configuration-1.6.jar
commons-crypto/1.0.0//commons-crypto-1.0.0.jar
Expand Down Expand Up @@ -97,7 +97,7 @@ jackson-module-jaxb-annotations/2.6.7//jackson-module-jaxb-annotations-2.6.7.jar
jackson-module-paranamer/2.7.9//jackson-module-paranamer-2.7.9.jar
jackson-module-scala_2.11/2.6.7.1//jackson-module-scala_2.11-2.6.7.1.jar
jackson-xc/1.9.13//jackson-xc-1.9.13.jar
janino/3.0.9//janino-3.0.9.jar
janino/3.0.16//janino-3.0.16.jar
javassist/3.18.1-GA//javassist-3.18.1-GA.jar
javax.annotation-api/1.2//javax.annotation-api-1.2.jar
javax.inject/1//javax.inject-1.jar
Expand Down
4 changes: 2 additions & 2 deletions dev/deps/spark-deps-hadoop-2.7
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ commons-beanutils/1.9.4//commons-beanutils-1.9.4.jar
commons-cli/1.2//commons-cli-1.2.jar
commons-codec/1.10//commons-codec-1.10.jar
commons-collections/3.2.2//commons-collections-3.2.2.jar
commons-compiler/3.0.9//commons-compiler-3.0.9.jar
commons-compiler/3.0.16//commons-compiler-3.0.16.jar
commons-compress/1.8.1//commons-compress-1.8.1.jar
commons-configuration/1.6//commons-configuration-1.6.jar
commons-crypto/1.0.0//commons-crypto-1.0.0.jar
Expand Down Expand Up @@ -97,7 +97,7 @@ jackson-module-jaxb-annotations/2.6.7//jackson-module-jaxb-annotations-2.6.7.jar
jackson-module-paranamer/2.7.9//jackson-module-paranamer-2.7.9.jar
jackson-module-scala_2.11/2.6.7.1//jackson-module-scala_2.11-2.6.7.1.jar
jackson-xc/1.9.13//jackson-xc-1.9.13.jar
janino/3.0.9//janino-3.0.9.jar
janino/3.0.16//janino-3.0.16.jar
javassist/3.18.1-GA//javassist-3.18.1-GA.jar
javax.annotation-api/1.2//javax.annotation-api-1.2.jar
javax.inject/1//javax.inject-1.jar
Expand Down
4 changes: 2 additions & 2 deletions dev/deps/spark-deps-hadoop-3.1
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ commons-beanutils/1.9.4//commons-beanutils-1.9.4.jar
commons-cli/1.2//commons-cli-1.2.jar
commons-codec/1.10//commons-codec-1.10.jar
commons-collections/3.2.2//commons-collections-3.2.2.jar
commons-compiler/3.0.9//commons-compiler-3.0.9.jar
commons-compiler/3.0.16//commons-compiler-3.0.16.jar
commons-compress/1.8.1//commons-compress-1.8.1.jar
commons-configuration2/2.1.1//commons-configuration2-2.1.1.jar
commons-crypto/1.0.0//commons-crypto-1.0.0.jar
Expand Down Expand Up @@ -97,7 +97,7 @@ jackson-mapper-asl/1.9.13//jackson-mapper-asl-1.9.13.jar
jackson-module-jaxb-annotations/2.6.7//jackson-module-jaxb-annotations-2.6.7.jar
jackson-module-paranamer/2.7.9//jackson-module-paranamer-2.7.9.jar
jackson-module-scala_2.11/2.6.7.1//jackson-module-scala_2.11-2.6.7.1.jar
janino/3.0.9//janino-3.0.9.jar
janino/3.0.16//janino-3.0.16.jar
javassist/3.18.1-GA//javassist-3.18.1-GA.jar
javax.annotation-api/1.2//javax.annotation-api-1.2.jar
javax.inject/1//javax.inject-1.jar
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
<!-- org.apache.commons/commons-lang3/-->
<commons-lang3.version>3.5</commons-lang3.version>
<datanucleus-core.version>3.2.10</datanucleus-core.version>
<janino.version>3.0.9</janino.version>
<janino.version>3.0.16</janino.version>
<jersey.version>2.22.2</jersey.version>
<joda.version>2.9.3</joda.version>
<jodd.version>3.5.2</jodd.version>
Expand Down

0 comments on commit 4add8ad

Please sign in to comment.