924060929 commented on PR #63366: URL: https://github.com/apache/doris/pull/63366#issuecomment-4674684816
### Follow-up on the [P1] check (AddLocalExchange) — removed it entirely After thinking it through (and with the CI evidence), the serial→non-serial validation isn't worth keeping in any form — neither throw nor warn: 1. **It reads the same `isSerialOperatorOnBe` flag it would validate**, so it's blind to the one class of bug that actually matters — a *wrong serial flag*. The framework and the check would agree on the same wrong answer; a check can't validate an input it shares with the thing it's checking. 2. **It fires on legitimate plans** — a serial child feeding a parent that requires PASSTHROUGH/noRequire (TableFunction, NLJ, Agg) needs no LE and is correct. So as a `warn` it cries wolf, and as a `throw` it breaks those plans (the 9 P0 patterns from the last run). 3. **The one class it *could* catch** — a `require`→LE logic regression (e.g. a broken `satisfy()`) — the regression/RQG already catch end-to-end, just without the FE fail-fast. Net: it adds noise and false confidence, not safety. The real oracle is the end-to-end run (which is exactly what surfaced the throw's false positives — no FE self-check did). The principled direction is a single source of truth for the serial/distribution decision (BE reading the FE-written flag) so there's no FE-prediction-vs-BE-reality gap to police in the first place. Removed the method + its call. Correctness still comes from `enforceRequire` inserting the LEs, validated by the regression/RQG. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
