DanielLeens commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5391439481
Thanks @SEZ9, mapping this against my own 2026-08-23 review head-by-head (still the same head `9a7bac1e4671`, no new commit since): - **F1 (destination-key collision, fixed):** agreed, confirmed independently via the fork's `unit-test` job logs on this head. - **F2 (restore-time state duplication, High):** agreed, this is the same as my own Issue 1 (`MultiTableSink.java:253-274` union not reconciled with the snapshot fan-out) -- still the primary blocker from my side too. - **F3 (schema/config divergence, arbitrary alias's sink instance backs the shared writer):** you're right that I didn't carry this into my own Issue Summary table last round, and it should have been there. I went back to the current head's actual source to check rather than just conceding on trust: `getDestinationKey`/`createWriter` (`MultiTableSink.java:144-164`) iterate `sinks.keySet()` and call `destinationWriters.computeIfAbsent(destKey, k -> sink.createWriter(proxy))`, where `sink` is whichever `TablePath` happens to be current in that `HashMap` iteration when the key is first computed -- so the writer (and therefore its schema/config) is built from one arbitrary aliased table's `SeaTunnelSink` instance, with no check that the other aliased tables are schema/config-compatible. That's a real, distinct gap from F2, not just restating it. Folding it in as its own blocking item alongside F2 for the next round: either fail fast when aliased sinks' resolved schema/config differ, o r explicitly document why divergence is safe for the connectors that opt into `getPhysicalDestinationIdentifier()` today. - **F5 (proxyContexts only registers the first alias) / F7 (`IOException` wrapped in `RuntimeException`):** agreed, same as my own Issues 5 and 6 -- F7 in particular has now survived roughly six rounds across both of us, would like to see it actually land this time. - **F4/F6 (docs) and F8 (Javadoc tags):** agreed, no new analysis needed, matches my own Issues 7 and 8. So the combined blocking set for the next round, from both our passes: F2 (restore-state duplication) and F3 (schema/config divergence on shared-writer creation), plus my own carryover Issue 2 (`createCommitter`/`createAggregatedCommitter` still build one committer per source table with no destKey dedup -- unrelated to F1-F8 but still open and not in this list). Once those close together I'll do a full re-review of the complete diff. @hesam-oxe, thanks for staying on top of this through this many rounds -- the destination-collapsing fix from earlier this week was a genuinely solid piece of work, this remaining set is the last structural piece. -- 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]
