Copilot commented on code in PR #10629:
URL: https://github.com/apache/seatunnel/pull/10629#discussion_r2963777284
##########
seatunnel-connectors-v2/connector-starrocks/src/main/java/org/apache/seatunnel/connectors/seatunnel/starrocks/catalog/StarRocksCatalog.java:
##########
@@ -242,7 +245,9 @@ public void truncateTable(TablePath tablePath, boolean
ignoreIfNotExists)
public void executeSql(TablePath tablePath, String sql) {
try {
- conn.createStatement().execute(sql);
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute(sql);
+ }
} catch (Exception e) {
throw new CatalogException(String.format("Failed EXECUTE SQL in
catalog %s", sql), e);
Review Comment:
executeSql() formats the error as "Failed EXECUTE SQL in catalog %s" but
passes the SQL string into the placeholder. Consider using catalogName (and
optionally tablePath) in the message, and include the SQL separately so logs
clearly indicate both catalog and statement.
```suggestion
String message =
String.format(
"Failed EXECUTE SQL in catalog %s for table %s.
SQL: %s",
catalogName, tablePath.getFullName(), sql);
throw new CatalogException(message, e);
```
##########
seatunnel-connectors-v2/connector-starrocks/src/main/java/org/apache/seatunnel/connectors/seatunnel/starrocks/catalog/StarRocksCatalog.java:
##########
@@ -449,9 +458,9 @@ public String name() {
protected Optional<PrimaryKey> getPrimaryKey(String schema, String table)
throws SQLException {
List<String> pkFields = new ArrayList<>();
- try (ResultSet rs =
- conn.createStatement()
- .executeQuery(
+ try (Statement stmt = conn.createStatement();
+ ResultSet rs =
+ stmt.executeQuery(
String.format(
"SELECT COLUMN_NAME FROM
information_schema.columns where TABLE_SCHEMA = '%s' AND TABLE_NAME = '%s' AND
COLUMN_KEY = 'PRI' ORDER BY ORDINAL_POSITION",
schema, table))) {
Review Comment:
getPrimaryKey() builds SQL via String.format with schema/table embedded into
the query string. This is inconsistent with listTables()/tableExists() (which
use parameterized PreparedStatement) and can break on names containing
quotes/special chars (and can allow SQL injection if inputs are ever
user-controlled). Prefer a PreparedStatement with '?' placeholders and
setString for schema and table.
--
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]
