Copilot commented on code in PR #61317:
URL: https://github.com/apache/doris/pull/61317#discussion_r2930119045
##########
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)) {
Review Comment:
ConfigUtil.parseAllTargetTableMappings() uses DataSourceConfigKeys and
HashMap, but this file currently has no imports for
org.apache.doris.job.cdc.DataSourceConfigKeys or java.util.HashMap, which will
cause a compilation failure. Add the missing imports (or fully qualify / adjust
existing imports).
##########
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());
+ String dstTable = entry.getValue().trim();
Review Comment:
parseAllTargetTableMappings() calls entry.getValue().trim() without
null-checking. Source config values for per-table keys are not validated
elsewhere, so a null value here will NPE and fail the pipeline. Consider
null-guarding (treat null as empty) and/or validating per-table config values
as non-empty during job creation.
##########
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];
+ if (!ALLOW_TABLE_LEVEL_SUFFIXES.contains(suffix)) {
+ throw new IllegalArgumentException("Unknown per-table
config key: '" + key + "'");
+ }
Review Comment:
validateSource() skips value validation for keys under the
"table.<tableName>.<suffix>" namespace. That allows configs like
table.foo.target_table='' or null to pass validation and later fail when
generating DDL / parsing mappings. Please enforce non-empty values for
per-table keys (or at least for target_table) in this validator.
##########
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
Review Comment:
Per-table config validation currently only allows suffix 'target_table'.
However DataSourceConfigKeys also defines TABLE_EXCLUDE_COLUMNS_SUFFIX
('exclude_columns'), and the validator comment mentions
'table.exclude_columns'. If exclude_columns is intended to remain supported,
add it to ALLOW_TABLE_LEVEL_SUFFIXES; otherwise remove/rename the constant and
update the comment to avoid an inconsistent API surface.
--
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]
