You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by vo...@apache.org on 2019/12/22 11:40:44 UTC

[drill] branch master updated (ecd8c81 -> 44f408c)

This is an automated email from the ASF dual-hosted git repository.

volodymyr pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git.


    from ecd8c81  DRILL-7485: NPE on PCAP Batch Reader
     new f258eb5  DRILL-7490: LIMIT is not pushed to JDBC storage plugin
     new 44f408c  DRILL-7494: Unable to connect to Drill using JDBC driver when using custom authenticator

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/drill/common/util/GuavaPatcher.java | 21 +++++++++++--
 .../apache/drill/common/util/ProtobufPatcher.java  | 27 +++++++++++++---
 .../exec/store/jdbc/TestJdbcPluginWithMySQLIT.java | 36 +++++++++++++---------
 .../exec/planner/cost/DrillRelMdRowCount.java      | 20 +-----------
 .../org/apache/drill/exec/rpc/user/UserClient.java |  4 +--
 exec/jdbc-all/pom.xml                              |  3 +-
 .../main/java/org/apache/drill/jdbc/Driver.java    |  7 +++--
 .../drill/exec/rpc/ConnectionMultiListener.java    | 10 +++---
 pom.xml                                            |  2 +-
 9 files changed, 77 insertions(+), 53 deletions(-)


[drill] 02/02: DRILL-7494: Unable to connect to Drill using JDBC driver when using custom authenticator

Posted by vo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

volodymyr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 44f408ce38bff52f4922cd6cf2387bd07d888e3e
Author: Volodymyr Vysotskyi <vv...@gmail.com>
AuthorDate: Sat Dec 21 16:14:55 2019 +0200

    DRILL-7494: Unable to connect to Drill using JDBC driver when using custom authenticator
    
    closes #1938
---
 .../org/apache/drill/common/util/GuavaPatcher.java | 21 ++++++++++++++---
 .../apache/drill/common/util/ProtobufPatcher.java  | 27 ++++++++++++++++++----
 .../org/apache/drill/exec/rpc/user/UserClient.java |  4 ++--
 exec/jdbc-all/pom.xml                              |  3 +--
 .../main/java/org/apache/drill/jdbc/Driver.java    |  7 ++++--
 .../drill/exec/rpc/ConnectionMultiListener.java    | 10 ++++----
 pom.xml                                            |  2 +-
 7 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java b/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java
index 5924df8..a618d05 100644
--- a/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java
+++ b/common/src/main/java/org/apache/drill/common/util/GuavaPatcher.java
@@ -29,6 +29,8 @@ import javassist.CtClass;
 import javassist.CtConstructor;
 import javassist.CtMethod;
 import javassist.CtNewMethod;
+import javassist.scopedpool.ScopedClassPoolRepository;
+import javassist.scopedpool.ScopedClassPoolRepositoryImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -51,7 +53,7 @@ public class GuavaPatcher {
    */
   private static void patchStopwatch() {
     try {
-      ClassPool cp = ClassPool.getDefault();
+      ClassPool cp = getClassPool();
       CtClass cc = cp.get("com.google.common.base.Stopwatch");
 
       // Expose the constructor for Stopwatch for old libraries who use the pattern new Stopwatch().start().
@@ -77,7 +79,7 @@ public class GuavaPatcher {
 
   private static void patchCloseables() {
     try {
-      ClassPool cp = ClassPool.getDefault();
+      ClassPool cp = getClassPool();
       CtClass cc = cp.get("com.google.common.io.Closeables");
 
       // Add back the Closeables.closeQuietly() method for old consumers.
@@ -100,7 +102,7 @@ public class GuavaPatcher {
    */
   private static void patchPreconditions() {
     try {
-      ClassPool cp = ClassPool.getDefault();
+      ClassPool cp = getClassPool();
       CtClass cc = cp.get("com.google.common.base.Preconditions");
 
       // Javassist does not support varargs, generate methods with varying number of arguments
@@ -192,4 +194,17 @@ public class GuavaPatcher {
       logger.warn("Unable to patch Guava classes.", e);
     }
   }
+
+  /**
+   * Returns {@link javassist.scopedpool.ScopedClassPool} instance which uses the same class loader
+   * which was used for loading current class.
+   *
+   * @return {@link javassist.scopedpool.ScopedClassPool} instance
+   */
+  private static ClassPool getClassPool() {
+    ScopedClassPoolRepository classPoolRepository = ScopedClassPoolRepositoryImpl.getInstance();
+    // sets prune flag to false to avoid freezing and pruning classes right after obtaining CtClass instance
+    classPoolRepository.setPrune(false);
+    return classPoolRepository.createScopedClassPool(GuavaPatcher.class.getClassLoader(), null);
+  }
 }
diff --git a/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java b/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java
index 3556eec..f8b6c51 100644
--- a/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java
+++ b/common/src/main/java/org/apache/drill/common/util/ProtobufPatcher.java
@@ -25,6 +25,8 @@ import javassist.CtMethod;
 import javassist.CtNewConstructor;
 import javassist.CtNewMethod;
 import javassist.Modifier;
+import javassist.scopedpool.ScopedClassPoolRepository;
+import javassist.scopedpool.ScopedClassPoolRepositoryImpl;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -54,7 +56,7 @@ public class ProtobufPatcher {
    */
   private static void patchByteString() {
     try {
-      ClassPool classPool = ClassPool.getDefault();
+      ClassPool classPool = getClassPool();
       CtClass byteString = classPool.get("com.google.protobuf.ByteString");
       removeFinal(byteString.getDeclaredMethod("toString"));
       removeFinal(byteString.getDeclaredMethod("hashCode"));
@@ -92,7 +94,7 @@ public class ProtobufPatcher {
    */
   private static void patchGeneratedMessageLite() {
     try {
-      ClassPool classPool = ClassPool.getDefault();
+      ClassPool classPool = getClassPool();
       CtClass generatedMessageLite = classPool.get("com.google.protobuf.GeneratedMessageLite");
       removeFinal(generatedMessageLite.getDeclaredMethod("getParserForType"));
       removeFinal(generatedMessageLite.getDeclaredMethod("isInitialized"));
@@ -102,7 +104,8 @@ public class ProtobufPatcher {
       generatedMessageLite.addMethod(CtNewMethod.make("protected void makeExtensionsImmutable() { }", generatedMessageLite));
 
       // A constructor with this signature was removed. Adding it back.
-      generatedMessageLite.addConstructor(CtNewConstructor.make("protected GeneratedMessageLite(com.google.protobuf.GeneratedMessageLite.Builder builder) { }", generatedMessageLite));
+      String className = "com.google.protobuf.GeneratedMessageLite.Builder";
+      generatedMessageLite.addConstructor(CtNewConstructor.make("protected GeneratedMessageLite(" + className + " builder) { }", generatedMessageLite));
 
       // This single method was added instead of several abstract methods.
       // MapR-DB client doesn't use it, but it was added in overridden equals() method.
@@ -111,7 +114,8 @@ public class ProtobufPatcher {
           classPool.get("com.google.protobuf.GeneratedMessageLite$MethodToInvoke"),
           classPool.get("java.lang.Object"),
           classPool.get("java.lang.Object")});
-      addImplementation(dynamicMethod, "if ($1.equals(com.google.protobuf.GeneratedMessageLite.MethodToInvoke.GET_DEFAULT_INSTANCE)) {" +
+      className = "com.google.protobuf.GeneratedMessageLite.MethodToInvoke";
+      addImplementation(dynamicMethod, "if ($1.equals(" + className + ".GET_DEFAULT_INSTANCE)) {" +
           "  return this;" +
           "} else {" +
           "  return null;" +
@@ -130,7 +134,7 @@ public class ProtobufPatcher {
    */
   private static void patchGeneratedMessageLiteBuilder() {
     try {
-      ClassPool classPool = ClassPool.getDefault();
+      ClassPool classPool = getClassPool();
       CtClass builder = classPool.get("com.google.protobuf.GeneratedMessageLite$Builder");
       removeFinal(builder.getDeclaredMethod("isInitialized"));
       removeFinal(builder.getDeclaredMethod("clear"));
@@ -183,4 +187,17 @@ public class ProtobufPatcher {
     int modifiers = Modifier.clear(ctMethod.getModifiers(), Modifier.ABSTRACT);
     ctMethod.setModifiers(modifiers);
   }
+
+  /**
+   * Returns {@link javassist.scopedpool.ScopedClassPool} instance which uses the same class loader
+   * which was used for loading current class.
+   *
+   * @return {@link javassist.scopedpool.ScopedClassPool} instance
+   */
+  private static ClassPool getClassPool() {
+    ScopedClassPoolRepository classPoolRepository = ScopedClassPoolRepositoryImpl.getInstance();
+    // sets prune flag to false to avoid freezing and pruning classes right after obtaining CtClass instance
+    classPoolRepository.setPrune(false);
+    return classPoolRepository.createScopedClassPool(ProtobufPatcher.class.getClassLoader(), null);
+  }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java
index dcb599b..c215a93 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java
@@ -454,8 +454,8 @@ public class UserClient
       Thread.currentThread().setContextClassLoader(oldThreadCtxtCL);
 
       startSaslHandshake(connectionHandler, saslProperties, ugi, factory, RpcType.SASL_MESSAGE);
-    } catch (final IOException e) {
-      logger.error("Failed while doing setup for starting SASL handshake for connection", connection.getName());
+    } catch (IOException e) {
+      logger.error("Failed while doing setup for starting SASL handshake for connection {}", connection.getName());
       final Exception ex = new RpcException(String.format("Failed to initiate authentication for connection %s",
         connection.getName()), e);
       connectionHandler.connectionFailed(RpcConnectionHandler.FailureType.AUTHENTICATION, ex);
diff --git a/exec/jdbc-all/pom.xml b/exec/jdbc-all/pom.xml
index ab6bf7b..a4bd523 100644
--- a/exec/jdbc-all/pom.xml
+++ b/exec/jdbc-all/pom.xml
@@ -342,7 +342,6 @@
               <exclude>commons-beanutils:commons-beanutils-core:jar:*</exclude>
               <exclude>commons-beanutils:commons-beanutils:jar:*</exclude>
               <exclude>io.netty:netty-tcnative:jar:*</exclude>
-              <exclude>com.fasterxml.woodstox:woodstox-core:jar:*</exclude>
               <exclude>com.google.code.findbugs:jsr305:*</exclude>
               <exclude>com.esri.geometry:esri-geometry-api:*</exclude>
               <exclude>fr.bmartel:pcapngdecoder:*</exclude>
@@ -409,7 +408,7 @@
             <relocation><pattern>org.apache.xpath.</pattern><shadedPattern>oadd.org.apache.xpath.</shadedPattern></relocation>
             <relocation><pattern>org.apache.zookeeper.</pattern><shadedPattern>oadd.org.apache.zookeeper.</shadedPattern></relocation>
             <relocation><pattern>org.apache.hadoop.</pattern><shadedPattern>oadd.org.apache.hadoop.</shadedPattern></relocation>
-            <relocation><pattern>com.fasterxml.woodstox.</pattern><shadedPattern>oadd.com.fasterxml.woodstox.</shadedPattern></relocation>
+            <relocation><pattern>com.ctc.wstx.</pattern><shadedPattern>oadd.com.ctc.wstx.</shadedPattern></relocation>
           </relocations>
           <transformers>
             <transformer implementation="org.apache.maven.plugins.shade.resource.AppendingTransformer">
diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java
index 39e0ad1..6b8dd32 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java
@@ -21,9 +21,10 @@ import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.DriverPropertyInfo;
 import java.sql.SQLException;
-import java.sql.SQLFeatureNotSupportedException;
 import java.util.Properties;
 
+import org.apache.drill.common.util.GuavaPatcher;
+import org.apache.drill.common.util.ProtobufPatcher;
 import org.apache.drill.jdbc.impl.DriverImpl;
 
 
@@ -42,6 +43,8 @@ public class Driver implements java.sql.Driver {
   // DriverManager access it:
 
   static {
+    ProtobufPatcher.patch();
+    GuavaPatcher.patch();
     // Upon loading of class, register an instance with DriverManager.
     try {
       DriverManager.registerDriver(new Driver());
@@ -100,7 +103,7 @@ public class Driver implements java.sql.Driver {
   }
 
   @Override
-  public java.util.logging.Logger getParentLogger() throws SQLFeatureNotSupportedException {
+  public java.util.logging.Logger getParentLogger() {
     return impl.getParentLogger();
   }
 
diff --git a/exec/rpc/src/main/java/org/apache/drill/exec/rpc/ConnectionMultiListener.java b/exec/rpc/src/main/java/org/apache/drill/exec/rpc/ConnectionMultiListener.java
index 3fee5d7..3944a2c 100644
--- a/exec/rpc/src/main/java/org/apache/drill/exec/rpc/ConnectionMultiListener.java
+++ b/exec/rpc/src/main/java/org/apache/drill/exec/rpc/ConnectionMultiListener.java
@@ -77,7 +77,7 @@ public class ConnectionMultiListener<T extends EnumLite, CC extends ClientConnec
   private class ConnectionHandler implements GenericFutureListener<ChannelFuture> {
 
     @Override
-    public void operationComplete(ChannelFuture future) throws Exception {
+    public void operationComplete(ChannelFuture future) {
       boolean isInterrupted = false;
 
       // We want to wait for at least 120 secs when interrupts occur. Establishing a connection fails/succeeds quickly,
@@ -131,7 +131,7 @@ public class ConnectionMultiListener<T extends EnumLite, CC extends ClientConnec
 
   private class SSLConnectionHandler implements GenericFutureListener<Future<Channel>> {
     @Override
-    public void operationComplete(Future<Channel> future) throws Exception {
+    public void operationComplete(Future<Channel> future) {
       // send the handshake
       parent.send(handshakeSendHandler, handshakeValue, true);
     }
@@ -165,9 +165,9 @@ public class ConnectionMultiListener<T extends EnumLite, CC extends ClientConnec
       } catch (NonTransientRpcException ex) {
         logger.error("Failure while validating client and server sasl compatibility", ex);
         connectionListener.connectionFailed(RpcConnectionHandler.FailureType.AUTHENTICATION, ex);
-      } catch (Exception ex) {
-        logger.error("Failure while validating handshake", ex);
-        connectionListener.connectionFailed(RpcConnectionHandler.FailureType.HANDSHAKE_VALIDATION, ex);
+      } catch (Throwable t) {
+        logger.error("Failure while validating handshake", t);
+        connectionListener.connectionFailed(RpcConnectionHandler.FailureType.HANDSHAKE_VALIDATION, t);
       }
     }
 
diff --git a/pom.xml b/pom.xml
index fb6735c..224b615 100644
--- a/pom.xml
+++ b/pom.xml
@@ -66,7 +66,7 @@
     <zookeeper.version>3.4.12</zookeeper.version>
     <mapr.release.version>6.1.0-mapr</mapr.release.version>
     <ojai.version>3.0-mapr-1808</ojai.version>
-    <kerby.version>1.0.0-RC2</kerby.version>
+    <kerby.version>1.0.0</kerby.version>
     <findbugs.version>3.0.0</findbugs.version>
     <netty.tcnative.classifier />
     <commons.io.version>2.4</commons.io.version>


[drill] 01/02: DRILL-7490: LIMIT is not pushed to JDBC storage plugin

Posted by vo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

volodymyr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit f258eb54a8181d8a375317f60b8b96d6a356358c
Author: Volodymyr Vysotskyi <vv...@gmail.com>
AuthorDate: Wed Dec 18 19:16:08 2019 +0200

    DRILL-7490: LIMIT is not pushed to JDBC storage plugin
    
    closes #1936
---
 .../exec/store/jdbc/TestJdbcPluginWithMySQLIT.java | 36 +++++++++++++---------
 .../exec/planner/cost/DrillRelMdRowCount.java      | 20 +-----------
 2 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java b/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java
index bd065ca..6253d11 100644
--- a/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java
+++ b/contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java
@@ -37,10 +37,7 @@ import org.junit.experimental.categories.Category;
 
 import java.math.BigDecimal;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
 
 /**
  * JDBC storage plugin tests against MySQL.
@@ -195,11 +192,12 @@ public class TestJdbcPluginWithMySQLIT extends ClusterTest {
   @Test
   public void pushdownJoin() throws Exception {
     String query = "select x.person_id from (select person_id from mysql.`drill_mysql_test`.person) x "
-            + "join (select person_id from mysql.`drill_mysql_test`.person) y on x.person_id = y.person_id ";
-    String plan = queryBuilder().sql(query).explainText();
-
-    assertThat("Query plan shouldn't contain Join operator",
-        plan, not(containsString("Join")));
+            + "join (select person_id from mysql.`drill_mysql_test`.person) y on x.person_id = y.person_id";
+    queryBuilder()
+        .sql(query)
+        .planMatcher()
+        .exclude("Join")
+        .match();
   }
 
   @Test
@@ -211,12 +209,11 @@ public class TestJdbcPluginWithMySQLIT extends ClusterTest {
             "ON e.first_name = s.first_name " +
             "WHERE e.last_name > 'hello'";
 
-    String plan = queryBuilder().sql(query).explainText();
-
-    assertThat("Query plan shouldn't contain Join operator",
-        plan, not(containsString("Join")));
-    assertThat("Query plan shouldn't contain Filter operator",
-        plan, not(containsString("Filter")));
+    queryBuilder()
+        .sql(query)
+        .planMatcher()
+        .exclude("Join", "Filter")
+        .match();
   }
 
   @Test
@@ -341,4 +338,15 @@ public class TestJdbcPluginWithMySQLIT extends ClusterTest {
         .baselineValuesForSingleColumn("SYSTEM VIEW", "TABLE", "VIEW")
         .go();
   }
+
+  @Test
+  public void testLimitPushDown() throws Exception {
+    String query = "select person_id from mysql.`drill_mysql_test`.person limit 10";
+    queryBuilder()
+        .sql(query)
+        .planMatcher()
+        .include("Jdbc\\(.*LIMIT 10")
+        .exclude("Limit\\(")
+        .match();
+  }
 }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdRowCount.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdRowCount.java
index a24f5d1..0bfb70a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdRowCount.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/cost/DrillRelMdRowCount.java
@@ -18,12 +18,9 @@
 package org.apache.drill.exec.planner.cost;
 
 import java.io.IOException;
-import org.apache.calcite.rel.SingleRel;
 import org.apache.calcite.rel.core.Aggregate;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Join;
-import org.apache.calcite.rel.core.Project;
-import org.apache.calcite.rel.core.Sort;
 import org.apache.calcite.rel.core.TableScan;
 import org.apache.calcite.rel.core.Union;
 import org.apache.calcite.rel.metadata.ReflectiveRelMetadataProvider;
@@ -59,7 +56,7 @@ public class DrillRelMdRowCount extends RelMdRowCount{
           ((AggPrelBase) rel).getOperatorPhase() == AggPrelBase.OperatorPhase.PHASE_1of2) {
       // Phase 1 Aggregate would return rows in the range [NDV, input_rows]. Hence, use the
       // existing estimate of 1/10 * input_rows
-        Double rowCount = mq.getRowCount(rel.getInput()) / 10;
+        double rowCount = mq.getRowCount(rel.getInput()) / 10;
         Double ndv = mq.getDistinctRowCount(rel.getInput(), groupKey, null);
         // Use max of NDV and input_rows/10
         if (ndv != null) {
@@ -83,21 +80,6 @@ public class DrillRelMdRowCount extends RelMdRowCount{
   }
 
   @Override
-  public Double getRowCount(Project rel, RelMetadataQuery mq) {
-    return rel.estimateRowCount(mq);
-  }
-
-  @Override
-  public Double getRowCount(Sort rel, RelMetadataQuery mq) {
-    return rel.estimateRowCount(mq);
-  }
-
-  @Override
-  public Double getRowCount(SingleRel rel, RelMetadataQuery mq) {
-    return rel.estimateRowCount(mq);
-  }
-
-  @Override
   public Double getRowCount(Join rel, RelMetadataQuery mq) {
     return rel.estimateRowCount(mq);
   }