[Feature][Connector-V2] PR1: Pass sink table-options into auto-created MySQL target tables

[det101]

Purpose of this pull request

Add table-options for JDBC sink auto-create flow (MySQL first delivery for #11047).

• Introduce shared table-options config in SinkConnectorCommonOptions
• Propagate options into CatalogTable in JdbcSinkFactory
• Validate per dialect; MySQL/TiDB allow engine, charset, collate
• Fail fast for unsupported JDBC dialects or unknown keys

Status — MySQL JDBC only; StarRocks/Paimon out of scope for this draft.

Does this PR introduce any user-facing change?

Yes. New optional sink config table-options (map). Applied only when schema_save_mode triggers DDL (e.g. CREATE_SCHEMA_WHEN_NOT_EXIST). Does not alter existing tables or runtime INSERT/UPSERT.

How was this patch tested?

• Unit: MysqlDialectTest, MysqlCreateTableSqlBuilderTest
• E2E: JdbcMysqlTableOptionsIT

Check list

• If any new Jar binary package adding in your PR, please add License Notice according
  New License Guide
• If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
• If necessary, please update incompatible-changes.md to describe the incompatibility caused by this PR.
• If you are contributing the connector code, please check that the following files are updated:
  1. Update plugin-mapping.properties and add new connector information in it
  2. Update the pom file of seatunnel-dist
  3. Add ci label in label-scope-conf
  4. Add e2e testcase in seatunnel-e2e
  5. Update connector plugin_config

[DanielLeens]

Thanks for working on this. I reviewed the full diff from JdbcSinkFactory down into the MySQL create-table builder and the new E2E instead of treating this as a docs-only option change.

What this PR fixes

• User pain: when JDBC sink auto-creates a MySQL target table, users currently cannot pass table-level properties such as engine, charset, or collate through the normal sink configuration path.
• Fix approach: add table_options, validate it at the dialect layer, propagate it into CatalogTable.options, and let the MySQL create-table builder append those clauses to the generated DDL.
• One-line summary: this makes MySQL table-level auto-create options real on the actual DDL path instead of leaving them as unsupported config intent.

Runtime chain I checked

JDBC sink initialization
  -> JdbcSinkFactory.createSink(...) [JdbcSinkFactory.java:71-209]
      -> read table_options [73]
      -> load JDBC dialect [187-192]
      -> validateTableOptions(...) [193]
      -> put table_options into CatalogTable.options [183]

Auto-create path
  -> save-mode / catalog createTable(...)
      -> MysqlCreateTableSqlBuilder.build(...) [MysqlCreateTableSqlBuilder.java:138-157]
          -> ENGINE = ... [145-147]
          -> DEFAULT CHARSET = ... [148-150]
          -> COLLATE = ... [151-153]

Unsupported dialect path
  -> JdbcDialect.validateTableOptions(...)
      -> non-empty table_options fail fast at job submission

Key findings

1. The normal MySQL auto-create path really does hit this change. The option is not just accepted syntactically; it reaches the final DDL builder.
2. The fail-fast behavior for non-MySQL dialects is the right safety boundary here. Unsupported dialects do not silently ignore user intent.
3. I did not find a blocking correctness issue in the current head.

Test / stability note

• The new MySQL dialect unit tests cover supported keys, unknown keys, and unsupported dialect rejection.
• The new E2E checks SHOW CREATE TABLE, which is the right signal for this feature and does not rely on weak timing assumptions.

Local verification note

• I checked the full diff, the MySQL dialect path, the E2E, the updated EN/ZH docs, and the TiDB source path reuse in code.
• I did not run local Maven in this batch; this is a source-level PR review only.
• At the time I queried GitHub metadata, statusCheckRollup was empty, so this conclusion is about source-level readiness rather than completed CI.

Conclusion: can merge

1. Blocking items

• None from my side.

2. Suggested follow-up

• A TiDB-specific coverage pass would still be useful later, since the docs mention MySQL / TiDB together and the current validation is source-level rather than TiDB-runtime coverage.

Overall, the implementation is clean and the scope is well controlled. The option, validation, DDL wiring, and docs are all aligned on the current head.

[davidzollo]

+1
LGTM

[dybyte]

Thanks for the update. It looks like the CI has not run yet. Could you please enable CI? https://github.com/det101/seatunnel/actions

[DanielLeens]

Thanks for the follow-up here. From my side there is no new code to re-review on this PR yet, so I am not adding another full review round at this point. I do agree with @dybyte that CI still needs to be enabled and completed before this can move forward, since the current discussion is now mostly about merge readiness rather than the code path I already reviewed.

[det101]

Thanks for the update. It looks like the CI has not run yet. Could you please enable CI? https://github.com/det101/seatunnel/actions

I am unable to enable Continuous Integration (CI) or run any workflows because the operation functionality is disabled at the account level (not just for this repository). I have submitted a support ticket to GitHub and am waiting for a response.

Could you please help close and reopen the PR to trigger the CI run? @dybyte

[DanielLeens]

Thanks for the update. From my side there is still no new code to re-review on this PR, and I agree the current blocker is now CI / maintainer workflow handling rather than the MySQL table-options code path itself.

If the PR gets retriggered or a new commit lands, I can re-check the latest head. Until then I am not starting another full review round on the unchanged code.

[dybyte]

Thanks for the update. It looks like the CI has not run yet. Could you please enable CI? https://github.com/det101/seatunnel/actions

I am unable to enable Continuous Integration (CI) or run any workflows because the operation functionality is disabled at the account level (not just for this repository). I have submitted a support ticket to GitHub and am waiting for a response.

Could you please help close and reopen the PR to trigger the CI run? @dybyte

Thanks for the clarification. I think we should wait for GitHub Support to resolve the Actions restriction on your side.

Once Actions can be enabled again, could you please push an empty commit to the PR branch to trigger CI?

Closing and reopening the PR would not enable Actions on your fork/account, so I think pushing a new commit after the restriction is resolved would be the safer path.

[DanielLeens]

Thanks for the clarification. From Daniel's side I still do not have a new source-level review item on the unchanged head. The remaining blocker is maintainer-side workflow help: either re-enable CI for this branch or mirror the same head under a workflow-enabled branch and rerun the normal Build check there. Once CI can actually run, I can re-check the result if needed, but I am not starting another full review round without a new commit.

[luxiaolong-ct]

Closing this PR in favor of #11120. The implementation scope is unchanged (JDBC MySQL table_options for SaveMode auto-create, fix #11047), but this PR was opened from the det101 fork where GitHub Actions could not run, which blocked CI and merge readiness despite prior review feedback. The same changes have been rebased onto feature-11047-pr1 under the luxiaolong-ct fork with aligned commit authorship, so CI can run normally. Please continue review and merge discussion on #11120; any comments from this PR still apply to the same code path there.

@davidzollo @dybyte @DanielLeens

[DanielLeens]

Thanks for the heads-up, and the handoff to #11120 makes sense.

From Daniel's side I will treat the previous source-level findings on this branch as carried over to the replacement PR, rather than reopening another full review round on this now-superseded branch. So I am keeping #11101 in reply-only state and will continue the actual code review on #11120 instead.

If anything in the implementation scope changed during the move, I will evaluate that on the new PR head directly.

[det101]

Closing this PR in favor of #11120. The implementation scope is unchanged (JDBC MySQL table_options for SaveMode auto-create, fix #11047), but this PR was opened from the det101 fork where GitHub Actions could not run, which blocked CI and merge readiness despite prior review feedback. The same changes have been rebased onto feature-11047-pr1 under the luxiaolong-ct fork with aligned commit authorship, so CI can run normally. Please continue review and merge discussion on #11120; any comments from this PR still apply to the same code path there.

@davidzollo @dybyte @DanielLeens

The account has been replied to, please continue reviewing. @dybyte

[DanielLeens]

Thanks for the follow-up.

I rechecked the current PR state before replying. From Daniel's side, I still want to keep the actual code-review thread centralized on #11120, because that PR was explicitly opened as the CI-enabled replacement for the same implementation scope.

So on #11101 I am still treating this as reply-only state rather than starting a second parallel code-review line on top of #11120.

If this branch diverges from #11120 or picks up unique code that is no longer covered there, please ping me again and I can revisit it directly. Otherwise I will continue the actual source review on the replacement PR.

[nzw921rx]

Thank you for your contribution. This is a very important feature supplement. I have a question about the configuration: currently, this feature only applies to JDBC, but the options are placed in SinkConnectorCommonOptions, which means it is a universal capability for all connectors. Can you explain why it is not only included in the JDBC sink configuration?

[det101]

Thank you for your contribution. This is a very important feature supplement. I have a question about the configuration: currently, this feature only applies to JDBC, but the options are placed in SinkConnectorCommonOptions, which means it is a universal capability for all connectors. Can you explain why it is not only included in the JDBC sink configuration?

You've hit on the key point: this issue adds table_options for all sinks. The JDBC implementation is the first PR, but non-JDBC sinks will also use this later, which is why the PR is named "PR1".

[nzw921rx]

When a dialect is specified, it should fail as early as possible, rather than waiting until creating Sink. I think a good implementation is to implement ConditioneExtension<Boolean> and validate the specified dialect, which can avoid some situations where the configuration fails at runtime and allow it to be recognized by API/CLI/AI generation checks

examples:

.optional(JdbcSinkOptions.DIALECT,Conditions.extension(JdbcSinkOptions.DIALECT,new ConditionExtension<String>(){

                    @Override
                    public String description() {
                        return null;
                    }

                    @Override
                    public boolean evaluate(ReadonlyConfig config, String value) throws OptionValidationException {
                        return false;
                    }
                }))

[DanielLeens]

Thanks for the clarification.

I rechecked the current PR state before replying. From Daniel's side there is still no new code on top of commit b1a9cdb6c63f602b62812d3c38fc506e6d07fc10, so I am keeping this in reply-only state rather than starting another full review round on the unchanged head.

Your explanation makes the intended scope clearer: the current JDBC work is the first implementation, while table_options is meant to become a shared sink contract later. Whether that broader common-option contract should stay in this PR or be narrowed or split is something I would still leave to the maintainer discussion already open on this PR.

So from my side there is no new source-level comment on the current head. If that design discussion leads to code changes, or if you push a new commit, please ping me again and I can re-review the latest revision right away.

[det101]

Thanks @dybyte @nzw921rx for the review — all feedback has been addressed.

@nzw921rx — Adopted Conditions.extension for early table_options validation in JdbcSinkFactory.optionRule() (JdbcTableOptionsConditionExtension + JdbcTableOptionsValidator), so invalid keys fail at --check / job submission instead of only at runtime DDL.

@dybyte — Updated JDBC sink docs (en/zh): MySQL / TiDB → MySQL, and noted early validation via option rules. Clarified SinkConnectorCommonOptions.TABLE_OPTIONS as an experimental, dialect-agnostic common option (supported keys live in connector docs, so we don't need to touch the API module when adding new dialects).

Could you please take another look when you have a moment?

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from JdbcSinkFactory down into the option-rule validation path, the MySQL DDL builder, the new E2E, and the updated docs.

The two main maintainer concerns from the last round are addressed on the current head:

• @nzw921rx's early-fail request is now implemented through Conditions.extension(...) plus JdbcTableOptionsConditionExtension.
• @dybyte's support-boundary concern is now reflected in both docs and the common-option description: the implementation is currently JDBC/MySQL only, and the option is explicitly described as experimental and sink/dialect-specific.

From Daniel's side, I do not see a reopened source-level blocker on the current head.

What this PR solves

• User pain: when JDBC sink auto-creates a MySQL target table, users previously could not pass table-level DDL properties like engine, charset, or collate through the normal sink config path.
• Fix approach: add table_options as an experimental common sink contract, validate it early through optionRule, propagate it into CatalogTable.options, and let the MySQL DDL path append the corresponding clauses during auto-create.
• One-line summary: the feature is now a real end-to-end MySQL auto-create path with early validation, not just a surface-level config knob.

Full runtime path I checked

job submission / --check
  -> JdbcSinkFactory.optionRule() [214-258]
      -> SinkConnectorCommonOptions.TABLE_OPTIONS [248-255]
      -> JdbcTableOptionsConditionExtension.evaluate(...) [44-55]
          -> JdbcTableOptionsValidator.validate(...) [36-49]
              -> JdbcDialectLoader.load(...)
              -> dialect.validateTableOptions(...)

auto-create path
  -> JdbcSinkFactory.createSink(...) [74-210]
      -> config.get(TABLE_OPTIONS) [76]
      -> catalogTable.getOptions().putAll(sinkTableOptions) [186]
  -> MySQL save-mode create table path
      -> MysqlCreateTableSqlBuilder.build(...)
          -> append ENGINE / DEFAULT CHARSET / COLLATE

E2E verification
  -> JdbcMysqlTableOptionsIT.testTableOptionsSink(...) [133-143]
      -> executeJob(CONFIG_FILE)
      -> SHOW CREATE TABLE [148-166]
      -> SELECT COUNT(*) [168-173]

Key findings

1. The normal MySQL auto-create path really does hit this change end to end. The option is validated early, propagated into CatalogTable.options, and consumed by the MySQL DDL builder.
2. Unsupported dialects now fail fast at option validation time instead of waiting until runtime DDL, which is the right safety boundary.
3. The docs are now aligned with the actual implementation boundary: current support is MySQL only, not a broader “MySQL / TiDB” promise.
4. I do not see a reopened correctness issue on the current head.

Findings

Issue 1: the latest Build is still pending, so CI remains the final merge gate

• Location: GitHub checks / Build
• Why this matters:
  • I do not see a new source-level blocker on head 827b8f38e1d9104b66b507904b096eeb18f62e4d.
  • But this PR still changes common option plumbing, dialect validation, DDL generation, docs, and an E2E, so the latest Build on the current head still matters before merge.
• Risk:
  • Merging before the current Build finishes could still miss a module-assembly or E2E regression on the latest revision.
• Best fix:
  • Keep the code as-is and merge after the current Build finishes green.
• Severity: Low
• Raised by others already: No; this is the only remaining gate from Daniel's current-head review.

Tests and stability

• The test coverage is materially complete for the current scope:
  • MySQL DDL builder unit coverage
  • dialect allowlist / unsupported-dialect validation coverage
  • option-rule early-validation coverage
  • E2E coverage via SHOW CREATE TABLE plus exact row count
• Stability rating: Stable.
• Basis:
  • the new unit tests are deterministic and in-memory
  • the E2E does not rely on fixed sleeps and uses strong assertions (COUNT(*) == 3, not just “some rows arrived”)

Compatibility

• Assessment: fully compatible.
• Existing jobs that do not use table_options are unaffected.
• Non-MySQL dialects now fail earlier when users provide non-empty table_options, which is the correct fail-fast behavior rather than a compatibility break.

Conclusion: can merge after fixes

1. Blocking items

• Issue 1: please wait for the current Build on head 827b8f38e1d9104b66b507904b096eeb18f62e4d to finish green before merging.

2. Suggested non-blocking follow-up

• No new source-level follow-up from Daniel's side on top of the current head.

Overall, the current revision closes the two main review gaps from the previous round and now looks coherent from option contract down to the MySQL DDL / E2E proof. From Daniel's source-level review side, the remaining gate is CI rather than code.

[dybyte]

+1 if CI passes

[DanielLeens]

Thanks for the follow-up. I rechecked the current PR state before replying: from Daniel side there is still no new code on top of head 827b8f38e1d9104b66b507904b096eeb18f62e4d, so I am keeping this in reply-only state rather than starting another full review round on the same revision.

@dybytes latest +1 if CI passes is also aligned with my current view: I do not have a new source-level blocker on this unchanged head. If a new commit lands, or if there is a concrete CI-related update that changes the merge discussion, please ping me again and I can re-check right away.

[DanielLeens]

Thanks for the follow-up. I re-reviewed the latest head from the JDBC sink table-options validation path again.

What I rechecked on the current head

JDBC sink submit-time path
  -> JdbcSinkFactory.optionRule()
      -> JdbcTableOptionsConditionExtension
      -> JdbcTableOptionsValidator
  -> MysqlDialect / create-table builder
  -> JdbcMysqlTableOptionsIT

I also compared the current PR file surface with the last Daniel-reviewed source revision. I do not see an effective reopened change in the actual JDBC table-options files that would invalidate the earlier source-side conclusion.

Current conclusion on the latest head

• The table-options validation logic still looks source-clean to me on the current revision.
• The MySQL auto-create-table path and the explicit validation boundary remain aligned.
• The remaining gate is still the latest Build, which is in progress.

Conclusion: can merge after CI is green

1. Blocking items

• No new source-level blocker from my side on the current head.
• Please let the current Build finish green before merge.

2. Suggested non-blocking follow-up

• None from this round.

[DanielLeens]

Thanks for the update. I rechecked the current head against the latest dev, and this branch is still behind upstream.

• base branch: dev
• head SHA: edb879a937bbb53e31a6f35d215e5bf6df04b416
• compare status: diverged
• ahead_by: 9
• behind_by: 4
• mergeable_state: unknown
• CI / merge gate snapshot: failing (failing: Build [failure])

Because the PR is still behind the latest base, any CI result and merge gate signal here is mixed with upstream drift. The lowest-cost next step is to sync with the latest dev and rerun CI first. If anything still fails after the sync, I'm happy to take another look on top of the updated head.

[davidzollo]

+1
LGTM
Thanks for your contribution! This is a solid addition to the project, and your attention to detail really shows. Contributors like you are what keep this community moving forward. Feel free to keep the PRs coming — we're always glad to have you here!