adriangb commented on code in PR #20348:
URL: https://github.com/apache/datafusion/pull/20348#discussion_r2813666975


##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -236,6 +236,12 @@ impl Default for Optimizer {
 impl Optimizer {
     /// Create a new optimizer using the recommended list of rules
     pub fn new() -> Self {
+        // NOTEs:
+        // - The order of rules in this list is important, as it determines the
+        //   order in which they are applied.
+        // - Adding a new rule here is expensive as it will be applied to all
+        //   queries, and will likely increase the optimization time. Please 
extend
+        //   existing rules when possible, rather than adding a new rule.

Review Comment:
   ```suggestion
           // - Adding a new rule here is expensive as it will be applied to all
           //   queries, and will likely increase the optimization time. Please 
extend
           //   existing rules when possible, rather than adding a new rule.
           //   If you do add a new rule considering having aggressive no-op 
paths
           //   (e.g. if the plan doesn't contain any of the nodes you are 
looking for
           //    return `Transformed::no`; only works if you control the 
traversal).
   ```



-- 
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]

Reply via email to