zhuqi-lucas commented on PR #19446: URL: https://github.com/apache/datafusion/pull/19446#issuecomment-3689288787
> Looks very cool! I suggest splitting out the changes to `FilterExec` for further consideration and moving the tests to a different SLT file (`order.slt`? start an `order_pushdown.slt`?) Thank you @adriangb for review, i addressed the comments to add a new sort_pushdown.slt. But i still keep the bypass sort pushdown for : ```rust FilterExec::try_pushdown_sort ProjectionExec::try_pushdown_sort CooperativeExec::try_pushdown_sort ``` And they all just bypass self to the child sort pushdown, i was testing and also the new slt testing here, it seems need this, and they are the basic cases i think. I also created a issue for the following operators to add more try_pushdown_sort implementation, we can investigate more for it: https://github.com/apache/datafusion/issues/19394 What do you think? -- 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]
