jayzhan211 commented on issue #26481: URL: https://github.com/apache/datafusion/issues/26481#issuecomment-6472700773
@2010YOUY01 yes, that was the idea, and the PoC says no. I moved each rule to the other side of `EnsureRequirements` and ran all sqllogictests. Only `EnsureCooperative` moved without changing plans. Moved after enforcement, the analyzer rules lose plans or break them (`join_selection` gives 767 invalid plans, `WindowTopN` keeps the window's sort, `LimitedDistinctAggregation` loses its limit). Moved before it, some optimizer rules give wrong results (`LimitPushdown`). So I agree with your guideline, and with the contract you describe. One addition on the analyzer side: its rules still have to keep results unchanged and stay correct on plans that already contain repartitions and sorts, because re-optimization (e.g. Ballista AQE) runs them on enforced plans. I think that is the other technique for the `WindowTopN` and `join_selection` invariants. Within one planning pass, the split already makes every analyzer rule run before every optimizer rule; when a plan is optimized again, they only need to skip shapes they don't recognize. I'll follow up with the per-rule numbers and what this means for #26757. -- 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]
