SEZ9 commented on PR #11206:
URL: https://github.com/apache/seatunnel/pull/11206#issuecomment-6160641496
Thanks for the follow-up on `9007896511517e4b874142cfe704102861bbc444` and
for syncing with `apache/dev` at `b0f6feddf9ed0dc1f5d485cfc69d387edfe6ac82`.
Going through the previous points based on your description; I'll confirm
each against the diff:
- **F3 (`scan.newly-added-table.enabled` default)** — making it opt-in
(`false`) is the right direction for restore compatibility of existing wildcard
jobs, and thanks for updating both the English and Chinese MySQL CDC docs.
- **F2 (runtime-sink capability gate)** — failing early with the sink type
and table path for a sink that does not opt in is the behavior we want. One
ask: please make sure the Javadoc on `SupportMultiTableSinkWriter` / the
`TableSinkFactory` default spells out this fail-fast contract, so implementers
know a runtime `CreateTableEvent` is never silently dropped.
- **F1 / F8 (identifier enforcement)** — WARN-and-skip at the deserializer
boundary with a negative regression test, plus accepting digit-leading names
like `2024_orders` while rejecting all-numeric names, sounds like it addresses
both points. Please confirm that the deserializer is the only path through
which runtime-discovered names reach the generated sink SQL and the
`${database_name}_${table_name}` template, or that the same check is applied
where the SQL is built as well.
- **F7 (JDBC sink docs)** — thanks for adding the English and Chinese JDBC
sink documentation. Please make sure it states the `exactly_once = false` /
`generate_sink_sql = true` constraints for runtime-created tables.
Still open, as you noted:
- **F4 (target-scope pinning):** a source-side CREATE TABLE should only be
able to trigger DDL in the explicitly configured sink target; please add strict
routing/target validation or explain why the current gate is sufficient.
- **F5 (retry/idempotency):** we need a defined behavior when runtime writer
creation throws `IOException` — either retry with idempotent table creation, or
fail the task — so a transient failure cannot orphan the table while its binlog
rows keep flowing.
- **F6 (checkpoint format):** please document old-format `MultiTableState`
restore and rollback behavior, and add a restore test from a pre-change
checkpoint if feasible.
Since local compilation and tests were not run, I'll wait for CI to finish
before a final pass. A short note on the `MySqlIncrementalSourceFactory`
conflict resolution (which guards were kept from dev and which options came
from this PR) would also help reviewers verify the merge.
<!-- streview-comment:1750 -->
--
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]
