HIVE-29258: Remove Class.forName() for driver loading #6207
Conversation
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
okumin
left a comment
okumin
left a comment
There was a problem hiding this comment.
We can amend some points, following SonarQube.
I wonder if we can remove some more. Probably, the answer is no.
% git grep 'Class.forName' | grep Driver
beeline/src/java/org/apache/hive/beeline/BeeLine.java: Driver driver = (Driver) Class.forName(clazzName, true,
beeline/src/java/org/apache/hive/beeline/Commands.java: Driver driver = (Driver) Class.forName(name).newInstance();
beeline/src/java/org/apache/hive/beeline/DatabaseConnection.java: (Driver) Class.forName(clazzName, true, Thread.currentThread().getContextClassLoader())
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java: classesToLoad.add(Class.forName("com.mysql.jdbc.Driver"));
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java: classesToLoad.add(Class.forName("org.postgresql.Driver"));
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java: classesToLoad.add(Class.forName("oracle.jdbc.OracleDriver"));
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java: classesToLoad.add(Class.forName("com.microsoft.sqlserver.jdbc.SQLServerDriver"));
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java: classesToLoad.add(Class.forName("com.ibm.db2.jcc.DB2Driver"));
ql/src/java/org/apache/hadoop/hive/ql/DriverFactory.java: Class<?> cls = Class.forName(name);
ql/src/test/org/apache/hadoop/hive/ql/TestDriverFactory.java: Class<?> cls = Class.forName(name);
If we can reduce the usage, is it possible to remove CONNECTION_DRIVER in HiveConf and MetastoreConf?
| public void setUpBefore() throws Exception { | ||
| if (miniHS2 == null) { | ||
| Class.forName(MiniHS2.getJdbcDriverName()); | ||
|
|
There was a problem hiding this comment.
An extra line has been added to some files and not been added to some files. I prefer to have no extra lines consistently
| } | ||
| try { | ||
| LOG.info("Going to load JDBC driver {}", driverName); | ||
| driver = (Driver) Class.forName(driverName).newInstance(); |
There was a problem hiding this comment.
This might change the behavior of L67 because this.driver is never assigned now
There was a problem hiding this comment.
Removed driver , connString as class attributes. Made connString local to method getConnection()
…ommands.java to leverage ServiceLoader
Have removed reflection code changes from |
|
3 participants
What changes were proposed in this pull request?
HIVE-29258
Why are the changes needed?
This PR removes explicit
Class.forNamecalls, leveraging the JDBC 4.0+ Service Provider Interface (SPI) for automatic driver discovery. TheDriverManagernow utilizesServiceLoaderto locate drivers defined inMETA-INF/servicesand registers them dynamically.Does this PR introduce any user-facing change?
NO
How was this patch tested?
Ran the affected files UT locally and will see CI outcome.