houqp commented on pull request #1010: URL: https://github.com/apache/arrow-datafusion/pull/1010#issuecomment-935662269
+1 for merging https://github.com/rdettai/arrow-datafusion/pull/1 into this PR. I think the changes in this PR is really really solid. I think the newly added code is ready to merge for sure. My only recommendation would be to remove the old format execution plan code in the same PR to make git blame easier to keep track of the changes and the git diff easier to reason for future developers. It looks like we are almost there in terms removing the legacy code. We can even just wrap the existing csv/parquet/json table providers with the new list table providers and call it a day. The biggest pain with long running PRs is conflict resolution. But given what we have right now is only additive change, I think there is not much of a difference between sending follow up clean up PRs into this PR v.s. master. I could help with the final clean up too during weekends if you want to keep this PR going. That said, it's not a strong no from me and I am cool with us merging it into master if you all think this would speed up the iteration process. -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
