IMPALA-4997: Fix overflows in Sorter::TupleIterator Various places in Sorter::TupleIterator multiply two int values (Sorter::Run::block_capacity_ and Sorter::TupleIterator::block_index_) and assigned the result to an int64_t value (Sorter::TupleIterator::buffer_start_index_). One such occurrence is in be/src/runtime/sorter.cc#L1080. This multiplication could overflow for runs with a large number of rows. Changing one of the operands to int64_t fixes this.
To test this Matt Mulder ran the failing query from IMPALA-4997 on a private cluster and it succeeded. Change-Id: Iea22aa96e0cc86102b60c6e551e9e607cef485c8 Reviewed-on: http://gerrit.cloudera.org:8080/6169 Reviewed-by: Lars Volker <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/abe860ce Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/abe860ce Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/abe860ce Branch: refs/heads/master Commit: abe860ced09d2bcd87f534e1e4e655f4b17d2aae Parents: 09f32d4 Author: Lars Volker <[email protected]> Authored: Mon Feb 27 13:29:24 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Wed Mar 1 07:41:42 2017 +0000 ---------------------------------------------------------------------- be/src/runtime/sorter.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/abe860ce/be/src/runtime/sorter.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/sorter.cc b/be/src/runtime/sorter.cc index 5f6bf8b..01554b8 100644 --- a/be/src/runtime/sorter.cc +++ b/be/src/runtime/sorter.cc @@ -258,8 +258,10 @@ class Sorter::Run { /// The size in bytes of the sort tuple. const int sort_tuple_size_; - /// Number of tuples per block in a run. - const int block_capacity_; + /// Number of tuples per block in a run. This gets multiplied with + /// TupleIterator::block_index_ in various places and to make sure we don't overflow the + /// result of that operation we make this int64_t here. + const int64_t block_capacity_; const bool has_var_len_slots_;
