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]

Reply via email to