viirya commented on PR #25004: URL: https://github.com/apache/datafusion/pull/25004#issuecomment-5598083756
Thanks for the re-review, and you're right about the coverage description — that imprecision was in my comment, not the code. I wrote "a real-plan test for each direction," but only the coordinate-then-drop case goes through a real plan (`nlj_pending_drop_cancels_active_peer_real_plan`); the drop-then-coordinate case is a unit test calling `FallbackCoordinator::begin_coordination` directly on a bare coordinator. For what it's worth, the test's own doc comment already draws that line — it says the opposite order "runs on a real plan in `nlj_pending_drop_cancels_active_peer_real_plan`" — so the source doesn't carry the overstatement. I'll keep the split as it is: the direct call is what lets that test pin the deferral without a plan that has to spill, and the ordering that actually regressed is the one covered end to end. -- 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]
