cgivre commented on PR #2995: URL: https://github.com/apache/drill/pull/2995#issuecomment-3019960154
> > > I submitted some minor changes. Just a reminder but we really need unit tests in order to merge this. > > > Also, have you considered adding a limit pushdown? It is usually pretty easy to do and only involves: > > > > > > * Implementing two methods in the group scan (`HiveScan`) which are: `supportsLimitPushdown` and `applyLimit`. > > > * Passing the limit through the subscans. > > > * Adding some logic in the readers to stop when the limit is reached. > > > Maybe it would be best to open a new JIRA for that, but IMHO, it is one of the easiest and most effective pushdowns that can be implemented yet Drill didn't seem to do for all the plugins. > > > > > > @cgivre ok, I will add some unit tests and support limit pushdown @shfshihuafeng Just to be clear, we can't merge this without unit tests. You don't have to do the limit pushdown if you don't want to or aren't able to, but I just wanted to recommend it as it can be a big performance benefit for not much work. Alternatively, you can create a separate pull request for that and do it later. It is your choice.. -- 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: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
