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

   > Thank you @peter-toth -- this seems like an improvement to me. Perhaps 
@viirya and @berkaysynnada can take a look too prior to merging this
   
   Thanks @alamb for the review. Just a note that in this discussion: 
https://github.com/apache/arrow-datafusion/pull/8672#discussion_r1438363204 
some concerns came up regarding the new `transform_with_payload()` methods 
might not be able to cover all usecases and so can't replace all derived trees. 
cc @ozankabak
   I might need a few days to check that with a more complex derived tree like 
`OrderPreservationContext` and extend this PR or open a follow-up PR if 
refactor is possible.
   But, I think even if it turns out that not all derived trees can be 
refactored, IMO the new methods are still useful as the usecase of 
`SortPushDown`, `PlanWithRequitements` and `ExprOrdering` proved.
   


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