[GitHub] ilooner commented on issue #1606: DRILL-6845: Semi-Hash-Join to skip incoming build duplicates, automatically stop skipping if too few

[GitBox <gi...@apache.org>]

ilooner commented on issue #1606: DRILL-6845: Semi-Hash-Join to skip incoming build duplicates, automatically stop skipping if too few
URL: https://github.com/apache/drill/pull/1606#issuecomment-454667185
 
 
   @Ben-Zvi let's not rush this PR. I agree with you we should not be trying to do things perfectly, that's why I mainly only focus on functional correctness in my reviews and avoid superficial comments about variable names and shortening lines of code. As you've experienced first hand with HashAgg, getting memory calculations right is extremely tricky, and they're even trickier to debug when users hit bugs. Let's make sure the logic is rock solid and unit tested while everything is still fresh in our minds. Doing this now will save us a lot more time in the coming months. Plus I think we are getting pretty close, I don't think there is that much code left to write.
   
   If there is a time crunch and this needs to go into our private branch soon, I have no issues with putting this into the private branch, and continuing the review process in open source. Since the changes are mainly in the memory calculators, the chances of any significant merge conflict are almost zero.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services