Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/839
  
    @paul-rogers 
    1. Renamed PR as you suggested to better convey changes idea. Thank you for 
suggestion.
    2. Included row count into batch size determination.  Thus batch size will 
be limited to max allowed rows (as before if memory limit does not exceed) or 
to number of records that are within max allowed memory limit.
    3. Unfortunately did not add unit tests, Not sure if there is a way to test 
Hbase reader in isolation for true unit testing (I know you have been working 
to some changes in this area but I don't know the details).
    But I have tested the fix manually, changing in code max rows size and 
memory allocation. Could not write Drill "unit tests" for this since it's not 
easy to mock `static final int fields` and adding such large data sets to test 
on real numbers may extend our unit tests running time. I guess in this case 
it's better to add such test to Functional tests suite. So if possible let's 
leave this change without unit tests for now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to