HIVE-29258: Remove Class.forName() for driver loading (#6207)

* HIVE-29258: Remove Class.forName() for driver loading

* Addressing SonaType

* Addressing review comments

* Remove Reflection code in BeeLine.java, DatabaseConnection.java and Commands.java to leverage ServiceLoader

* Review comments

Author: Aggarwal-Raghav

beeline/src/java/org/apache/hive/beeline/BeeLine.java

@@ -71,7 +71,6 @@
 import java.util.ResourceBundle;
 import java.util.ServiceLoader;
 import java.util.Set;
-import java.util.SortedSet;
 import java.util.StringTokenizer;
 import java.util.TreeMap;
 import java.util.TreeSet;
@@ -310,12 +309,6 @@ public class BeeLine implements Closeable {
 
   private final Completer beeLineCommandCompleter = new BeeLineCommandCompleter(Arrays.asList(commandHandlers));
 
-  static final SortedSet<String> KNOWN_DRIVERS = new TreeSet<String>(Arrays.asList(
-      new String[] {
-          "org.apache.hive.jdbc.HiveDriver",
-          "org.apache.hadoop.hive.jdbc.HiveDriver",
-      }));
-
   static {
     try {
       Class.forName("org.jline.reader.LineReader");
@@ -2370,23 +2363,17 @@ private Driver findRegisteredDriver(String url) {
     return null;
   }
 
-  public Driver findLocalDriver(String url) throws Exception {
+  public Driver findLocalDriver(String url) throws SQLException {
     Objects.requireNonNull(url);
 
     Collection<Driver> currentDrivers = drivers == null ? Collections.emptyList() : drivers;
-    for (Driver d : currentDrivers) {
-      try {
-        String clazzName = d.getClass().getName();
-        Driver driver = (Driver) Class.forName(clazzName, true,
-          Thread.currentThread().getContextClassLoader()).newInstance();
-        if (driver.acceptsURL(url) && isSupportedLocalDriver(driver)) {
-          return driver;
-        }
-      } catch (SQLException e) {
-        throw e;
+    for (Driver driver : currentDrivers) {
+      // The 'driver' is already an instance from the ServiceLoader.
+      // We can use it directly without creating a new one via reflection.
+      if (driver.acceptsURL(url) && isSupportedLocalDriver(driver)) {
+        return driver;
       }
     }
-
     return null;
   }
 

beeline/src/java/org/apache/hive/beeline/Commands.java

@@ -44,6 +44,7 @@
 import java.sql.Statement;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -340,48 +341,48 @@ public boolean reconnect(String line) {
     return true;
   }
 
-
   public boolean scan(String line) throws IOException {
-    TreeSet<String> names = new TreeSet<String>();
-
     if (beeLine.getDrivers() == null) {
       beeLine.setDrivers(beeLine.scanDrivers());
     }
 
-    beeLine.info(beeLine.loc("drivers-found-count", beeLine.getDrivers().size()));
+    // Use a TreeSet to get a unique, sorted list of drivers by class name.
+    Set<Driver> drivers =
+        new TreeSet<>(Comparator.comparing(d -> d.getClass().getName()));
+    drivers.addAll(beeLine.getDrivers());
 
-    // unique the list
-    for (Iterator<Driver> i = beeLine.getDrivers().iterator(); i.hasNext();) {
-      names.add(i.next().getClass().getName());
-    }
+    // Get count of the unique driver in classpath
+    beeLine.info(beeLine.loc("drivers-found-count", drivers.size()));
 
-    beeLine.output(beeLine.getColorBuffer()
-        .bold(beeLine.getColorBuffer().pad(beeLine.loc("compliant"), 10).getMono())
-        .bold(beeLine.getColorBuffer().pad(beeLine.loc("jdbc-version"), 8).getMono())
-        .bold(beeLine.getColorBuffer(beeLine.loc("driver-class")).getMono()));
+    beeLine.output(
+        beeLine
+            .getColorBuffer()
+            .bold(beeLine.getColorBuffer().pad(beeLine.loc("compliant"), 10).getMono())
+            .bold(beeLine.getColorBuffer().pad(beeLine.loc("jdbc-version"), 8).getMono())
+            .bold(beeLine.getColorBuffer(beeLine.loc("driver-class")).getMono()));
 
-    for (Iterator<String> i = names.iterator(); i.hasNext();) {
-      String name = i.next().toString();
+    for (Driver driver : drivers) {
+      String name = driver.getClass().getName();
       try {
-        Driver driver = (Driver) Class.forName(name).newInstance();
-        ColorBuffer msg = beeLine.getColorBuffer()
-            .pad(driver.jdbcCompliant() ? "yes" : "no", 10)
-            .pad(driver.getMajorVersion() + "."
-                + driver.getMinorVersion(), 8)
-            .append(name);
+        // Use the driver instance that ServiceLoader already created for us.
+        ColorBuffer msg =
+            beeLine
+                .getColorBuffer()
+                .pad(driver.jdbcCompliant() ? "yes" : "no", 10)
+                .pad(driver.getMajorVersion() + "." + driver.getMinorVersion(), 8)
+                .append(name);
         if (driver.jdbcCompliant()) {
           beeLine.output(msg);
         } else {
           beeLine.output(beeLine.getColorBuffer().red(msg.getMono()));
         }
       } catch (Throwable t) {
-        beeLine.output(beeLine.getColorBuffer().red(name)); // error with driver
+        beeLine.error("Error processing driver " + name);
       }
     }
     return true;
   }
 
-
   public boolean save(String line) throws IOException {
     beeLine.info(beeLine.loc("saving-options", beeLine.getOpts().getPropertiesFile()));
     beeLine.getOpts().save();
@@ -1670,7 +1671,7 @@ public boolean connect(Properties props) throws IOException {
 
     try {
       beeLine.getDatabaseConnections().setConnection(
-          new DatabaseConnection(beeLine, driver, url, props));
+          new DatabaseConnection(beeLine, url, props));
       beeLine.getDatabaseConnection().getConnection();
 
       if (!beeLine.isBeeLine()) {

beeline/src/java/org/apache/hive/beeline/DatabaseConnection.java

@@ -49,7 +49,6 @@ class DatabaseConnection {
   private final BeeLine beeLine;
   private Connection connection;
   private DatabaseMetaData meta;
-  private final String driver;
   private final String url;
   private final Properties info;
   private Schema schema = null;
@@ -59,10 +58,8 @@ public boolean isClosed() {
     return (null == connection);
   }
 
-  public DatabaseConnection(BeeLine beeLine, String driver, String url,
-       Properties info) throws SQLException {
+  DatabaseConnection(BeeLine beeLine, String url, Properties info) {
     this.beeLine = beeLine;
-    this.driver = driver;
     this.url = url;
     this.info = info;
   }
@@ -85,14 +82,6 @@ void setCompletions(boolean skipmeta) throws SQLException, IOException {
    * Connection to the specified data source.
    */
   boolean connect() throws SQLException {
-    try {
-      if (driver != null && driver.length() != 0) {
-        Class.forName(driver);
-      }
-    } catch (ClassNotFoundException cnfe) {
-      return beeLine.error(cnfe);
-    }
-
     boolean isDriverRegistered = false;
     try {
       isDriverRegistered = DriverManager.getDriver(getUrl()) != null;
@@ -163,18 +152,19 @@ boolean connect() throws SQLException {
 
   public Connection getConnectionFromLocalDriver(String url, Properties properties) {
     Collection<Driver> drivers = beeLine.getDrivers();
-    for (Driver d : drivers) {
+    for (Driver driver : drivers) {
       try {
-        if (d.acceptsURL(url) && beeLine.isSupportedLocalDriver(d)) {
-          String clazzName = d.getClass().getName();
-          beeLine.debug("Driver name is " + clazzName);
-          Driver driver =
-            (Driver) Class.forName(clazzName, true, Thread.currentThread().getContextClassLoader())
-              .newInstance();
+        if (driver.acceptsURL(url) && beeLine.isSupportedLocalDriver(driver)) {
+          beeLine.debug("Driver name is " + driver.getClass().getName());
+          // The 'driver' is already an instance from the ServiceLoader, so we can use it directly.
           return driver.connect(url, properties);
         }
-      } catch (Exception e) {
-        beeLine.error("Fail to connect with a local driver due to the exception:" + e);
+      } catch (SQLException e) {
+        beeLine.error(
+            "Failed to connect with local driver "
+                + driver.getClass().getName()
+                + " due to exception: "
+                + e);
         beeLine.error(e);
       }
     }

beeline/src/test/org/apache/hive/beeline/ProxyAuthTest.java

@@ -44,7 +44,6 @@
  *      <HS2host> <HS2Port> <HS2-Server-principal> <client-principal>
  */
 public class ProxyAuthTest {
-  private static final String driverName = "org.apache.hive.jdbc.HiveDriver";
   private static final String BEELINE_EXIT = "beeline.system.exit";
   private static Connection con = null;
   private static boolean noClose = false;
@@ -68,7 +67,6 @@ public static void main(String[] args) throws Exception {
     File currentResultFile = null;
     String [] beeLineArgs = {};
 
-    Class.forName(driverName);
     String host = args[0];
     String port = args[1];
     String serverPrincipal = args[2];

beeline/src/test/org/apache/hive/beeline/TestShutdownHook.java

@@ -30,11 +30,11 @@ public void testShutdownHook() throws Exception {
     PrintStream ops = new PrintStream(os);
     BeeLine beeline = new BeeLine();
     DatabaseConnections dbConnections = beeline.getDatabaseConnections();
-    dbConnections.setConnection(new DatabaseConnection(beeline,null,null, null));
-    dbConnections.setConnection(new DatabaseConnection(beeline,null,null, null));
+    dbConnections.setConnection(new DatabaseConnection(beeline, null, null));
+    dbConnections.setConnection(new DatabaseConnection(beeline, null, null));
     Assert.assertEquals(2, dbConnections.size());
     beeline.setOutputStream(ops);
     beeline.getShutdownHook().run();
     Assert.assertEquals(0, dbConnections.size());
   }
-}
+}

hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/tool/TempletonControllerJob.java

@@ -218,11 +218,6 @@ private String buildHS2DelegationToken(String user) throws IOException, Interrup
     return real.doAs(new PrivilegedExceptionAction<String>() {
       @Override
       public String run() throws IOException, TException, InterruptedException {
-        try {
-          Class.forName("org.apache.hive.jdbc.HiveDriver");
-        } catch (ClassNotFoundException e) {
-          throw new IOException(e);
-        }
         String hs2Url = appConf.get(AppConfig.HIVE_SERVER2_URL);
         final HiveConnection con;
         try {

hplsql/src/main/java/org/apache/hive/hplsql/Conn.java

@@ -146,15 +146,11 @@ synchronized Connection getConnection(String connName) throws Exception {
    * @throws Exception 
    */
   Connection openConnection(String connStr) throws Exception {
-    String driver = "org.apache.hadoop.hive.jdbc.HiveDriver";
     StringBuilder url = new StringBuilder();
     String usr = "";
     String pwd = "";
     if (connStr != null) {
       String[] c = connStr.split(";");
-      if (c.length >= 1) {
-        driver = c[0];
-      }
       if (c.length >= 2) {
         url.append(c[1]);
       }
@@ -174,7 +170,6 @@ else if (pwd.isEmpty()) {
         }
       }
     }
-    Class.forName(driver);
     timer.start();
     Connection conn = DriverManager.getConnection(url.toString().trim(), usr, pwd);
     timer.stop();

itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/JdbcWithMiniKdcSQLAuthTest.java

@@ -51,7 +51,6 @@ public static void beforeTestBase(String transportMode) throws Exception {
     hiveConf.setVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE, transportMode);
     System.err.println("Testing using HS2 mode:" + transportMode);
 
-    Class.forName(MiniHS2.getJdbcDriverName());
     hiveConf.setVar(ConfVars.HIVE_AUTHORIZATION_MANAGER,
         SQLStdHiveAuthorizerFactory.class.getName());
     hiveConf.setVar(ConfVars.HIVE_AUTHENTICATOR_MANAGER,

itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestHS2AuthMechsWithMiniKdc.java

@@ -87,7 +87,6 @@ public class TestHS2AuthMechsWithMiniKdc extends AbstractLdapTestUnit {
   @Before
   public void setUpBefore() throws Exception {
     if (miniHS2 == null) {
-      Class.forName(MiniHS2.getJdbcDriverName());
       miniHiveKdc = new MiniHiveKdc();
       HiveConf hiveConf = new HiveConf();
       hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_SERVER2_ENABLE_DOAS, false);

itests/hive-minikdc/src/test/java/org/apache/hive/minikdc/TestHS2JWTWithMiniKdc.java

@@ -73,7 +73,6 @@ public class TestHS2JWTWithMiniKdc {
 
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
-    Class.forName(MiniHS2.getJdbcDriverName());
     MOCK_JWKS_SERVER.stubFor(get("/jwks")
         .willReturn(ok()
             .withBody(Files.readAllBytes(jwtVerificationJWKSFile.toPath()))));