2010YOUY01 commented on code in PR #26107: URL: https://github.com/apache/datafusion/pull/26107#discussion_r4235964676
########## datafusion/physical-optimizer/src/optimizer.rs: ########## @@ -16,6 +16,63 @@ // under the License. //! Physical optimizer traits +//! +//! # Physical Optimizer Contract +//! +//! This section explains the contract for extending the default list of +//! optimizer rules. +//! +//! [`PhysicalOptimizer::new`] defines the default rule sequence: +//! +//! ```text +//! // Default rules +//! let rules = vec![ +//! rule1, +//! rule2, +//! rule3, +//! rule4, +//! // ... +//! ]; +//! ``` +//! +//! 1. **Keep the default order.** Rules may rely on properties established by +//! earlier rules. Correctness is only guaranteed in the default order. +//! +//! 2. **Use configuration to disable optimizations.** Configuration options +//! provide supported variations of the default pipeline that preserve +//! correctness. For example, +//! `SET datafusion.optimizer.enable_distinct_aggregation_soft_limit = false` +//! disables the distinct aggregation soft-limit optimization. Removing rules +//! directly from the pipeline may produce invalid plans. +//! +//! 3. **Adding optimizer rules.** +//! +//! 1. Rules added within DataFusion or downstream must respect the +//! assumptions of the surrounding rules. Many of these are implicit or +//! documented only in individual rules. Changes to the default pipeline +//! may require updates to rules that rely on them. +//! +//! 2. DataFusion aims to make these assumptions easier to understand and +//! verify. +//! +//! 3. Extension rules should adapt to the built-in rules, not the other Review Comment: I tried to propose a practical way to restrict it reasonably in - (see point 3) https://github.com/apache/datafusion/issues/26171 I think if we leave it unspecified, this assumption could be interpreted too broadly and eventually become a maintenance issue. -- 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]
