SEZ9 commented on PR #12079: URL: https://github.com/apache/seatunnel/pull/12079#issuecomment-6553381175
Thanks for the follow-up at 67f3bde2506a. Based on the latest automated review pass over the full diff, the picture against the earlier findings looks like this: - **F1 (duplicate output IDs double-releasing joins)** — the switch to distinct missing-ID tracking plus `waitingByInputId.remove(outputId)` so a repeated producer satisfies an ID only once addresses this. Please make sure there is a unit test with two transforms both omitting `plugin_output` (both landing on `DEFAULT_ID`) feeding a multi-input downstream, asserting it is scheduled only after all distinct inputs are available and does not hit "missing scheduled inputs". - **F5 (scheduler duplicated between dry-run and `MultipleTableJobConfigParser`)** — resolved by the shared indexed scheduler used for graph checks, runtime parsing and connect dry-run. - **F6 (single-transform self-cycle slipping through the legacy fallback)** — resolved: Kahn cycle detection now runs before the narrow terminal fallback, so an explicit self-cycle fails with a concrete cycle path. - **F7 (docs/behavior-change note)** — resolved with the added method contracts. - **F2 / F4 (narrowed terminal fallback, multi-transform jobs relying on the implicit last-schema behavior)** — the fallback is now described as "terminal omitted/empty or single-transform total-mismatch". Could you confirm explicitly that a multi-transform job whose *last* transform omits `plugin_input` still resolves via `DEFAULT_ID` / the terminal fallback rather than raising `ConfigCheckException`, ideally with a test covering that shape? - **F3 / F8 (`getTransformInputIds` vs `getInputIds` divergence; duplicated ID helpers and the unused `getOutputId()`)** — these were the "optional runtime-ID reuse" items and remain non-blocking. If you'd rather not fold them into this PR, a short note on whether the two input-ID paths are now guaranteed to agree (or whether one delegates to the other) is enough for me. On CI: the connector-lane failure was reported as a Docker Hub 502 during Spark image setup, so please re-trigger once the build finishes and we'll re-check the full result before merging. So the concrete remaining asks are: (1) the duplicate-`DEFAULT_ID` join test for F1, (2) confirmation/test for the multi-transform last-transform-omits-`plugin_input` case for F2/F4, (3) a one-line answer on F3/F8, and (4) a green rerun. Nothing else outstanding from my side. <!-- streview-comment:2200 --> -- 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]
