You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@drill.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/10/19 12:54:00 UTC

[jira] [Commented] (DRILL-6795) Upgrade Janino compiler from 2.7.6 to 3.0.10

    [ https://issues.apache.org/jira/browse/DRILL-6795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16656767#comment-16656767 ] 

ASF GitHub Bot commented on DRILL-6795:
---------------------------------------

asfgit closed pull request #1503: DRILL-6795: Upgrade Janino compiler from 2.7.6 to 3.0.10
URL: https://github.com/apache/drill/pull/1503
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/exec/java-exec/pom.xml b/exec/java-exec/pom.xml
index 5b00a8ce8c8..43897efd0d7 100644
--- a/exec/java-exec/pom.xml
+++ b/exec/java-exec/pom.xml
@@ -349,6 +349,14 @@
       <artifactId>joda-time</artifactId>
       <version>2.9</version>
     </dependency>
+    <dependency>
+      <groupId>org.codehaus.janino</groupId>
+      <artifactId>janino</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.codehaus.janino</groupId>
+      <artifactId>commons-compiler</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-common</artifactId>
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ImportGrabber.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ImportGrabber.java
index e75d20b9401..3e67ebf0d9f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ImportGrabber.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ImportGrabber.java
@@ -17,6 +17,7 @@
  */
 package org.apache.drill.exec.expr.fn;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.codehaus.janino.Java;
@@ -24,20 +25,32 @@
 import org.codehaus.janino.Java.CompilationUnit.SingleTypeImportDeclaration;
 import org.codehaus.janino.Java.CompilationUnit.StaticImportOnDemandDeclaration;
 import org.codehaus.janino.Java.CompilationUnit.TypeImportOnDemandDeclaration;
-import org.codehaus.janino.util.Traverser;
-
-import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
-
+import org.codehaus.janino.util.AbstractTraverser;
 
 public class ImportGrabber {
 
-  private final List<String> imports = Lists.newArrayList();
-  private final ImportFinder finder = new ImportFinder();
+  private final List<String> imports = new ArrayList<>();
+  private final ImportFinder importFinder = new ImportFinder();
 
   private ImportGrabber() {
   }
 
-  public class ImportFinder extends Traverser {
+  /**
+   * Creates list of imports that are present in compilation unit.
+   * For example:
+   * [import io.netty.buffer.DrillBuf;, import org.apache.drill.exec.expr.DrillSimpleFunc;]
+   *
+   * @param compilationUnit compilation unit
+   * @return list of imports
+   */
+  public static List<String> getImports(Java.CompilationUnit compilationUnit) {
+    ImportGrabber visitor = new ImportGrabber();
+    compilationUnit.importDeclarations.forEach(visitor.importFinder::visitImportDeclaration);
+
+    return visitor.imports;
+  }
+
+  public class ImportFinder extends AbstractTraverser<RuntimeException> {
 
     @Override
     public void traverseSingleTypeImportDeclaration(SingleTypeImportDeclaration stid) {
@@ -58,26 +71,6 @@ public void traverseTypeImportOnDemandDeclaration(TypeImportOnDemandDeclaration
     public void traverseStaticImportOnDemandDeclaration(StaticImportOnDemandDeclaration siodd) {
       imports.add(siodd.toString());
     }
-
-
-  }
-
-  /**
-   * Creates list of imports that are present in compilation unit.
-   * For example:
-   * [import io.netty.buffer.DrillBuf;, import org.apache.drill.exec.expr.DrillSimpleFunc;]
-   *
-   * @param compilationUnit compilation unit
-   * @return list of imports
-   */
-  public static List<String> getImports(Java.CompilationUnit compilationUnit){
-    final ImportGrabber visitor = new ImportGrabber();
-
-    for (Java.CompilationUnit.ImportDeclaration importDeclaration : compilationUnit.importDeclarations) {
-      importDeclaration.accept(visitor.finder.comprehensiveVisitor());
-    }
-
-    return visitor.imports;
   }
 
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
index 47ca9b2881b..782e1e2b82e 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/MethodGrabbingVisitor.java
@@ -18,62 +18,60 @@
 package org.apache.drill.exec.expr.fn;
 
 import java.io.StringWriter;
+import java.util.HashMap;
 import java.util.Map;
 
 import org.codehaus.janino.Java;
-import org.codehaus.janino.Java.ClassDeclaration;
+import org.codehaus.janino.Java.AbstractClassDeclaration;
 import org.codehaus.janino.Java.MethodDeclarator;
-import org.codehaus.janino.util.Traverser;
+import org.codehaus.janino.util.AbstractTraverser;
 
-import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+public class MethodGrabbingVisitor {
 
-
-public class MethodGrabbingVisitor{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(MethodGrabbingVisitor.class);
-
-  private Class<?> c;
-  private Map<String, String> methods = Maps.newHashMap();
-  private ClassFinder classFinder = new ClassFinder();
+  private final Class<?> clazz;
+  private final Map<String, String> methods = new HashMap<>();
+  private final ClassFinder classFinder = new ClassFinder();
   private boolean captureMethods = false;
 
-  private MethodGrabbingVisitor(Class<?> c) {
-    super();
-    this.c = c;
+  private MethodGrabbingVisitor(Class<?> clazz) {
+    this.clazz = clazz;
+  }
+
+  /**
+   * Creates a map with all method names and their modified bodies
+   * from specified {@link Java.CompilationUnit}.
+   *
+   * @param compilationUnit the source of the methods to collect
+   * @param clazz           type of the class to handle
+   * @return a map with all method names and their modified bodies.
+   */
+  public static Map<String, String> getMethods(Java.CompilationUnit compilationUnit, Class<?> clazz) {
+    MethodGrabbingVisitor visitor = new MethodGrabbingVisitor(clazz);
+    visitor.classFinder.visitTypeDeclaration(compilationUnit.getPackageMemberTypeDeclarations()[0]);
+    return visitor.methods;
   }
 
-  public class ClassFinder extends Traverser{
+  public class ClassFinder extends AbstractTraverser<RuntimeException> {
 
     @Override
-    public void traverseClassDeclaration(ClassDeclaration cd) {
-//      logger.debug("Traversing: {}", cd.getClassName());
+    public void traverseClassDeclaration(AbstractClassDeclaration classDeclaration) {
       boolean prevCapture = captureMethods;
-      captureMethods = c.getName().equals(cd.getClassName());
-      super.traverseClassDeclaration(cd);
+      captureMethods = clazz.getName().equals(classDeclaration.getClassName());
+      super.traverseClassDeclaration(classDeclaration);
       captureMethods = prevCapture;
     }
 
     @Override
-    public void traverseMethodDeclarator(MethodDeclarator md) {
-//      logger.debug(c.getName() + ": Found {}, include {}", md.name, captureMethods);
-
-      if(captureMethods){
+    public void traverseMethodDeclarator(MethodDeclarator methodDeclarator) {
+      if (captureMethods) {
         StringWriter writer = new StringWriter();
-        ModifiedUnparseVisitor v = new ModifiedUnparseVisitor(writer);
-//        UnparseVisitor v = new UnparseVisitor(writer);
-
-        md.accept(v);
-        v.close();
+        ModifiedUnparser unparser = new ModifiedUnparser(writer);
+        unparser.visitMethodDeclarator(methodDeclarator);
+        unparser.close();
         writer.flush();
-        methods.put(md.name, writer.getBuffer().toString());
+        methods.put(methodDeclarator.name, writer.getBuffer().toString());
       }
     }
   }
 
-
-  public static Map<String, String> getMethods(Java.CompilationUnit cu, Class<?> c){
-    MethodGrabbingVisitor visitor = new MethodGrabbingVisitor(c);
-    cu.getPackageMemberTypeDeclarations()[0].accept(visitor.classFinder.comprehensiveVisitor());
-    return visitor.methods;
-  }
-
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparseVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparseVisitor.java
deleted file mode 100644
index fe8349057e1..00000000000
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparseVisitor.java
+++ /dev/null
@@ -1,120 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.drill.exec.expr.fn;
-
-import java.io.Writer;
-import java.util.List;
-
-import org.codehaus.janino.Java;
-import org.codehaus.janino.UnparseVisitor;
-import org.codehaus.janino.util.AutoIndentWriter;
-
-/**
- * This is a modified version of {@link UnparseVisitor} so that we can avoid
- * rendering few things. Based on janino version 2.7.4.
- */
-public class ModifiedUnparseVisitor extends UnparseVisitor {
-
-  private String returnLabel;
-
-  /**
-   * Testing of parsing/unparsing.
-   * <p>
-   * Reads compilation units from the files named on the command line
-   * and unparses them to {@link System#out}.
-   */
-  public static void main(String[] args) throws Exception {
-    UnparseVisitor.main(args);
-  }
-
-  /**
-   * Unparse the given {@link org.codehaus.janino.Java.CompilationUnit} to the given {@link java.io.Writer}.
-   */
-  public static void unparse(Java.CompilationUnit cu, Writer w) {
-    UnparseVisitor.unparse(cu, w);
-  }
-
-  public ModifiedUnparseVisitor(Writer w) {
-    super(w);
-  }
-
-  @Override
-  public void visitMethodDeclarator(Java.MethodDeclarator md) {
-    if (md.optionalStatements == null) {
-      this.pw.print(';');
-    } else
-      if (md.optionalStatements.isEmpty()) {
-        this.pw.print(" {}");
-      } else {
-        this.pw.println(' ');
-        // Add labels to handle return statements within function templates
-        String[] fQCN = md.getDeclaringType().getClassName().split("\\.");
-        returnLabel = fQCN[fQCN.length - 1] + "_" + md.name;
-        this.pw.println(returnLabel + ": {");
-        this.pw.print(AutoIndentWriter.INDENT);
-        this.unparseStatements(md.optionalStatements);
-        this.pw.print(AutoIndentWriter.UNINDENT);
-        this.pw.println("}");
-        this.pw.print(' ');
-      }
-  }
-
-  @Override
-  public void visitReturnStatement(Java.ReturnStatement rs) {
-    this.pw.print("break " + returnLabel);
-    if (rs.optionalReturnValue != null) {
-      this.pw.print(' ');
-      this.unparse(rs.optionalReturnValue);
-    }
-    this.pw.print(';');
-  }
-
-  /*
-   * The following helper methods are copied from the parent class since they
-   * are declared private in the parent class and can not be used in the child
-   * class (this).
-   */
-
-  private void unparseStatements(List<? extends Java.BlockStatement> statements) {
-    int state = -1;
-    for (Java.BlockStatement bs : statements) {
-      int x  = (
-          bs instanceof Java.Block                             ? 1 :
-          bs instanceof Java.LocalClassDeclarationStatement    ? 2 :
-          bs instanceof Java.LocalVariableDeclarationStatement ? 3 :
-          bs instanceof Java.SynchronizedStatement             ? 4 :
-          99
-      );
-      if (state != -1 && state != x) {
-        this.pw.println(AutoIndentWriter.CLEAR_TABULATORS);
-      }
-      state = x;
-
-      this.unparseBlockStatement(bs);
-      this.pw.println();
-    }
-  }
-
-  private void unparseBlockStatement(Java.BlockStatement blockStatement) {
-    blockStatement.accept(this);
-  }
-
-  private void unparse(Java.Atom operand) {
-    operand.accept(this);
-  }
-}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java
new file mode 100644
index 00000000000..e7b22447c6e
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ModifiedUnparser.java
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.expr.fn;
+
+import java.io.Writer;
+import java.util.List;
+
+import org.codehaus.janino.Java;
+import org.codehaus.janino.Unparser;
+import org.codehaus.janino.util.AutoIndentWriter;
+
+/**
+ * This is a modified version of {@link Unparser} so that we can avoid
+ * rendering few things.
+ */
+public class ModifiedUnparser extends Unparser {
+
+  private String returnLabel;
+
+  public ModifiedUnparser(Writer writer) {
+    super(writer);
+  }
+
+  @Override
+  public void unparseBlockStatement(Java.BlockStatement blockStatement) {
+    // Unparser uses anonymous classes for visiting statements,
+    // therefore added this check for customizing of handling ReturnStatement.
+    if (blockStatement instanceof Java.ReturnStatement) {
+      visitReturnStatement((Java.ReturnStatement) blockStatement);
+    } else {
+      super.unparseBlockStatement(blockStatement);
+    }
+  }
+
+  /**
+   * Parses specified {@link Java.MethodDeclarator}, wraps its content
+   * with replaced {@code return} statements by {@code break} ones into the
+   * block with label and stores it into {@link java.io.PrintWriter}.
+   *
+   * @param methodDeclarator method to parse
+   */
+  public void visitMethodDeclarator(Java.MethodDeclarator methodDeclarator) {
+    if (methodDeclarator.optionalStatements == null) {
+      pw.print(';');
+    } else if (methodDeclarator.optionalStatements.isEmpty()) {
+      pw.print(" {}");
+    } else {
+      pw.println(' ');
+      // Add labels to handle return statements within function templates
+      String[] fQCN = methodDeclarator.getDeclaringType().getClassName().split("\\.");
+      returnLabel = fQCN[fQCN.length - 1] + "_" + methodDeclarator.name;
+      pw.print(returnLabel);
+      pw.println(": {");
+      pw.print(AutoIndentWriter.INDENT);
+      unparseStatements(methodDeclarator.optionalStatements);
+      pw.print(AutoIndentWriter.UNINDENT);
+      pw.println("}");
+      pw.print(' ');
+    }
+  }
+
+  private void visitReturnStatement(Java.ReturnStatement returnStatement) {
+    pw.print("break ");
+    pw.print(returnLabel);
+    if (returnStatement.optionalReturnValue != null) {
+      pw.print(' ');
+      unparseAtom(returnStatement.optionalReturnValue);
+    }
+    pw.print(';');
+  }
+
+  /**
+   * The following helper method is copied from the parent class since it
+   * is declared as private in the parent class and can not be used in the child
+   * class (this).
+   */
+  private void unparseStatements(List<? extends Java.BlockStatement> statements) {
+    int state = -1;
+    for (Java.BlockStatement bs : statements) {
+      int x = (
+        bs instanceof Java.Block ? 1 :
+          bs instanceof Java.LocalClassDeclarationStatement ? 2 :
+            bs instanceof Java.LocalVariableDeclarationStatement ? 3 :
+              bs instanceof Java.SynchronizedStatement ? 4 : 99
+      );
+      if (state != -1 && state != x) {
+        pw.println(AutoIndentWriter.CLEAR_TABULATORS);
+      }
+      state = x;
+
+      unparseBlockStatement(bs);
+      pw.println();
+    }
+  }
+}
diff --git a/pom.xml b/pom.xml
index 3a2d21ef806..dbd8caf07f7 100644
--- a/pom.xml
+++ b/pom.xml
@@ -48,7 +48,7 @@
     <parquet.version>1.10.0</parquet.version>
     <calcite.version>1.17.0-drill-r1</calcite.version>
     <avatica.version>1.12.0</avatica.version>
-    <janino.version>2.7.6</janino.version>
+    <janino.version>3.0.10</janino.version>
     <sqlline.version>1.5.0</sqlline.version>
     <jackson.version>2.9.5</jackson.version>
     <jackson.databind.version>2.9.5</jackson.databind.version>
@@ -1366,6 +1366,11 @@
         <artifactId>janino</artifactId>
         <version>${janino.version}</version>
       </dependency>
+      <dependency>
+        <groupId>org.codehaus.janino</groupId>
+        <artifactId>commons-compiler</artifactId>
+        <version>${janino.version}</version>
+      </dependency>
       <dependency>
         <groupId>com.fasterxml.jackson.core</groupId>
         <artifactId>jackson-annotations</artifactId>


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Upgrade Janino compiler from 2.7.6 to 3.0.10
> --------------------------------------------
>
>                 Key: DRILL-6795
>                 URL: https://issues.apache.org/jira/browse/DRILL-6795
>             Project: Apache Drill
>          Issue Type: Task
>    Affects Versions: 1.14.0
>            Reporter: Volodymyr Vysotskyi
>            Assignee: Volodymyr Vysotskyi
>            Priority: Major
>              Labels: ready-to-commit
>             Fix For: 1.15.0
>
>
> Upgrade Janino compiler from 2.7.6 to 3.0.10.
> In newer Janino version some classes were renamed; some classes were refactored to correspond the visitor pattern.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)