[Feature][Connector-V2] PR1: Pass sink table-options into auto-created MySQL target tables#11120
[Feature][Connector-V2] PR1: Pass sink table-options into auto-created MySQL target tables#11120luxiaolong-ct wants to merge 4 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for working on this. I reviewed the full diff from JdbcSinkFactory down into the MySQL create-table builder and the new E2E, rather than treating this as a docs-only option addition.
What this PR fixes
- User pain: when JDBC sink auto-creates a MySQL target table, users currently have no real way to pass table-level options such as
engine,charset, orcollatethrough the sink config and get them into the generated DDL. - Fix approach: add shared
table_options, propagate them intoCatalogTable.options, validate them per dialect, and let the MySQL create-table builder append those clauses to the finalCREATE TABLE. - 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(...) [connector-jdbc/.../JdbcSinkFactory.java:71-208]
-> read table_options [73]
-> put table_options into CatalogTable.options [183]
-> load JDBC dialect [187-192]
-> validateTableOptions(...) [193]
Dialect boundary
-> JdbcDialect.validateTableOptions(...) [connector-jdbc/.../JdbcDialect.java:916-925]
-> non-empty table_options fail fast by default
-> MysqlDialect.validateTableOptions(...) [connector-jdbc/.../MysqlDialect.java:402-418]
-> allow only engine / charset / collate
Auto-create path
-> save-mode / catalog createTable(...)
-> MysqlCreateTableSqlBuilder.build(...) [connector-jdbc/.../MysqlCreateTableSqlBuilder.java:138-157]
-> ENGINE = ... [145-147]
-> DEFAULT CHARSET = ... [148-150]
-> COLLATE = ... [151-153]
TiDB compatibility boundary
-> JdbcDialectLoader.load(url, compatibleMode, dialect, fieldIde)
-> MySqlDialectFactory.create(...) [connector-jdbc/.../MySqlDialectFactory.java:44-48]
-> still returns MysqlDialect for MySQL-family compatible modes except StarRocks
Key findings
- The normal MySQL auto-create path really does hit this change.
table_optionsis not only accepted syntactically; it reaches the final DDL builder. - The fail-fast behavior for unsupported dialects is the right safety boundary here. Unsupported JDBC dialects do not silently ignore user intent.
- I did not find a blocking correctness issue in the current head.
- The current docs mention MySQL / TiDB together. From the source path that is reasonable, because the sink still routes MySQL-family compatible modes through
MysqlDialect; the current tests just do not add a TiDB runtime pass yet.
Test / stability note
MysqlDialectTestcovers supported keys, unknown keys, and unsupported-dialect rejection.MysqlCreateTableSqlBuilderTestchecks that the generated DDL really contains the expected clauses.JdbcMysqlTableOptionsITusesSHOW CREATE TABLEplus row-count verification, which is the right signal for this feature and does not rely on weak timing assumptions. I did not see an obvious flaky-test anti-pattern in the new test code.
My merge conclusion for the current head is: can merge.
Blocking items
- None from my side.
Suggested follow-up
- A TiDB runtime coverage pass would still be useful later, since the docs explicitly mention MySQL / TiDB together and the current verification is source-level plus MySQL E2E only.
Overall, the implementation is clean and the scope is well controlled. The option, validation boundary, DDL wiring, E2E, and EN/ZH docs are aligned on the current head.
DanielLeens
left a comment
There was a problem hiding this comment.
Thanks for the update. I re-checked the current head from the sink factory through MySQL dialect validation and the updated E2E path, and I do not see a blocker from the current source state.
What this PR fixes
- User pain: Users could configure JDBC MySQL
table_options, but those options were not really reaching the auto-create-table path before. - Fix approach: The PR passes
TABLE_OPTIONSinto the sink-side catalog table options and letsMysqlDialectvalidate the supported option set. - One-line summary: MySQL auto-create-table can now really receive and validate table-level options such as
engine,charset, andcollate.
Runtime path I traced
Job submission
-> `JdbcSinkFactory.createSink()` reads `TABLE_OPTIONS` and writes them into the sink-side `CatalogTable` options
-> `dialect.validateTableOptions(...)` validates the passed map
Dialect path
-> `MysqlDialect` explicitly supports `engine`, `charset`, and `collate`
Result
-> MySQL auto-create-table receives the intended table options end to end
Key findings
- The normal JDBC MySQL auto-create path hits this change directly.
- Factory-side propagation and dialect-side validation are cleanly separated.
- The current head looks wired end to end for the intended option set.
Blocking items
None at the source-code level on the current head.
Other review notes
- No additional non-blocking source-level comments from this round.
Compatibility and side effects
- Compatibility: Fully backward compatible. This extends an optional path without breaking older configs.
- Side effects: Whitelist validation is submission-time only and does not change the runtime resource model.
- Error handling / logging: Invalid table options now fail earlier and more clearly.
- Tests: The current unit tests cover both supported and unsupported options, and the E2E path has also been extended on the branch.
- Docs: Both English and Chinese JDBC sink docs were updated.
- CI: Some checks were still in progress when I reviewed.
Merge verdict
- I do not see a blocker on the current head. The JDBC MySQL table-options path now looks wired end to end from user config into dialect validation and table creation.
|
Account operations have been restored, this PR is closed, and you can continue to complete it in #11101 |
Purpose of this pull request
Add
table-optionsfor JDBC sink auto-create flow (MySQL first delivery for #11047).table-optionsconfig inSinkConnectorCommonOptionsCatalogTableinJdbcSinkFactoryengine,charset,collateStatus — 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 whenschema_save_modetriggers DDL (e.g.CREATE_SCHEMA_WHEN_NOT_EXIST). Does not alter existing tables or runtime INSERT/UPSERT.How was this patch tested?
MysqlDialectTest,MysqlCreateTableSqlBuilderTestJdbcMysqlTableOptionsITCheck list
New License Guide
incompatible-changes.mdto describe the incompatibility caused by this PR.