Copilot commented on code in PR #9380:
URL: https://github.com/apache/seatunnel/pull/9380#discussion_r2148916546
##########
seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/utils/JdbcCatalogUtils.java:
##########
@@ -409,4 +428,102 @@ private static ReadonlyConfig
extractCatalogConfig(JdbcConnectionConfig config)
catalogConfig.put(JdbcOptions.HANDLE_BLOB_AS_STRING.key(),
config.isHandleBlobAsString());
return ReadonlyConfig.fromMap(catalogConfig);
}
+
+ /** Process table path with regex pattern and add matched tables to the
result. */
+ private static void processRegexTablePath(
+ AbstractJdbcCatalog jdbcCatalog,
+ JdbcDialect jdbcDialect,
+ JdbcSourceTableConfig tableConfig,
+ Map<TablePath, JdbcSourceTable> result)
+ throws SQLException {
+
+ String tablePath = tableConfig.getTablePath();
+ log.info("Processing table path with regex: {}", tablePath);
+
+ // Parse table path to extract database, schema and table patterns
+ String databasePattern = ".*";
+ String tableNamePattern = ".*"; // This is just the table name part
+
+ // Convert the table path to a format suitable for regex matching
+
+ // Step 1: Replace escaped dots (\.) with a placeholder
+ String processedTablePath = tablePath.replace("\\.", DOT_PLACEHOLDER);
+ log.info("After replacing escaped dots with placeholder: {}",
processedTablePath);
+
+ // Step 2: Split by unescaped dots
+ String[] parts = processedTablePath.split("\\.");
+
+ String fullTablePattern;
+
+ if (parts.length == 1) {
+ // Only table pattern
+ tableNamePattern = parts[0];
+ fullTablePattern = String.format("%s", tableNamePattern);
+ } else if (parts.length == 2) {
+ // database.table or schema.table format
+ databasePattern = parts[0];
+ tableNamePattern = parts[1];
Review Comment:
When splitting on unescaped dots, placeholders may remain in databasePattern
or tableNamePattern. You should replace DOT_PLACEHOLDER back to literal dots
when building fullTablePattern here (e.g., `.replace(DOT_PLACEHOLDER, ".")`).
```suggestion
databasePattern = parts[0].replace(DOT_PLACEHOLDER, ".");
tableNamePattern = parts[1].replace(DOT_PLACEHOLDER, ".");
```
##########
seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/OracleDialect.java:
##########
@@ -211,6 +211,12 @@ public Long approximateRowCntStatement(Connection
connection, JdbcSourceTable ta
String query = table.getQuery();
+ // Add null value judgment
+ if (table.getTablePath() == null) {
Review Comment:
Checking only for a null tablePath may skip the intended logic when a custom
query is provided but tablePath is non-null. Consider guarding based on `query`
presence (e.g., `if (query != null && !query.isEmpty())`) instead to preserve
correct behavior.
```suggestion
if (query != null && !query.isEmpty()) {
```
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/Catalog.java:
##########
@@ -164,15 +164,42 @@ default List<CatalogTable> getTables(ReadonlyConfig
config) throws CatalogExcept
Pattern databasePattern =
Pattern.compile(config.get(ConnectorCommonOptions.DATABASE_PATTERN));
Pattern tablePattern =
Pattern.compile(config.get(ConnectorCommonOptions.TABLE_PATTERN));
+
List<String> allDatabase = this.listDatabases();
allDatabase.removeIf(s -> !databasePattern.matcher(s).matches());
List<TablePath> tablePaths = new ArrayList<>();
+
for (String databaseName : allDatabase) {
tableNames = this.listTables(databaseName);
tableNames.forEach(
tableName -> {
- if (tablePattern.matcher(databaseName + "." +
tableName).matches()) {
- tablePaths.add(TablePath.of(databaseName,
tableName));
+ // Parse table name to get schema (if any)
+ String schemaName = null;
+ String actualTableName = tableName;
+
+ if (tableName.contains(".")) {
+ String[] parts = tableName.split("\\.");
+ if (parts.length == 2) {
+ schemaName = parts[0];
+ actualTableName = parts[1];
+ }
Review Comment:
[nitpick] Splitting on every dot can mis-handle table names or schemas that
themselves include dots. Consider using `lastIndexOf('.')` with substring, or
using `split("\\.", 2)` to limit splits to known components.
```suggestion
int lastDotIndex = tableName.lastIndexOf('.');
if (lastDotIndex != -1) {
schemaName = tableName.substring(0,
lastDotIndex);
actualTableName =
tableName.substring(lastDotIndex + 1);
```
##########
seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/config/JdbcSourceTableConfig.java:
##########
@@ -84,6 +91,7 @@ public static List<JdbcSourceTableConfig> of(ReadonlyConfig
connectorConfig) {
.partitionNumber(connectorConfig.get(JdbcOptions.PARTITION_NUM))
.partitionStart(connectorConfig.get(JdbcOptions.PARTITION_LOWER_BOUND))
.partitionEnd(connectorConfig.get(JdbcOptions.PARTITION_UPPER_BOUND))
+
.useRegex(connectorConfig.get(JdbcSourceOptions.USE_REGEX))
Review Comment:
Directly getting USE_REGEX from config may yield null if not set. Use the
`getUseRegex()` helper (which defaults to false) when building the
`JdbcSourceTableConfig` to avoid potential NPE or unexpected null Boolean
values.
--
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]
