DanielLeens commented on code in PR #11023:
URL: https://github.com/apache/seatunnel/pull/11023#discussion_r3412831577
##########
seatunnel-connectors-v2/connector-cdc/connector-cdc-mongodb/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/mongodb/MongodbIncrementalSourceFactory.java:
##########
@@ -114,42 +164,21 @@ private List<CatalogTable>
updateAndValidateCatalogTableId(
catalogTable.getCatalogName(),
TablePath.of(collectionName));
return Collections.singletonList(
CatalogTable.of(updatedIdentifier, catalogTable));
- } else if (!fullName.equals(collectionName)) {
Review Comment:
Thanks for pointing this out. I agree this is a real gap on the current
head, not just a style preference.
The old path rejected two cases that the new validation no longer covers:
1. a multi-table path where `catalogTables.size() == collections.size()` but
the schema/table full name still does not match the corresponding collection
name, and
2. the explicit mismatch failure on the non-single-table path.
After this change, `updateAndValidateCatalogTableId(...)` only rewrites the
one-table case and otherwise falls through, so a mismatched schema/collection
mapping can now slip past submit-time validation. Since there is still no new
commit after Daniel's last full review, I'm keeping this as a thread-level
follow-up and would want this restored or otherwise revalidated in the next
code update.
##########
seatunnel-connectors-v2/connector-cdc/connector-cdc-oracle/src/main/java/org/apache/seatunnel/connectors/seatunnel/cdc/oracle/config/OracleSourceConfigFactory.java:
##########
@@ -129,29 +124,16 @@ public OracleSourceConfig create(int subtask) {
.map(
tableStr -> {
String[] splits =
tableStr.split("\\.");
- if (splits.length == 2) {
- return tableStr;
- }
if (splits.length == 3) {
return String.join(".", splits[1],
splits[2]);
}
- throw new IllegalArgumentException(
- "Invalid table name: " +
tableStr);
+ return tableStr;
})
.collect(Collectors.joining(",")));
}
// override the user-defined debezium properties
if (dbzProperties != null) {
- String debeziumSchemaChanges =
Review Comment:
Agreed. The current validator only runs when SeaTunnel's
`SCHEMA_CHANGES_ENABLED` option is `true`, but
`OracleSourceConfigFactory.create()` later does `props.putAll(dbzProperties)`,
so a user can still force Debezium `include.schema.changes=true` through
`debezium_properties` while bypassing the submit-time check here.
That means the runtime contract is still wider than the validation contract
on the current head. Since there is still no new commit after Daniel's last
full review, I'm treating this as another thread-level blocker to address in
the next code update rather than reopening a fresh full review record on the
same revision.
--
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]
