IMPALA-7777: Fix crash due to arithmetic overflows in Exchange Node Fixes an arithmetic overflow in ExchangeNode::GetNextMerging. Prior to this patch, the code read:
int rows_to_keep = num_rows_skipped_ - offset_; Where num_rows_skipped_ and offset_ were of type int64_t. The result was cast to an int which can lead to an overflow if the result exceeds the value of 2^31. The value of rows_to_keep would be passed into row-batch.h::CopyRows which would crash due to a DCHECK_LE error. This crash arises when the value of the OFFSET is a large number, for example, the query: select int_col from functional.alltypes order by 1 limit 1 offset 9223372036854775800; Would crash the Impalad executor for this query. The fix is to change rows_to_keep to an int64_t to avoid the overflow, which prevents the DCHECK_LE from failing. Change-Id: I8bb8064aae6ad25c8a19f6a8869086be7e70400a Reviewed-on: http://gerrit.cloudera.org:8080/11844 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/c73530d1 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/c73530d1 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/c73530d1 Branch: refs/heads/branch-3.1.0 Commit: c73530d1b1ad566f34f68b677c8423fb0abafb27 Parents: e5d0757 Author: stakiar <[email protected]> Authored: Wed Oct 31 12:06:08 2018 -0500 Committer: Zoltan Borok-Nagy <[email protected]> Committed: Tue Nov 13 12:50:23 2018 +0100 ---------------------------------------------------------------------- be/src/exec/exchange-node.cc | 2 +- be/src/runtime/row-batch.h | 2 +- .../workloads/functional-query/queries/QueryTest/top-n.test | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/c73530d1/be/src/exec/exchange-node.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/exchange-node.cc b/be/src/exec/exchange-node.cc index 26579c1..ca8b973 100644 --- a/be/src/exec/exchange-node.cc +++ b/be/src/exec/exchange-node.cc @@ -221,7 +221,7 @@ Status ExchangeNode::GetNextMerging(RuntimeState* state, RowBatch* output_batch, while (num_rows_skipped_ < offset_) { num_rows_skipped_ += output_batch->num_rows(); // Throw away rows in the output batch until the offset is skipped. - int rows_to_keep = num_rows_skipped_ - offset_; + int64_t rows_to_keep = num_rows_skipped_ - offset_; if (rows_to_keep > 0) { output_batch->CopyRows(0, output_batch->num_rows() - rows_to_keep, rows_to_keep); output_batch->set_num_rows(rows_to_keep); http://git-wip-us.apache.org/repos/asf/impala/blob/c73530d1/be/src/runtime/row-batch.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/row-batch.h b/be/src/runtime/row-batch.h index 1334858..dd737d7 100644 --- a/be/src/runtime/row-batch.h +++ b/be/src/runtime/row-batch.h @@ -330,7 +330,7 @@ class RowBatch { /// Copy 'num_rows' rows from 'src' to 'dest' within the batch. Useful for exec /// nodes that skip an offset and copied more than necessary. - void CopyRows(int dest, int src, int num_rows) { + void CopyRows(int64_t dest, int64_t src, int64_t num_rows) { DCHECK_LE(dest, src); DCHECK_LE(src + num_rows, capacity_); memmove(tuple_ptrs_ + num_tuples_per_row_ * dest, http://git-wip-us.apache.org/repos/asf/impala/blob/c73530d1/testdata/workloads/functional-query/queries/QueryTest/top-n.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/top-n.test b/testdata/workloads/functional-query/queries/QueryTest/top-n.test index 32b1504..37c986c 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/top-n.test +++ b/testdata/workloads/functional-query/queries/QueryTest/top-n.test @@ -1384,3 +1384,11 @@ select cast(string_col as char(20)) from alltypes order by 1 limit 5 ---- TYPES CHAR ==== +---- QUERY +# Test queries with the maximum value for the limit and offset +select string_col from alltypes order by 1 limit 9223372036854775807 +offset 9223372036854775807 +---- RESULTS +---- TYPES +STRING +==== \ No newline at end of file
