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


Reply via email to