Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3344: Simplify sorter and document/enforce invariants.
......................................................................


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/2826/13/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:

Line 858: if (HasVarLenBlocks() &&
        :           fixed_len_blocks_index_ == fixed_len_blocks_.size() - 1) {
        :         
output_batch->AddBlock(var_len_blocks_[var_len_blocks_index_]);
        :         var_len_blocks_[var_len_blocks_index_] = NULL;
        :       }
> I take it the DCHECK in GetNext() helped find this? We already have a test 
Yeah the DCHECK was hit on one of the existing sorter tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/2826
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c619e81fd1b8ac50e257172c8bce101a112b52a
Gerrit-PatchSet: 13
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to