zhuqi-lucas commented on issue #25355: URL: https://github.com/apache/datafusion/issues/25355#issuecomment-5763338254
Strongly agree, and the chain behind #25356 is direct evidence for the split: it hand-repeats enforcement after every rewrite that can invalidate requirements, and 28 of its 35 rule passes leave the plan untouched in production. Filed the independent ticket as #25572 with the design questions we hit while running that chain, three of which seem worth deciding early: 1. Some downstream optimizer rules consume the operators enforcement materializes rather than the abstract requirement, and are deliberately scheduled after an enforcement pass today, so the split needs either a sanctioned enforce/optimize alternation or a "rules read requirements, not materialized operators" convention. 2. The loop needs revisit detection, not just "unchanged since last pass": we have a measured case where an enforcement rule oscillates between two forms, which the logical loop's `HashSet` of prior signatures handles and a last-pass comparison would not. 3. `ExecutionPlan` has no `Hash`/`Eq`, so a `PhysicalPlanSignature` has to be derived; the fingerprint from #25356 can be repurposed wholesale as that identity, and in the loop model the per-rule memoization and config key there become unnecessary, which I am happy to drop. Glad to drive #25572 and bring this chain as the case study. -- 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]
