Kristofferjoha commented on PR #26719: URL: https://github.com/apache/datafusion/pull/26719#issuecomment-6497184302
> > @jayzhan211 @kumarUjjawal Can I get your opinion on this? Im pretty sure we should avoid it upfront, but let me know what you think is best here > > I’d keep the limit where the sort was removed and let `LimitPushdown` clean it up. It already runs after `EnsureRequirements` and can absorb the redundant limit into the merge. When a window blocks pushdown, the limit must remain above it. This keeps limit preservation explicit without duplicating cleanup logic here. A plan assertion showing that the redundant wrapper disappears would be useful. > > The optimizer order and absorption logic support this approach: [rule order](https://github.com/apache/datafusion/blob/4febe93c53f54e6964bfe045314012caec0f2dbb/datafusion/physical-optimizer/src/optimizer.rs#L250-L276), [limit absorption](https://github.com/apache/datafusion/blob/4febe93c53f54e6964bfe045314012caec0f2dbb/datafusion/physical-optimizer/src/limit_pushdown.rs#L267-L286). Thanks, that makes sense. I’ve kept the limit where the sort is removed and added the assertion. -- 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]
