You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/05/26 02:52:19 UTC

[GitHub] [arrow-datafusion] jackwener opened a new issue, #2620: [enhancement] rules don't need to recursion inside themselves

jackwener opened a new issue, #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620

   **Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
   Now, all rules in optimizer must handle `subplan` by recursing optimize themselves.
   
   I'd like to let optimizer to handle `subplan`, rule just focus on the `optimize logic`.
   
   I have add optimizer struct for this PR in #2616 .
   
   **Describe the solution you'd like**
   Handle the `subplan` in optimizer by  recursing optimization.
   
   **Describe alternatives you've considered**
   
   **Additional context**
   


-- 
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: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jackwener closed issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
jackwener closed issue #2620: [enhancement] rules don't need to recursion inside themselves
URL: https://github.com/apache/arrow-datafusion/issues/2620


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jackwener commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
jackwener commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1165281099

   Currently, `datafusion` optimize the plan many times. Because the all rules will traverse the whole plan tree one time.
   Times is O(n). (n is number of rules).
   
   I think it's not good choice. But now `explain verbose` print the changes after apply one rule. This ability depends on apply rule one by one.
   
   I think the better and usual practice is traverse plan instead of traverse rules.
   
   ```
   Now:
   rules.foreach(
        plan.apply(rule)
   )
   
   After
   DFS(plannode) {
        plannode.appy(rules).
   }
   
   ```


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
alamb commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1165620421

   > How do you think about this trade-off
   
   I think it may be quite challenging to implement all the rewrite rules in parallel like this, though it would be interesting to try.
   
   In terms of efficiency, I think it would be worth profiling to see if the optimizer it taking significant time and if so if there are any tall poles that can be addressed (having each pass make a `clone()` of the input plan would be one thing I suspect could be improved)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jackwener commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
jackwener commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1138111535

   It need some time๐Ÿ˜„


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
alamb commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1144740198

   I think it would be awesome to create some sort of `LogicalPlanRewriter` that is in the style of `ExprRewriter` or `ExprVisitor` and captures the traversal pattern across a plan


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
alamb commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1351558148

   ๐ŸŽ‰ 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] jackwener commented on issue #2620: [enhancement] rules don't need to recursion inside themselves

Posted by GitBox <gi...@apache.org>.
jackwener commented on issue #2620:
URL: https://github.com/apache/arrow-datafusion/issues/2620#issuecomment-1165283020

   @alamb @Dandandan @andygrove @houqp 
   
   How do you think about this trade-off


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org