Copilot commented on code in PR #54483:
URL: https://github.com/apache/doris/pull/54483#discussion_r2261941073
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java:
##########
@@ -416,17 +416,30 @@ protected String[] getTableTypes() {
protected void processTable(String remoteDbName, String remoteTableName,
String[] tableTypes,
Consumer<ResultSet> resultSetConsumer) {
Connection conn = null;
- ResultSet rs = null;
+ ResultSet standardRs = null;
+ Statement stmt = null;
+ ResultSet customRs = null;
+
try {
conn = getConnection();
DatabaseMetaData databaseMetaData = conn.getMetaData();
String catalogName = getCatalogName(conn);
- rs = databaseMetaData.getTables(catalogName, remoteDbName,
remoteTableName, tableTypes);
- resultSetConsumer.accept(rs);
+
+ // 1. Process standard tables from getTables() method
+ standardRs = databaseMetaData.getTables(catalogName, remoteDbName,
remoteTableName, tableTypes);
+ resultSetConsumer.accept(standardRs);
+
+ // 2. Process additional tables from custom SQL query (if any)
+ String additionalQuery = getAdditionalTablesQuery(remoteDbName,
remoteTableName, tableTypes);
+ if (additionalQuery != null && !additionalQuery.trim().isEmpty()) {
+ stmt = conn.createStatement();
+ customRs = stmt.executeQuery(additionalQuery);
+ resultSetConsumer.accept(customRs);
+ }
} catch (SQLException e) {
throw new JdbcClientException("Failed to process table", e);
} finally {
- close(rs, conn);
+ close(customRs, stmt, standardRs, conn);
Review Comment:
Similar to the Oracle client, this close method call uses 4 parameters but
the original close method likely only accepts 2 parameters. This will cause a
compilation error unless a new overloaded close method has been added.
```suggestion
// Close resources in reverse order of creation
if (customRs != null) {
try {
customRs.close();
} catch (SQLException e) {
// log and ignore
}
}
if (standardRs != null) {
try {
standardRs.close();
} catch (SQLException e) {
// log and ignore
}
}
if (stmt != null) {
try {
stmt.close();
} catch (SQLException e) {
// log and ignore
}
}
if (conn != null) {
try {
conn.close();
} catch (SQLException e) {
// log and ignore
}
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java:
##########
@@ -76,11 +81,28 @@ public List<JdbcFieldSchema> getJdbcColumnsInfo(String
remoteDbName, String remo
}
tableSchema.add(new JdbcFieldSchema(rs));
}
+ if (tableSchema.isEmpty()) {
+ // maybe the table is a synonym
+ stmt = conn.createStatement();
+ isSynonymRs = stmt.executeQuery(
+ "SELECT OBJECT_TYPE FROM ALL_OBJECTS WHERE OBJECT_NAME
= '" + remoteTableName
+ + "' AND OWNER = '" + remoteDbName + "'");
+ if (isSynonymRs.next() &&
"SYNONYM".equalsIgnoreCase(isSynonymRs.getString("OBJECT_TYPE"))) {
+ // if it is a synonym, get the actual table name and
owner(database)
+ String additionalTablesQuery =
getAdditionalTablesQuery(remoteDbName, remoteTableName, null);
+ synonymInfoRs = stmt.executeQuery(additionalTablesQuery);
+ while (synonymInfoRs.next()) {
+ String baseTableName =
synonymInfoRs.getString("BASE_TABLE_NAME");
+ String baseTableOwner =
synonymInfoRs.getString("BASE_TABLE_OWNER");
+ return getJdbcColumnsInfo(baseTableOwner,
baseTableName);
+ }
+ }
+ }
} catch (SQLException e) {
throw new JdbcClientException("failed to get table name list from
jdbc for table %s:%s", e, remoteTableName,
Util.getRootCauseMessage(e));
} finally {
- close(rs, conn);
+ close(rs, conn, stmt, isSynonymRs, synonymInfoRs);
Review Comment:
The close method is being called with 5 parameters but the original close
method likely only accepts 2 parameters (rs, conn). This will cause a
compilation error unless a new overloaded close method has been added.
```suggestion
close(rs, conn);
if (stmt != null) {
try {
stmt.close();
} catch (SQLException e) {
// ignore
}
}
if (isSynonymRs != null) {
try {
isSynonymRs.close();
} catch (SQLException e) {
// ignore
}
}
if (synonymInfoRs != null) {
try {
synonymInfoRs.close();
} catch (SQLException e) {
// ignore
}
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java:
##########
@@ -109,6 +131,24 @@ protected Set<String> getFilterInternalDatabases() {
.build();
}
+ @Override
+ protected String getAdditionalTablesQuery(String remoteDbName, String
remoteTableName, String[] tableTypes) {
+ StringBuilder sb = new StringBuilder(
+ "SELECT SYNONYM_NAME as TABLE_NAME, TABLE_OWNER as
BASE_TABLE_OWNER, TABLE_NAME as BASE_TABLE_NAME "
+ + "FROM ALL_SYNONYMS");
+ List<String> conditions = Lists.newArrayList();
+ if (!Strings.isNullOrEmpty(remoteDbName)) {
+ conditions.add("OWNER = '" + remoteDbName + "'");
+ }
+ if (!Strings.isNullOrEmpty(remoteTableName)) {
+ conditions.add("SYNONYM_NAME = '" + remoteTableName + "'");
Review Comment:
SQL injection vulnerability: user input (remoteDbName) is directly
concatenated into the SQL query without proper escaping or parameterization.
```suggestion
conditions.add("OWNER = '" + escapeSql(remoteDbName) + "'");
}
if (!Strings.isNullOrEmpty(remoteTableName)) {
conditions.add("SYNONYM_NAME = '" + escapeSql(remoteTableName) +
"'");
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java:
##########
@@ -109,6 +131,24 @@ protected Set<String> getFilterInternalDatabases() {
.build();
}
+ @Override
+ protected String getAdditionalTablesQuery(String remoteDbName, String
remoteTableName, String[] tableTypes) {
+ StringBuilder sb = new StringBuilder(
+ "SELECT SYNONYM_NAME as TABLE_NAME, TABLE_OWNER as
BASE_TABLE_OWNER, TABLE_NAME as BASE_TABLE_NAME "
+ + "FROM ALL_SYNONYMS");
+ List<String> conditions = Lists.newArrayList();
+ if (!Strings.isNullOrEmpty(remoteDbName)) {
+ conditions.add("OWNER = '" + remoteDbName + "'");
+ }
+ if (!Strings.isNullOrEmpty(remoteTableName)) {
Review Comment:
SQL injection vulnerability: user input (remoteTableName) is directly
concatenated into the SQL query without proper escaping or parameterization.
```suggestion
if (!Strings.isNullOrEmpty(remoteDbName)) {
validateIdentifier(remoteDbName);
conditions.add("OWNER = '" + remoteDbName + "'");
}
if (!Strings.isNullOrEmpty(remoteTableName)) {
validateIdentifier(remoteTableName);
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java:
##########
@@ -76,11 +81,28 @@ public List<JdbcFieldSchema> getJdbcColumnsInfo(String
remoteDbName, String remo
}
tableSchema.add(new JdbcFieldSchema(rs));
}
+ if (tableSchema.isEmpty()) {
+ // maybe the table is a synonym
+ stmt = conn.createStatement();
+ isSynonymRs = stmt.executeQuery(
+ "SELECT OBJECT_TYPE FROM ALL_OBJECTS WHERE OBJECT_NAME
= '" + remoteTableName
+ + "' AND OWNER = '" + remoteDbName + "'");
Review Comment:
The SQL query is constructed using string concatenation with user input
(remoteTableName and remoteDbName) which could lead to SQL injection
vulnerabilities. Consider using prepared statements or proper SQL escaping.
```suggestion
"SELECT OBJECT_TYPE FROM ALL_OBJECTS WHERE
OBJECT_NAME = " + quoteOracleIdentifier(remoteTableName)
+ " AND OWNER = " +
quoteOracleIdentifier(remoteDbName));
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
