CalvinKirs commented on code in PR #43059:
URL: https://github.com/apache/doris/pull/43059#discussion_r1825502838
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java:
##########
@@ -294,30 +296,33 @@ private boolean isConvertDatetimeToNull(JdbcClientConfig
jdbcClientConfig) {
* get all columns like DatabaseMetaData.getColumns in mysql-jdbc-connector
*/
private Map<String, String> getColumnsDataTypeUseQuery(String
remoteDbName, String remoteTableName) {
- Connection conn = getConnection();
+ Connection conn = null;
+ Statement stmt = null;
ResultSet resultSet = null;
- Map<String, String> fieldtoType = Maps.newHashMap();
+ Map<String, String> fieldToType = Maps.newHashMap();
StringBuilder queryBuf = new StringBuilder("SHOW FULL COLUMNS FROM ");
queryBuf.append(remoteTableName);
queryBuf.append(" FROM ");
queryBuf.append(remoteDbName);
- try (Statement stmt = conn.createStatement()) {
+ try {
+ conn = getConnection();
+ stmt = conn.createStatement();
resultSet = stmt.executeQuery(queryBuf.toString());
while (resultSet.next()) {
// get column name
String fieldName = resultSet.getString("Field");
// get original type name
String typeName = resultSet.getString("Type");
- fieldtoType.put(fieldName, typeName);
+ fieldToType.put(fieldName, typeName);
}
} catch (SQLException e) {
throw new JdbcClientException("failed to get jdbc columns info for
remote table `%s.%s`: %s",
remoteDbName, remoteTableName,
Util.getRootCauseMessage(e));
} finally {
- close(resultSet, conn);
+ close(resultSet, stmt, conn);
}
- return fieldtoType;
+ return fieldToType;
Review Comment:
Using try-with-resources is preferable; otherwise, you have to be extra
careful about potential null pointers. In a finally block, you’d need to check
if each item is null before closing it,
```
finally {
// Null check before closing each resource
if (resultSet != null) {
try {
resultSet.close();
} catch (SQLException e) {
LOG.warn("Failed to close ResultSet", e);
}
}
if (stmt != null) {
try {
stmt.close();
} catch (SQLException e) {
LOG.warn("Failed to close Statement", e);
}
}
if (conn != null) {
try {
conn.close();
} catch (SQLException e) {
LOG.warn("Failed to close Connection", e);
}
}
}
```
which makes the code messy.
```
try (Connection conn = getConnection();
Statement stmt = conn.createStatement();
ResultSet resultSet = stmt.executeQuery(queryBuf.toString())) {
while (resultSet.next()) {
String fieldName = resultSet.getString("Field");
String typeName = resultSet.getString("Type");
fieldtoType.put(fieldName, typeName);
}
} catch (SQLException e) {
throw new JdbcClientException(String.format("failed to get jdbc columns
info for remote table `%s.%s`: %s",
remoteDbName, remoteTableName, Util.getRootCauseMessage(e)), e);
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
