> On May 12, 2015, 9:13 a.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java, 
> > line 218
> > <https://reviews.apache.org/r/34075/diff/1/?file=956037#file956037line218>
> >
> >     Shouldn't this and above be target.clear() rather than reaching inside?

I go back and forth on it. As I've mentioned before, I'd like to see us move to 
not treating the empty buffer specially. One of the changes there is to have a 
close() that is different from clear(), which does a data.release(1), and then 
sets data to null. I tried that out, and found that some vectors are 
resurrected after being closed, and are used again. So I started going with the 
check for null.

I can change it to clear() if you like. What's your preference?


- Chris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34075/#review83413
-----------------------------------------------------------


On May 12, 2015, 8:22 a.m., Chris Westin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34075/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 8:22 a.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Bugs: DRILL-3033
>     https://issues.apache.org/jira/browse/DRILL-3033
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Fixes some missing buffer retains() and missing vector clears().
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7d85810 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 9d03efb 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 
> 659d99b 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java
>  8e2ce96 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java
>  c233ac5 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java
>  369c0ec 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
>  c3e70f5 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java
>  0430f1b 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatchBuilder.java
>  1187bd6 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
>  5d990f0 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java
>  ca6d83c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
>  8748aaf 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortRecordBatchBuilder.java
>  e559ece 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
>  3159811 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/MSortTemplate.java
>  9b97e1c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java
>  f7786b7 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/WritableBatch.java 
> 308a8bc 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 
> ae5fad5 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java
>  78846dc 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java
>  b615b66 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestValueVector.java
>  2b1dff0 
> 
> Diff: https://reviews.apache.org/r/34075/diff/
> 
> 
> Testing
> -------
> 
> mvn install
> precommit suite
> 
> 
> Thanks,
> 
> Chris Westin
> 
>

Reply via email to