DanielLeens commented on PR #12875: URL: https://github.com/apache/seatunnel/pull/12875#issuecomment-6341348148
Thanks for the careful follow-up, @hutiefang76. I re-checked your conclusion against the code and I agree with it. Evidence on head `e0785877946839775177cbb234b364b0ebd534f1` versus current `dev` (`4dcfa28ccc8`): - `JdbcOptionRuleCompletenessTest` on `dev` already defines `NOT_SINK_OPTIONS` (line 59) with `JdbcCommonOptions.NUMERIC_SPECIAL_VALUE_AS_NULL`, and already passes it to `collectOptionKeys(JdbcSinkOptions.class, NOT_SINK_OPTIONS)` (line 73). It also already has the source-accepts / sink-rejects guard test (`testNumericSpecialValueAsNullIsSourceOnly`, line 77) that uses the real `JdbcSourceFactory` and `JdbcSinkFactory`. - The file's last change on `dev` is cc54bb433 (#12938), so the sink-completeness failure from #12874 no longer reproduces on `dev`. - The diff between this head and `dev` for that single file is only a Javadoc paragraph, `Collections.singleton` replaced by an equivalent unmodifiable `HashSet`, a renamed test, and a more descriptive assertion message. There is no behavior or coverage change, so nothing in this PR is left to land. Since the original goal is already satisfied on `dev`, I would suggest not spending more effort on this branch: no sync or rerun is needed to make it mergeable, because the intended fix is already merged. The branch is also currently conflicting with `dev` (ahead 3, behind 11), so keeping it open would only mean resolving conflicts for cosmetic changes. If you still want the Javadoc and naming polish, it would be better as a fresh small PR rebased on current `dev`; otherwise it is reasonable for you to close this one, referencing #12874 and #12938. I will leave the close decision to you or a committer. Thanks again for the quick root-cause on #12874 and for being upfront that the fix was covered. -- 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]
