IMPALA-3923: fix overflow in BufferedTupleStream::GetRows() Testing: Added a targeted unit test.
Change-Id: Iabb977014d855d6edb035bf2e9519ac8b3234767 Reviewed-on: http://gerrit.cloudera.org:8080/3878 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Internal 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/e6702c67 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e6702c67 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e6702c67 Branch: refs/heads/master Commit: e6702c67309cae4b46da6de935bcf873800a2f3c Parents: 16a50f7 Author: Tim Armstrong <[email protected]> Authored: Wed Jul 27 13:50:23 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Thu Aug 11 01:31:37 2016 +0000 ---------------------------------------------------------------------- be/src/runtime/buffered-tuple-stream-test.cc | 18 ++++++++++++++++++ be/src/runtime/buffered-tuple-stream.cc | 5 +++++ be/src/runtime/buffered-tuple-stream.h | 3 ++- 3 files changed, 25 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e6702c67/be/src/runtime/buffered-tuple-stream-test.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream-test.cc b/be/src/runtime/buffered-tuple-stream-test.cc index 7d28e69..558b617 100644 --- a/be/src/runtime/buffered-tuple-stream-test.cc +++ b/be/src/runtime/buffered-tuple-stream-test.cc @@ -750,6 +750,24 @@ TEST_F(SimpleTupleStreamTest, BigRow) { nullable_stream.Close(); } +// Test for IMPALA-3923: overflow of 32-bit int in GetRows(). +TEST_F(SimpleTupleStreamTest, TestGetRowsOverflow) { + InitBlockMgr(-1, 8 * 1024 * 1024); + BufferedTupleStream stream(runtime_state_, *int_desc_, runtime_state_->block_mgr(), + client_, false, false); + ASSERT_OK(stream.Init(-1, NULL, true)); + + Status status; + // Add more rows than can be fit in a RowBatch (limited by its 32-bit row count). + // Actually adding the rows would take a very long time, so just set num_rows_. + // This puts the stream in an inconsistent state, but exercises the right code path. + stream.num_rows_ = 1L << 33; + bool got_rows; + scoped_ptr<RowBatch> overflow_batch; + ASSERT_FALSE(stream.GetRows(&overflow_batch, &got_rows).ok()); + stream.Close(); +} + // Basic API test. No data should be going to disk. TEST_F(SimpleNullStreamTest, Basic) { InitBlockMgr(-1, IO_BLOCK_SIZE); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e6702c67/be/src/runtime/buffered-tuple-stream.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream.cc b/be/src/runtime/buffered-tuple-stream.cc index 263071e..978a221 100644 --- a/be/src/runtime/buffered-tuple-stream.cc +++ b/be/src/runtime/buffered-tuple-stream.cc @@ -503,6 +503,11 @@ int BufferedTupleStream::ComputeNumNullIndicatorBytes(int block_size) const { } Status BufferedTupleStream::GetRows(scoped_ptr<RowBatch>* batch, bool* got_rows) { + if (num_rows() > numeric_limits<int>::max()) { + // RowBatch::num_rows_ is a 32-bit int, avoid an overflow. + return Status(Substitute("Trying to read $0 rows into in-memory batch failed. Limit " + "is $1", num_rows(), numeric_limits<int>::max())); + } RETURN_IF_ERROR(PinStream(false, got_rows)); if (!*got_rows) return Status::OK(); bool got_read_buffer; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e6702c67/be/src/runtime/buffered-tuple-stream.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream.h b/be/src/runtime/buffered-tuple-stream.h index c5d9793..e14f16c 100644 --- a/be/src/runtime/buffered-tuple-stream.h +++ b/be/src/runtime/buffered-tuple-stream.h @@ -317,9 +317,10 @@ class BufferedTupleStream { std::string DebugString() const; private: - friend class MultiNullableTupleStreamTest_TestComputeRowSize_Test; friend class ArrayTupleStreamTest_TestArrayDeepCopy_Test; friend class ArrayTupleStreamTest_TestComputeRowSize_Test; + friend class MultiNullableTupleStreamTest_TestComputeRowSize_Test; + friend class SimpleTupleStreamTest_TestGetRowsOverflow_Test; /// If true, this stream is still using small buffers. bool use_small_buffers_;
