Copilot commented on code in PR #16771:
URL: https://github.com/apache/iotdb/pull/16771#discussion_r2533348083
##########
integration-test/src/test/java/org/apache/iotdb/pipe/it/autocreate/IoTDBPipeLifeCycleIT.java:
##########
@@ -47,8 +47,6 @@
import static org.apache.iotdb.db.it.utils.TestUtils.createUser;
import static org.apache.iotdb.db.it.utils.TestUtils.executeQueryWithRetry;
import static org.apache.iotdb.db.it.utils.TestUtils.grantUserSystemPrivileges;
Review Comment:
The static imports for `tryExecuteNonQueriesWithRetry` and
`tryExecuteNonQueryWithRetry` are removed, but these methods are still being
used on lines 778, 790, and 803 through the `TestUtils` class prefix. This is
inconsistent - either the imports should be kept or all usages should use the
`TestUtils.` prefix consistently.
```suggestion
import static
org.apache.iotdb.db.it.utils.TestUtils.grantUserSystemPrivileges;
import static
org.apache.iotdb.db.it.utils.TestUtils.tryExecuteNonQueriesWithRetry;
import static
org.apache.iotdb.db.it.utils.TestUtils.tryExecuteNonQueryWithRetry;
```
##########
integration-test/src/test/java/org/apache/iotdb/db/it/utils/TestUtils.java:
##########
@@ -491,6 +491,60 @@ public static boolean tryExecuteNonQueriesWithRetry(
return false;
}
+ public static void executeNonQuery(BaseEnv env, String sql, Connection
defaultConnection) {
+ executeNonQuery(
+ env, sql, SessionConfig.DEFAULT_USER, SessionConfig.DEFAULT_PASSWORD,
defaultConnection);
+ }
+
+ public static void executeNonQuery(
+ BaseEnv env, String sql, String userName, String password, Connection
defaultConnection) {
+ executeNonQueries(env, Collections.singletonList(sql), userName, password,
defaultConnection);
+ }
+
+ public static void executeNonQueries(
+ BaseEnv env, List<String> sqlList, Connection defaultConnection) {
+ executeNonQueries(
+ env,
+ sqlList,
+ SessionConfig.DEFAULT_USER,
+ SessionConfig.DEFAULT_PASSWORD,
+ defaultConnection);
+ }
+
+ public static void executeNonQueries(
+ BaseEnv env,
+ List<String> sqlList,
+ String userName,
+ String password,
+ Connection defaultConnection) {
+ int lastIndex = 0;
+ Connection localConnection = null;
+ Connection connectionToUse = defaultConnection;
+ Statement statement;
+ try {
+ // create a new connection if default is not provided or the previous is
broken
+ if (connectionToUse == null) {
+ localConnection = env.getConnection(userName, password);
+ connectionToUse = localConnection;
+ }
+ statement = connectionToUse.createStatement();
+ for (int i = lastIndex; i < sqlList.size(); ++i) {
+ statement.execute(sqlList.get(i));
+ }
+ } catch (SQLException e) {
+ // the default connection should be closed by the upper level
+ // while the local connection should be closed here
+ if (connectionToUse == localConnection && localConnection != null) {
+ try {
+ localConnection.close();
+ } catch (SQLException ex) {
+ // ignore
+ }
+ }
+ throw new RuntimeException(e);
+ }
Review Comment:
The new `executeNonQuery` and `executeNonQueries` methods throw
`RuntimeException` wrapping `SQLException` on failure (line 544), but they
don't close the `Statement` object created on line 530. This could lead to
resource leaks. The statement should be closed in a finally block or using
try-with-resources.
##########
integration-test/src/test/java/org/apache/iotdb/db/it/utils/TestUtils.java:
##########
@@ -491,6 +491,60 @@ public static boolean tryExecuteNonQueriesWithRetry(
return false;
}
+ public static void executeNonQuery(BaseEnv env, String sql, Connection
defaultConnection) {
+ executeNonQuery(
+ env, sql, SessionConfig.DEFAULT_USER, SessionConfig.DEFAULT_PASSWORD,
defaultConnection);
+ }
+
+ public static void executeNonQuery(
+ BaseEnv env, String sql, String userName, String password, Connection
defaultConnection) {
+ executeNonQueries(env, Collections.singletonList(sql), userName, password,
defaultConnection);
+ }
+
+ public static void executeNonQueries(
+ BaseEnv env, List<String> sqlList, Connection defaultConnection) {
+ executeNonQueries(
+ env,
+ sqlList,
+ SessionConfig.DEFAULT_USER,
+ SessionConfig.DEFAULT_PASSWORD,
+ defaultConnection);
+ }
+
+ public static void executeNonQueries(
+ BaseEnv env,
+ List<String> sqlList,
+ String userName,
+ String password,
+ Connection defaultConnection) {
+ int lastIndex = 0;
+ Connection localConnection = null;
+ Connection connectionToUse = defaultConnection;
+ Statement statement;
+ try {
+ // create a new connection if default is not provided or the previous is
broken
+ if (connectionToUse == null) {
+ localConnection = env.getConnection(userName, password);
+ connectionToUse = localConnection;
+ }
+ statement = connectionToUse.createStatement();
+ for (int i = lastIndex; i < sqlList.size(); ++i) {
Review Comment:
The `lastIndex` variable is initialized to 0 on line 520 but never updated.
This variable appears to serve no purpose in the current implementation and
should be removed. The loop on line 531 starts from `lastIndex` which is always
0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
