kezhuw commented on a change in pull request #15104: URL: https://github.com/apache/flink/pull/15104#discussion_r589166185
########## File path: flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/internal/connection/SimpleJdbcConnectionProvider.java ########## @@ -100,7 +100,16 @@ private Driver getDriver() throws SQLException, ClassNotFoundException { @Override public Connection getOrEstablishConnection() throws SQLException, ClassNotFoundException { - if (connection == null) { + if (connection != null) { + return connection; + } + if (jdbcOptions.getDriverName() == null) { + connection = + DriverManager.getConnection( + jdbcOptions.getDbURL(), + jdbcOptions.getUsername().orElse(null), + jdbcOptions.getPassword().orElse(null)); + } else { Review comment: Hi @wuchong, I pushed a fixup commit with following work: * Separate a static method `loadDriver(String driverName)` to load driver and do not-null checking there. * Keep signature of old no-static `getDriver`(renamed to `getLoadedDriver`) unchanged. I think a `driverName` parameter in no-static method might give maintainers illusion that it could be used to load a different driver while it is not. Please check whether it solves your concern ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org