[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-415126887 @arina-ielchiieva - Nice catch. @weijietong - Please remove [this line](https://github.com/apache/drill/pull/1334/files#diff-e58119193708a202639717568263672fR43) and line 78 from same file. The extra WARN lines are because of enabling logger in those tests cases. Since logger is not closed it's not restored back, hence for all future tests WARN level log is enabled. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-414020875 Thanks for all the changes. I ran the regression tests and everything was green. +1 LGTM from my side. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-413248827 @weijietong - Would have been easier to just cherry-pick the commits on top of your rebased branch. But I guess it's fine. `recordCount` is not the actual record count, its the selected number of records. I have also added comments to differentiate between the two variables. Few things to resolve: 1) Since I have already shared a commit with generated protobuf files for c++ client. It would be good to take that as well. 2) Please resolve the travis build failure. 3) Meanwhile I ran tests on my shared branch yesterday and there are unit tests failures. Can you run unit tests locally and make sure all of them are passing. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-412738302 @weijietong - Thanks for making the changes. But I am seeing issues in implementation related to how internals work, following protocol of iterative model of Drill and few other things. I have left few comments for now and was wondering that it will be easier to provide a commit myself to address those issues and help you get this PR pushed in faster. Also I was working on a change for SV2 optimization [here](https://github.com/sohami/drill/commits/SV2_Optimization). Given you also have subset of the change for it with some issues, it would be great if you can rebase on top of my change. Please let me know what you think. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-411552364 Overall other changes looks good but `ScanBatch` still has issues as before. I will comment in JIRA for ScanBatch Issue since github is very slow. Also for future reference please add separate commit for changes made to address review comments, that will make it easier to review just those changes. :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sohami commented on issue #1334: DRILL-6385: Support JPPD feature
sohami commented on issue #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#issuecomment-410301857 Sorry for delayed response but I promise to complete review by this weekend. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
