Copilot commented on code in PR #11840:
URL: https://github.com/apache/gravitino/pull/11840#discussion_r3510140469
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java:
##########
@@ -66,6 +66,14 @@ protected Set<String> createSysDatabaseNameSet() {
return CLICK_HOUSE_SYSTEM_DATABASES;
}
+ @Override
+ protected String generateDatabaseExistSql(String databaseName) {
+ // Escape single quotes to prevent SQL injection. ClickHouse does not
support PreparedStatement
+ // for DDL-level system queries; single-quote doubling is the standard SQL
escape.
Review Comment:
The comment here attributes escaping to ClickHouse not supporting
PreparedStatement, but this SQL is executed via Statement in
JdbcDatabaseOperations#exist(). Rewording it to reference the actual constraint
will prevent future confusion about driver capabilities vs the inherited
execution path.
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -111,7 +111,9 @@ ARRAY JOIN arrayZip(splitByChar(',', primary_key),
arrayEnumerate(splitByChar(',
protected List<Index> getIndexes(Connection connection, String databaseName,
String tableName) {
// cause clickhouse not impl getPrimaryKeys yet, ref:
// https://github.com/ClickHouse/clickhouse-java/issues/1625
- String sql = QUERY_INDEXES_SQL.formatted(databaseName, tableName);
+ String sql =
+ QUERY_INDEXES_SQL.formatted(
+ escapeSingleQuotes(databaseName), escapeSingleQuotes(tableName));
Review Comment:
This change hardens getIndexes() against single-quote injection, but there’s
no test covering the escaping behavior. The module already has unit tests using
Mockito (e.g., TestClickHouseDatabaseOperations), so please add a focused unit
test that passes names like "db'1"/"t'1" and asserts the SQL passed to
Connection.prepareStatement() contains doubled quotes (db''1, t''1).
--
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]
