peter-toth commented on PR #9913:
URL: 
https://github.com/apache/arrow-datafusion/pull/9913#issuecomment-2035312114

   > Thank you (again) @peter-toth -- this looks really nice as well
   > 
   > I think there are is some more documentation we could add, but overall the 
only concern I have about this PR is the lack of test changes (like it seems to 
change the semantics of the analyzer / correlated expressions but no tests are 
changed). So this perhaps suggests we have a lack of test coverage in this area 
🤔
   
   I rephrased the PR description to be clear. This PR doesn't change the 
analyzer or anything that uses `LogicalPlan` APIs. This PR just moves the 
overriden `TreeNode` APIs (`apply()`/`visit()`) in `LogicalPlan` to under new 
names so as to be clear they recurse into subqueries. Also, this PR adds the 
missing `LogicalPlan::rewrite_with_subqueries()` that is capable to rewrite 
`LogicalPlan`s with their subqueries.


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

Reply via email to