CodeWithPravinMaske commented on PR #12608: URL: https://github.com/apache/seatunnel/pull/12608#issuecomment-5970582443
Thanks a lot @SEZ9 and @DanielLeens for the thorough reviews. You were right about the binlog DDL path, so I reworked the fix to address the root cause instead of patching one code path. **Issues 1/2 (SEZ9) / Issue 1 (Daniel): binlog DDL and checkpointed history bypass the fix** The NOT NULL comes from Debezium 1.9.8's `MySqlAntlrDdlParser#parsePrimaryIndexColumnNames`, which every DDL parse goes through: the snapshot `SHOW CREATE TABLE` / `DESC`, `parseStreamingDdl` in the binlog reader, and SeaTunnel's schema evolution parser. Debezium 2.x no longer promotes a unique key that has nullable columns (`parseUniqueIndexColumnNames`). - The parser is now overridden in the existing `io/debezium` override folder (same pattern as `MySqlStreamingChangeEventSource`), with one change: only a real primary key (`PRIMARY KEY (...)` / `ALTER TABLE ... ADD PRIMARY KEY`) forces its columns NOT NULL. A promoted unique key keeps its columns as declared, from `CREATE TABLE`, `ALTER TABLE ... ADD UNIQUE KEY` and `CREATE UNIQUE INDEX` alike. The promotion itself is unchanged, to keep the change minimal. - Since every schema is now built correctly at parse time, the tables captured into `historyTableChanges` and checkpointed are correct too. `MySqlSchema#restoreNullableColumns` is removed, and `MySqlSchema` is back to the `dev` version. - Unit tests (`MySqlSchemaTest`): `parseStreamingDdl` with `ALTER TABLE ... ADD UNIQUE KEY` and `CREATE UNIQUE INDEX` keeps the column optional, while a real primary key (table constraint and `ALTER TABLE ... ADD PRIMARY KEY`) stays NOT NULL. With Debezium's original behaviour restored, 3 tests fail. - E2E: a fifth table `uk_added_null` starts without any key and gets `ALTER TABLE ... ADD UNIQUE KEY` during the binlog phase, then a NULL is inserted. E2E_RESULT_PLACEHOLDER **Issue 3/4 (SEZ9) / Issue 3 (Daniel): nullable column configured in `table-names-config.primaryKeys`** With the parser fix, the column keeps its declared nullability, and `mergeCatalogTableConfig` only overrides the key names, so NULL is now emitted as NULL there as well. The silent coercion no longer exists. The MySQL-CDC docs (en/zh) now say so and recommend a NOT NULL key column when UPDATE/DELETE must be applied by key. **Issue 6 (SEZ9) / Issue 2 (Daniel): behaviour change not documented** Added an entry to `incompatible-changes.md` (en/zh) with the old and new output, the sink impact (`NOT NULL` columns, sink `primary_keys`) and migration guidance. It also states the one remaining limitation: a job restored from a checkpoint taken by an older version, whose history already holds such a table (after a binlog DDL on it), keeps the old schema for that table until it is started without that checkpoint. **Issues 5/7/8 (SEZ9) / Issues 5/6 (Daniel): null guard, Javadoc, INFO log** These are gone together with `restoreNullableColumns`. **Issue 4 (Daniel): NULL rows skipped by multi-chunk snapshots on a nullable split column** Confirmed. This is fixed separately in #12597 (for #12598): a nullable column is no longer used as the snapshot split column. Happy to adjust anything else. -- 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]
