github-actions[bot] commented on code in PR #61317:
URL: https://github.com/apache/doris/pull/61317#discussion_r2930161672


##########
fe/fe-common/src/main/java/org/apache/doris/job/cdc/DataSourceConfigKeys.java:
##########
@@ -38,6 +38,11 @@ public class DataSourceConfigKeys {
     public static final String SSL_MODE = "ssl_mode";
     public static final String SSL_ROOTCERT = "ssl_rootcert";
 
+    // per-table config: key format is "table.<tableName>.<suffix>"
+    public static final String TABLE = "table";
+    public static final String TABLE_EXCLUDE_COLUMNS_SUFFIX = 
"exclude_columns";
+    public static final String TABLE_TARGET_TABLE_SUFFIX = "target_table";
+
     // target properties
     public static final String TABLE_PROPS_PREFIX = "table.create.properties.";

Review Comment:
   **[Minor] Dead code:** `TABLE_EXCLUDE_COLUMNS_SUFFIX` is declared here but 
is never used anywhere in the codebase — not in 
`DataSourceConfigValidator.ALLOW_TABLE_LEVEL_SUFFIXES`, not in any consumer 
code. If this is a placeholder for a future feature, consider removing it from 
this PR to avoid confusion and adding it when the feature is actually 
implemented.



##########
fs_brokers/cdc_client/src/main/java/org/apache/doris/cdcclient/utils/ConfigUtil.java:
##########
@@ -116,6 +116,30 @@ public static boolean isJson(String str) {
         }
     }
 
+    /**
+     * Parse all target-table name mappings from config.
+     *
+     * <p>Scans all keys matching {@code "table.<srcTableName>.target_table"} 
and returns a map from
+     * source table name to target (Doris) table name. Tables without a 
mapping are NOT included;
+     * callers should use {@code getOrDefault(srcTable, srcTable)}.
+     */
+    public static Map<String, String> parseAllTargetTableMappings(Map<String, 
String> config) {
+        String prefix = DataSourceConfigKeys.TABLE + ".";
+        String suffix = "." + DataSourceConfigKeys.TABLE_TARGET_TABLE_SUFFIX;
+        Map<String, String> result = new HashMap<>();
+        for (Map.Entry<String, String> entry : config.entrySet()) {
+            String key = entry.getKey();
+            if (key.startsWith(prefix) && key.endsWith(suffix)) {
+                String srcTable = key.substring(prefix.length(), key.length() 
- suffix.length());

Review Comment:
   **[CRITICAL] Compilation error:** `HashMap` is used here but 
`java.util.HashMap` is not imported. Similarly, `DataSourceConfigKeys` (used on 
the two lines above) is not imported either. This file will fail to compile.
   
   Add the following imports:
   ```java
   import org.apache.doris.job.cdc.DataSourceConfigKeys;
   import java.util.HashMap;
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/DataSourceConfigValidator.java:
##########
@@ -43,11 +43,33 @@ public class DataSourceConfigValidator {
             DataSourceConfigKeys.SSL_ROOTCERT
     );
 
+    // Known suffixes for per-table config keys (format: 
"table.<tableName>.<suffix>")
+    private static final Set<String> ALLOW_TABLE_LEVEL_SUFFIXES = 
Sets.newHashSet(
+            DataSourceConfigKeys.TABLE_TARGET_TABLE_SUFFIX
+    );
+
+    private static final String TABLE_LEVEL_PREFIX = 
DataSourceConfigKeys.TABLE + ".";
+
     public static void validateSource(Map<String, String> input) throws 
IllegalArgumentException {
         for (Map.Entry<String, String> entry : input.entrySet()) {
             String key = entry.getKey();
             String value = entry.getValue();
 
+            if (key.startsWith(TABLE_LEVEL_PREFIX)) {
+                // per-table config key must be exactly: 
table.<tableName>.<suffix>
+                // reject malformed keys like "table.exclude_columns" (missing 
tableName)
+                String[] parts = key.split("\\.", -1);
+                if (parts.length != 3 || parts[1].isEmpty()) {
+                    throw new IllegalArgumentException("Malformed per-table 
config key: '" + key
+                            + "'. Expected format: 
table.<tableName>.<suffix>");
+                }
+                String suffix = parts[parts.length - 1];

Review Comment:
   **[Minor] Table names with dots will be rejected:** `split("\\.", -1)` with 
`parts.length != 3` means that a table name containing a dot (e.g., 
`table.my.dotted.table.target_table`) will produce more than 3 parts and fail 
validation. PostgreSQL allows dots in quoted identifiers.
   
   Consider using `indexOf`/`lastIndexOf` instead:
   ```java
   int firstDot = key.indexOf('.', TABLE_LEVEL_PREFIX.length());
   int lastDot = key.lastIndexOf('.');
   if (firstDot == -1 || firstDot != lastDot - ???) { ... }
   ```
   Or at minimum, document this limitation (no dots in source table names).



-- 
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]

Reply via email to