SEZ9 commented on PR #11912: URL: https://github.com/apache/seatunnel/pull/11912#issuecomment-6568630125
Quick status update on this PR at head `d3a8a348ddb08c9866c305276ec8f91b01f0953f`: **Merge gate** - GitHub now reports actual content conflicts with `dev` (`CONFLICTING` / `DIRTY`), so this is a real conflict to resolve rather than a branch-age sync. Please rebase/merge the latest `dev`, keeping both this PR's intended change and the intervening upstream changes intact. - The existing-head [Build](https://github.com/apache/seatunnel/runs/120034146429) check is at `ACTION_REQUIRED`; once the conflicts are resolved, the required checks will need to go green on the new head. **Open review items** Resolving the conflict on its own doesn't close the source-level points from the [previous review](https://github.com/apache/seatunnel/pull/11912#pullrequestreview-5625202431). The ones I'd still like to see addressed (or explicitly discussed) in the updated head: 1. `MultiTableSink` eagerly creating one `SchemaChangeApplier` per table in every parallel writer — this holds external connections for the whole job lifetime and multiplies resource usage by parallelism × table count, even for jobs that never use coordinated schema evolution. Lazy creation would be preferable. 2. `createSchemaChangeApplier(TablePath)` resolving the target sink nondeterministically when several logical tables map to the same physical sink table, plus the key mismatch (logical `fullName` key vs. physical-path applier) in the applier map. 3. The `SchemaChangeApplier` contract executes source-derived DDL but says nothing about identifier safety — please document that requirement in the SPI javadoc. 4. The `SupportCoordinatedSchemaEvolutionSink` requirement that writers implement `SupportSchemaRefreshSinkWriter` is javadoc-only; a partially implemented connector currently fails or silently skips the refresh at runtime mid-DDL. A compile-time or startup-time check would be much safer. 5. Docs: a documentation update for the new coordinated schema evolution capability (concept docs / connector dev guide), and an upgrade note on checkpoint/savepoint restore for schema-evolution jobs after the Flink sink coordination changes. The schema-refresh/checkpoint ordering blocker from that review also remains open; resolving the conflict alone doesn't supply the downstream-refresh acknowledgement it asks for. If any of these were already handled, a short pointer in the PR description would be enough. Happy to take another look as soon as the rebased head is up. <!-- streview-comment:2224 --> -- 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]
