> On March 2, 2014, 7:35 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java,
> >  line 144
> > <https://reviews.apache.org/r/18501/diff/1/?file=504048#file504048line144>
> >
> >     The logic here is a bit confusing since the previous statement checks 
> > if adding 1 exceeds queueSize; if that condition is true then adding 2 will 
> > also exceed queueSize. You might want to add some comments to clarify the 
> > logic..

I added comments in the code to explain logic.


> On March 2, 2014, 7:35 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortRecordBatchBuilder.java,
> >  line 99
> > <https://reviews.apache.org/r/18501/diff/1/?file=504038#file504038line99>
> >
> >     Would be good to make these exceptions more descriptive of the 
> > underlying problem.

I actually had the exceptions in there to try to track down a bug. I have 
removed them now.


> On March 2, 2014, 7:35 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java,
> >  line 81
> > <https://reviews.apache.org/r/18501/diff/1/?file=504048#file504048line81>
> >
> >     Should this do the unsigned right shift ?

yes. fixed.


- Steven


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


On Feb. 26, 2014, 1:35 a.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18501/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2014, 1:35 a.m.)
> 
> 
> Review request for drill.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> See https://issues.apache.org/jira/browse/DRILL-386
> 
> 
> Diffs
> -----
> 
>   distribution/src/resources/runbit 85daf87 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java c357dd6 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java 051c62d 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java 127c6fd 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 
> 57927a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 
> d957457 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/client/QuerySubmitter.java 
> 7adefdb 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
>  93f1992 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/XSort.java 
> PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java
>  89edaf3 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java
>  9fd081a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java
>  8e4c9d9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/RecordBatchData.java
>  e01bf37 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java
>  4d04735 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortRecordBatchBuilder.java
>  d8bbc6a 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java
>  1c03467 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java
>  d3946a4 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java
>  c9a73f9 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/BatchGroup.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/MSortTemplate.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/MSorter.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopier.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueSelector.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueSelectorTemplate.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/SingleBatchSorter.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/SingleBatchSorterTemplate.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java
>  91eb3cb 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
>  b0d6ab4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 1be5392 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java
>  39821e3 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java
>  f73ee84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/util/Utilities.java 
> PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 
> cbc8b86 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueVector.java 
> 4e5364a 
>   exec/java-exec/src/main/resources/drill-module.conf 67622d0 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java
>  b79ccd0 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/xsort/TestSimpleExternalSort.java
>  PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/util/BatchPrinter.java 
> aa68752 
>   exec/java-exec/src/test/resources/external-sort.conf PRE-CREATION 
>   exec/java-exec/src/test/resources/sort/one_key_sort.json baabcb3 
>   exec/java-exec/src/test/resources/xsort/one_key_sort_descending.json 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18501/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Reply via email to