DanielCarter-stack commented on PR #10368: URL: https://github.com/apache/seatunnel/pull/10368#issuecomment-3770837520
<!-- code-pr-reviewer --> ## Testing & Documentation - **DataValidatorTransformTest.java:52-96** - Tests only cover non-null database/schema scenarios. When using sources without db/schema prefixes (e.g., FakeSource where `getDatabaseName()`/`getSchemaName()` return null), the error table's tableId may lack prefixes, causing downstream connectors like JDBC to fail writing. - **incompatible-changes.md** - Missing documentation for user-visible behavior change. Error row `table_id` now includes database/schema prefix (e.g., "db1.ffp" instead of "ffp"). Downstream applications parsing tableId strings may break after upgrade. ## Code Quality & Robustness - **DataValidatorTransform.java:72-73** - Fixed redundant nested `getOptional()` calls for `ERROR_TABLE_OPTION`, but this cleanup wasn't mentioned in PR description or commit message, reducing transparency of changes. - **DataValidatorTransform.java:214-237** - `generateErrorRow` throws RuntimeException on JSON serialization failure, stopping the entire job instead of routing to error table. Consider adding fallback handling in a follow-up PR to improve pipeline stability. -- 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]
