siwen-yu commented on PR #10626: URL: https://github.com/apache/seatunnel/pull/10626#issuecomment-4095686408
@DanielCarter-stack Thanks for the detailed and thorough review — I’ve gone through all the points carefully. ### Changes to be addressed in this PR **Issue 1** Good catch — this is an oversight on my side. **Issue 2** This appears to be a legacy method and was not introduced in this PR. I plan to remove it, but I’ll double-check all call sites before doing so. **Issue 4** I’ll improve the fallback handling for null or unknown `sourceType`. **Issue 5** I’ll add JavaDoc/comments to clarify **Issue 6** I’ll extend unit test coverage to include: - `sourceType == null` - non-BYTES null handling - unknown `sourceType` --- ### Follow-ups **Issue 3** I agree that similar null-binding issues may exist in other JDBC dialects (Oracle, DB2, MySQL, etc.). To keep this PR focused on the reported SQLServer IMAGE issue, I’ll address that in a separate follow-up issue/PR. **Issue 7** I agree that E2E coverage would be valuable. I’ll add E2E tests in a follow-up PR to keep the current changeset focused and easier to review. --- Thanks again for the valuable feedback — I’ll update the PR soon. -- 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]
