SEZ9 commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5650588757
Thanks for the follow-up. To reiterate: the commit-authorship question is closed and has no bearing on the code review or merge decision for this PR. On the substance, thanks for the F1–F8 status walkthrough on `153355ce8c`. Since nothing has changed code-wise on that head since my last review, I'll hold my next full pass until a new commit lands so I don't split one code version's findings across multiple review posts. For clarity, here is what I still expect to see addressed in that push (or a short note on why an item shouldn't change): - **F1 (destination-key collisions)** — a guard so that two tables mapping to the same destination key cannot silently share a writer unless the sharing is intentional and validated; a hard failure with a clear message is preferable to routing rows through another sink's writer. - **F2 (state fan-out + restore-time union)** — snapshot the shared writer's state once per physical destination (or dedupe on restore) so recovery does not multiply the state across aliased identifiers. A unit test covering snapshot→restore with two or more aliases would be very welcome. - **F3 (arbitrary alias used to create the shared writer)** — either assert that the CatalogTable/config of all aliased tables is compatible before sharing, or document clearly that divergence is the connector's responsibility. - **F4 / F6 (docs)** — document `getPhysicalDestinationIdentifier()` and the changed `restoreWriter` contract (merged state from all aliased identifiers delivered in one `restoreWriter` call) so connector implementers know what they're opting into. - **F5 (proxyContexts / containsValue)** — register a context for every aliased identifier rather than only the first per destination, and replace the O(n) `containsValue` lookup with a keyed structure. - **F7 (IOException wrapped in RuntimeException inside computeIfAbsent)** — surface the checked exception per the declared contract, e.g. by building the writer outside `computeIfAbsent` or unwrapping the cause. - **F8 (`getDestinationKey` Javadoc)** — add the param and return tags for consistency with the rest of the package. Once the new commit is up, I'll do one full pass against that head. <!-- streview-comment:1009 --> -- 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]
