You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Steven Phillips <sp...@maprtech.com> on 2014/03/06 12:59:54 UTC
Re: Review Request 18499: DRILL-385 Implement Top-N sort operator
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18499/
-----------------------------------------------------------
(Updated March 6, 2014, 11:59 a.m.)
Review request for drill.
Changes
-------
addressed code review comments and rebased off latest master.
Repository: drill-git
Description
-------
See https://issues.apache.org/jira/browse/DRILL-385
Diffs (updated)
-----
distribution/src/assemble/bin.xml 4e37c6a
distribution/src/resources/drill-override.conf 5fe2362
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java bcc113f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Sort.java 82aa830
exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/TopN.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueue.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java f9835f6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java 2a3a2e2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 78326ca
exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/record/HyperVectorWrapper.java 44b935b
exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java 7c6b105
exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java 6e764a8
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector4.java ac9dcda
exec/java-exec/src/main/resources/drill-module.conf 4dd08c1
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java PRE-CREATION
exec/java-exec/src/test/resources/topN/one_key_sort.json PRE-CREATION
exec/java-exec/src/test/resources/topN/two_key_sort.json PRE-CREATION
Diff: https://reviews.apache.org/r/18499/diff/
Testing
-------
Thanks,
Steven Phillips
Re: Review Request 18499: DRILL-385 Implement Top-N sort operator
Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18499/
-----------------------------------------------------------
(Updated March 24, 2014, 7:47 p.m.)
Review request for drill.
Repository: drill-git
Description
-------
See https://issues.apache.org/jira/browse/DRILL-385
Diffs (updated)
-----
distribution/src/assemble/bin.xml 4e37c6a
distribution/src/resources/drill-override.conf 5fe2362
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java bcc113f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Sort.java 82aa830
exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/TopN.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueue.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java f9835f6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java 2a3a2e2
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 78326ca
exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/record/HyperVectorWrapper.java 44b935b
exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java 7c6b105
exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java 6e764a8
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector4.java ac9dcda
exec/java-exec/src/main/resources/drill-module.conf a929a69
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java PRE-CREATION
exec/java-exec/src/test/resources/topN/one_key_sort.json PRE-CREATION
exec/java-exec/src/test/resources/topN/two_key_sort.json PRE-CREATION
Diff: https://reviews.apache.org/r/18499/diff/
Testing
-------
Thanks,
Steven Phillips
Re: Review Request 18499: DRILL-385 Implement Top-N sort operator
Posted by Steven Phillips <sp...@maprtech.com>.
> On March 7, 2014, 3:01 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java, line 35
> > <https://reviews.apache.org/r/18499/diff/2/?file=512233#file512233line35>
> >
> > Initialize..
This is fixed in the ExternalSort patch.
- Steven
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18499/#review36480
-----------------------------------------------------------
On March 6, 2014, 11:59 a.m., Steven Phillips wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18499/
> -----------------------------------------------------------
>
> (Updated March 6, 2014, 11:59 a.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> See https://issues.apache.org/jira/browse/DRILL-385
>
>
> Diffs
> -----
>
> distribution/src/assemble/bin.xml 4e37c6a
> distribution/src/resources/drill-override.conf 5fe2362
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java bcc113f
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Sort.java 82aa830
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/TopN.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueue.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java f9835f6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java 2a3a2e2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 78326ca
> exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/record/HyperVectorWrapper.java 44b935b
> exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java 7c6b105
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java 6e764a8
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector4.java ac9dcda
> exec/java-exec/src/main/resources/drill-module.conf 4dd08c1
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java PRE-CREATION
> exec/java-exec/src/test/resources/topN/one_key_sort.json PRE-CREATION
> exec/java-exec/src/test/resources/topN/two_key_sort.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18499/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Steven Phillips
>
>
Re: Review Request 18499: DRILL-385 Implement Top-N sort operator
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18499/#review36480
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java
<https://reviews.apache.org/r/18499/#comment67470>
I am wondering if p should be initialized to (queueSize - 1) since the compare() and swap functions are accessing p which may be out-of-bound. I am not completely sure about this though...since you would probably have hit an error while testing.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java
<https://reviews.apache.org/r/18499/#comment67465>
Initialize..
- Aman Sinha
On March 6, 2014, 11:59 a.m., Steven Phillips wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18499/
> -----------------------------------------------------------
>
> (Updated March 6, 2014, 11:59 a.m.)
>
>
> Review request for drill.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> See https://issues.apache.org/jira/browse/DRILL-385
>
>
> Diffs
> -----
>
> distribution/src/assemble/bin.xml 4e37c6a
> distribution/src/resources/drill-override.conf 5fe2362
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java bcc113f
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Sort.java 82aa830
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/TopN.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueue.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/PriorityQueueTemplate.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortTemplate.java f9835f6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java 2a3a2e2
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 78326ca
> exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/record/HyperVectorWrapper.java 44b935b
> exec/java-exec/src/main/java/org/apache/drill/exec/record/SchemaBuilder.java 7c6b105
> exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java 6e764a8
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector4.java ac9dcda
> exec/java-exec/src/main/resources/drill-module.conf 4dd08c1
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java PRE-CREATION
> exec/java-exec/src/test/resources/topN/one_key_sort.json PRE-CREATION
> exec/java-exec/src/test/resources/topN/two_key_sort.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18499/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Steven Phillips
>
>