ilooner commented on issue #1344: DRILL-6461: Added basic data correctness 
tests for hash agg, and improved operator unit testing framework.
URL: https://github.com/apache/drill/pull/1344#issuecomment-413355862
 
 
   @sohami I removed the **RowSetBatch** and made the **MockRecordBatch** work 
with RowSets. Please take another look. 
   
   This proved to be a fruitful exercise because using MockRecordBatch exposed 
some bugs and corner cases in the vectors, and then those bugs and corner cases 
lead to even more bugs being exposed. The specific issues were:
   
    - Previously I assumed **copyEntry** was a safe operation (does reallocs 
when needed), but this was not entirely true. **copyEntry** was safe for 
non-empty VarLength vectors, and Nullable vectors copying from another Nullable 
vector. It was not safe for empty VarLength vectors, copying from a 
non-nullable vector to a nullable vector, or fixed length vectors. So Included 
fixes for these three corner cases.
    - I made **copyEntry** safe for empty VarLength vectors by always making 
sure the first entry in the offset vector existed and was zero on 
initialization of the vector. This caused memory to be leaked because some 
operators created empty VectorContainers and never closed them:
     - TopN created a dummy VectorContainer for no reason and never closed it, 
so I removed it.
     - HashTableTemplate created a dummy template VectorContainer and never 
closed it, so I close it now. We should actually probably avoid using the dummy 
VectorContainer entirely and just use a schema, but I would rather do that in a 
separate change.
   

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to