davidzollo commented on PR #10744: URL: https://github.com/apache/seatunnel/pull/10744#issuecomment-4478304916
> > @davidzollo @dybyte @DanielLeens — closing thought before merge. > > This PR delivers the file-sink half of schema evolution. End-to-end on a real CDC pipeline (MySQL → S3 Parquet), I found three CDC-source-side fixes are also required for the feature to work correctly in restart scenarios. Without them, an upstream user enabling schema_evolution_enabled=true will hit one of: > > > > 1. Null values for new columns— when job restores from pre-DDL checkpoint and resumes streaming from a binlog position past the DDL (DDL during outage scenario). Root cause: Debezium's MySqlSchema cache + the deserializer's stale converter list. > > 2. AIOOBE on first row after restart-with-DDL —SeaTunnelSourceCollector captures rowType at task-open time; when the deserializer is later restored to a post-ALTER (wider) rowType,getBytesSize(oldRowType) AIOOBEs. Live rowType supplier fixes this. > > > > 3.tinyint(1) columns added via ALTER come through as TINYINT, not BOOLEAN — length info is dropped during MySqlSchema rebuild. Affects MySQL CDC + int_type_narrowing users. > > I have working implementations of all three on a private branch,validated end-to-end on a stage cluster. They're all CDC-source-side , so they fix schema delivery for all CDC sinks (Iceberg, JDBC, file), not justfile sink. > > Question: would you prefer (a) bundle these into this PR — feature is complete end-to-end on merge but PR scope grows beyond "file sink schema evolution" > > (b) keep this PR file-sink-scoped and merge it; I'll open a separate "CDC source-side schema evolution hardening" PR within the week that depends on this PR > > (c) any other split you'd prefer > > I'd marginally lean (b) for cleaner review boundaries, but (a) is also fine if you'd rather not have a temporarily-broken feature in dev. Your call. > > @davidzollo what are your views on this part? Hi @ricky2129, thanks for the thorough write-up and the end-to-end validation on a real CDC cluster — really appreciate the depth here. On PR scope: going with (b). The file-sink changes are already well-reviewed and stand on their own correctness. The CDC source fixes touch connector-cdc-mysql and SeaTunnelSourceCollector — different modules, different reviewers needed, and bundling them would make both halves harder to review properly. Please open the follow-up CDC source PR within the week and reference this PR in it. Also please add a clear note in this PR's description calling out the known limitation (restart-with-DDL scenarios) and the dependency on the follow-up, so users who enable the feature know what to expect. On schema_evolution_enabled=false + source sending AlterTableEvent: going with (a) fail-fast. The current no-op is actually worse for backward compatibility than fail-fast — it silently leaves sinkColumnsIndexInRow stale and surfaces as a confusing ClassCastException several rows later with no actionable context. The fail-fast path only fires when the source is actively sending schema change events (i.e. schema-changes.enabled=true), so users on the default config are completely unaffected. The error message should be explicit about the fix path — something like: Received AlterTableEvent but schema_evolution_enabled=false at this sink. Either set schema_evolution_enabled=true to handle schema changes, or set schema-changes.enabled=false at the CDC source to suppress them. Please push both changes and we can move this toward merge. -- 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]
