You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ja...@apache.org on 2015/03/20 06:15:19 UTC
[2/5] drill git commit: DRILL-2503: AsmUtil ClassTransformer
MergeAdapter - add option to pass through ClassReader.EXPAND_FRAMES to
satisfy complaint from ASM - rationalize AsmUtils methods' argument lists
DRILL-2503: AsmUtil ClassTransformer MergeAdapter - add option to pass through ClassReader.EXPAND_FRAMES to satisfy complaint from ASM - rationalize AsmUtils methods' argument lists
TestBugFixes (in ...drill.jdbc.test)
- created this to hold random bug fix tests
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/7af5f9a0
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/7af5f9a0
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/7af5f9a0
Branch: refs/heads/master
Commit: 7af5f9a01b1433bb8bb62c1e04a6cef68d629b48
Parents: 838fd08
Author: Chris Westin <cw...@yahoo.com>
Authored: Thu Mar 19 17:13:35 2015 -0700
Committer: Jacques Nadeau <ja...@apache.org>
Committed: Thu Mar 19 22:14:14 2015 -0700
----------------------------------------------------------------------
.../org/apache/drill/exec/compile/AsmUtil.java | 25 +++---
.../drill/exec/compile/ClassTransformer.java | 5 +-
.../apache/drill/exec/compile/MergeAdapter.java | 58 +++-----------
.../apache/drill/jdbc/test/TestBugFixes.java | 84 ++++++++++++++++++++
4 files changed, 109 insertions(+), 63 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/7af5f9a0/exec/java-exec/src/main/java/org/apache/drill/exec/compile/AsmUtil.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/AsmUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/AsmUtil.java
index 032aebd..81904df 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/AsmUtil.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/AsmUtil.java
@@ -42,12 +42,12 @@ public class AsmUtil {
/**
* Check to see if a class is well-formed.
*
- * @param classNode the class to check
- * @param logTag a tag to print to the log if a problem is found
* @param logger the logger to write to if a problem is found
+ * @param logTag a tag to print to the log if a problem is found
+ * @param classNode the class to check
* @return true if the class is ok, false otherwise
*/
- public static boolean isClassOk(final Logger logger, final ClassNode classNode, final String logTag) {
+ public static boolean isClassOk(final Logger logger, final String logTag, final ClassNode classNode) {
final StringWriter sw = new StringWriter();
final ClassWriter verifyWriter = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
classNode.accept(verifyWriter);
@@ -71,26 +71,27 @@ public class AsmUtil {
/**
* Check to see if a class is well-formed.
*
- * @param classBytes the bytecode of the class to check
- * @param logTag a tag to print to the log if a problem is found
* @param logger the logger to write to if a problem is found
+ * @param logTag a tag to print to the log if a problem is found
+ * @param classBytes the bytecode of the class to check
* @return true if the class is ok, false otherwise
*/
- public static boolean isClassBytesOk(final Logger logger, final byte[] classBytes, final String logTag) {
- final ClassNode classNode = classFromBytes(classBytes);
- return isClassOk(logger, classNode, logTag);
+ public static boolean isClassBytesOk(final Logger logger, final String logTag, final byte[] classBytes) {
+ final ClassNode classNode = classFromBytes(classBytes, 0);
+ return isClassOk(logger, logTag, classNode);
}
/**
* Create a ClassNode from bytecode.
*
* @param classBytes the bytecode
+ * @param asmReaderFlags flags for ASM; see {@link org.objectweb.asm.ClassReader#accept(org.objectweb.asm.ClassVisitor, int)}
* @return the ClassNode
*/
- public static ClassNode classFromBytes(final byte[] classBytes) {
+ public static ClassNode classFromBytes(final byte[] classBytes, final int asmReaderFlags) {
final ClassNode classNode = new ClassNode(CompilationConfig.ASM_API_VERSION);
final ClassReader classReader = new ClassReader(classBytes);
- classReader.accept(classNode, 0);
+ classReader.accept(classNode, asmReaderFlags);
return classNode;
}
@@ -99,9 +100,9 @@ public class AsmUtil {
*
* <p>Writes at level DEBUG.
*
+ * @param logger the logger to write to
* @param logTag a tag to print to the log
* @param classNode the class
- * @param logger the logger to write to
*/
public static void logClass(final Logger logger, final String logTag, final ClassNode classNode) {
logger.debug(logTag);
@@ -122,7 +123,7 @@ public class AsmUtil {
* @param logger the logger to write to
*/
public static void logClassFromBytes(final Logger logger, final String logTag, final byte[] classBytes) {
- final ClassNode classNode = classFromBytes(classBytes);
+ final ClassNode classNode = classFromBytes(classBytes, 0);
logClass(logger, logTag, classNode);
}
http://git-wip-us.apache.org/repos/asf/drill/blob/7af5f9a0/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
index 493f6ce..d4d74dd 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java
@@ -30,6 +30,7 @@ import org.apache.drill.exec.server.options.OptionManager;
import org.apache.drill.exec.server.options.OptionValue;
import org.apache.drill.exec.server.options.TypeValidators.EnumeratedStringValidator;
import org.codehaus.commons.compiler.CompileException;
+import org.objectweb.asm.ClassReader;
import org.objectweb.asm.tree.ClassNode;
import com.google.common.annotations.VisibleForTesting;
@@ -226,8 +227,8 @@ public class ClassTransformer {
Map<String, ClassNode> classesToMerge = Maps.newHashMap();
for (byte[] clazz : implementationClasses) {
totalBytecodeSize += clazz.length;
- final ClassNode node = AsmUtil.classFromBytes(clazz);
- if (!AsmUtil.isClassOk(logger, node, "implementationClasses")) {
+ final ClassNode node = AsmUtil.classFromBytes(clazz, ClassReader.EXPAND_FRAMES);
+ if (!AsmUtil.isClassOk(logger, "implementationClasses", node)) {
throw new IllegalStateException("Problem found with implementationClasses");
}
classesToMerge.put(node.name, node);
http://git-wip-us.apache.org/repos/asf/drill/blob/7af5f9a0/exec/java-exec/src/main/java/org/apache/drill/exec/compile/MergeAdapter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/MergeAdapter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/MergeAdapter.java
index 1522102..82bd413 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/compile/MergeAdapter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/compile/MergeAdapter.java
@@ -17,8 +17,6 @@
*/
package org.apache.drill.exec.compile;
-import java.io.PrintWriter;
-import java.io.StringWriter;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Iterator;
@@ -39,7 +37,6 @@ import org.objectweb.asm.commons.SimpleRemapper;
import org.objectweb.asm.tree.ClassNode;
import org.objectweb.asm.tree.FieldNode;
import org.objectweb.asm.tree.MethodNode;
-import org.objectweb.asm.util.TraceClassVisitor;
import com.google.common.collect.Sets;
@@ -191,10 +188,10 @@ class MergeAdapter extends ClassVisitor {
public static MergedClassResult getMergedClass(final ClassSet set, final byte[] precompiledClass,
ClassNode generatedClass, final boolean scalarReplace) {
if (verifyBytecode) {
- if (!AsmUtil.isClassBytesOk(logger, precompiledClass, "precompiledClass")) {
+ if (!AsmUtil.isClassBytesOk(logger, "precompiledClass", precompiledClass)) {
throw new IllegalStateException("Problem found in precompiledClass");
}
- if ((generatedClass != null) && !AsmUtil.isClassOk(logger, generatedClass, "generatedClass")) {
+ if ((generatedClass != null) && !AsmUtil.isClassOk(logger, "generatedClass", generatedClass)) {
throw new IllegalStateException("Problem found in generatedClass");
}
}
@@ -225,7 +222,7 @@ class MergeAdapter extends ClassVisitor {
*/
generatedClass.accept(new ValueHolderReplacementVisitor(mergeGenerator, verifyBytecode));
if (verifyBytecode) {
- if (!AsmUtil.isClassOk(logger, generatedMerged, "generatedMerged")) {
+ if (!AsmUtil.isClassOk(logger, "generatedMerged", generatedMerged)) {
throw new IllegalStateException("Problem found with generatedMerged");
}
}
@@ -266,15 +263,13 @@ class MergeAdapter extends ClassVisitor {
}
}
-
- static class RemapClasses extends Remapper {
-
+ private static class RemapClasses extends Remapper {
final Set<String> innerClasses = Sets.newHashSet();
ClassSet top;
ClassSet current;
- public RemapClasses(ClassSet set) {
- super();
- this.current = set;
+
+ public RemapClasses(final ClassSet set) {
+ current = set;
ClassSet top = set;
while (top.parent != null) {
top = top.parent;
@@ -283,11 +278,9 @@ class MergeAdapter extends ClassVisitor {
}
@Override
- public String map(String typeName) {
-
+ public String map(final String typeName) {
// remap the names of all classes that start with the old class name.
if (typeName.startsWith(top.precompiled.slash)) {
-
// write down all the sub classes.
if (typeName.startsWith(current.precompiled.slash + "$")) {
innerClasses.add(typeName);
@@ -295,45 +288,12 @@ class MergeAdapter extends ClassVisitor {
return typeName.replace(top.precompiled.slash, top.generated.slash);
}
+
return typeName;
}
public Set<String> getInnerClasses() {
return innerClasses;
}
-
- }
-
- private static final void check(ClassNode node) {
- Exception e = null;
- String error = "";
-
- try {
- ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES);
- ClassVisitor cv = new DrillCheckClassAdapter(CompilationConfig.ASM_API_VERSION, cw, true);
- node.accept(cv);
-
- StringWriter sw = new StringWriter();
- PrintWriter pw = new PrintWriter(sw);
- DrillCheckClassAdapter.verify(new ClassReader(cw.toByteArray()), false, pw);
-
- error = sw.toString();
- } catch (Exception ex) {
- e = ex;
- }
-
- if (!error.isEmpty() || e != null) {
- StringWriter sw2 = new StringWriter();
- PrintWriter pw2 = new PrintWriter(sw2);
- TraceClassVisitor v = new TraceClassVisitor(pw2);
- node.accept(v);
- if (e != null) {
- throw new RuntimeException("Failure validating class. ByteCode: \n" +
- sw2.toString() + "\n\n====ERRROR====\n" + error, e);
- } else {
- throw new RuntimeException("Failure validating class. ByteCode: \n" +
- sw2.toString() + "\n\n====ERRROR====\n" + error);
- }
- }
}
}
http://git-wip-us.apache.org/repos/asf/drill/blob/7af5f9a0/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestBugFixes.java
----------------------------------------------------------------------
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestBugFixes.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestBugFixes.java
new file mode 100644
index 0000000..c316484
--- /dev/null
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestBugFixes.java
@@ -0,0 +1,84 @@
+/**
+ * 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.jdbc.test;
+
+import org.junit.Test;
+
+public class TestBugFixes extends JdbcTestQueryBase {
+// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestBugFixes.class);
+
+ @Test
+ public void testDrill2503() throws Exception {
+ final StringBuilder sb = new StringBuilder();
+ sb.append(
+ "SELECT "
+ + " CASE "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " WHEN 'y' = 'x' THEN 0 "
+ + " END "
+ + " FROM INFORMATION_SCHEMA.CATALOGS "
+ );
+
+ testQuery(sb.toString());
+ }
+}