SEZ9 commented on issue #12874: URL: https://github.com/apache/seatunnel/issues/12874#issuecomment-6281821069
@fatmanverse thanks for actually trying the move and writing up the findings — that answers the open question clearly. To answer you directly: yes, please open it as a separate PR rather than folding it into #12875. Reasoning: - #12875 is the right thing to unblock CI now. A test-only correction (named exclusions plus the sink-rejection regression) is the smallest change that fixes `testSinkRuleDeclaresAllSinkOptions` on `dev`, and I agree with @hutiefang76 that we should not widen that PR into a source change. - The move itself looks like the better long-term home for the option. Your points are the ones that matter to me: the key, type, default and description stay the same; `NUMERIC_SPECIAL_VALUE_AS_NULL` is only referenced inside `connector-jdbc` (`JdbcSourceFactory#optionRule` and the three `JdbcConnectionConfig` spots); the CDC `*IncrementalSourceOptions` classes extend a different `JdbcSourceOptions`, so they are unaffected; and since #12792 is not in a release yet, there is no user-visible change. That is a small, safe refactor, not an API break, so it does not need to wait on a bigger discussion. Concrete asks for the follow-up PR: 1. Base it on `dev` after #12875 lands, and drop the `NOT_SINK_OPTIONS`-style exclusion for `numeric_special_value_as_null` in `JdbcOptionRuleCompletenessTest` since the move makes it unnecessary — the test should pass as-is, as you observed. 2. Keep the `JdbcSinkFactoryTest` guard that the sink still rejects `numeric_special_value_as_null`. 3. In the PR description, state the verification you listed here (JDK 17 against `dev` e2957d2, full `connector-jdbc` unit tests green, `connector-cdc-postgres` / `connector-cdc-mysql` still build) and link back to this issue. @hutiefang76 — nothing further needed on #12875 from this thread; the test-only fix stays as the CI unblocker, and the relocation will be reviewed on its own. <!-- streview-comment:1883 --> -- 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]
