pitrou commented on code in PR #43064:
URL: https://github.com/apache/arrow/pull/43064#discussion_r1654778586
##########
cpp/src/arrow/io/buffered_test.cc:
##########
@@ -329,7 +329,8 @@ class TestBufferedInputStream : public
FileTestFixture<BufferedInputStream> {
local_pool_ = MemoryPool::CreateDefault();
}
- void MakeExample1(int64_t buffer_size, MemoryPool* pool =
default_memory_pool()) {
+ void MakeExample1(int64_t buffer_size, MemoryPool* pool =
default_memory_pool(),
+ bool fill_raw_read_bound = false) {
Review Comment:
We should instead pass the raw_read_bound explicitly, to allow setting it to
a smaller value than the file size.
##########
cpp/src/arrow/io/buffered_test.cc:
##########
@@ -472,6 +480,24 @@ TEST_F(TestBufferedInputStream, SetBufferSize) {
ASSERT_OK(buffered_->SetBufferSize(5));
}
+// GH-43060: Internal buffer should not greater than the
+// bytes could buffer.
+TEST_F(TestBufferedInputStream, BufferSizeLimit) {
+ {
+ MakeExample1(/*buffer_size=*/100000, default_memory_pool(),
+ /*fill_raw_read_bound=*/true);
+ // buffer_sizes should be limited to the size of the data
+ EXPECT_EQ(test_data_.size(), buffered_->buffer_size());
+ }
+
+ {
+ MakeExample1(/*buffer_size=*/10, default_memory_pool(),
/*fill_raw_read_bound=*/true);
+ ASSERT_OK(buffered_->Read(10));
+ ASSERT_OK(buffered_->SetBufferSize(/*new_buffer_size=*/100000));
+ EXPECT_EQ(test_data_.size() - 10, buffered_->buffer_size());
+ }
Review Comment:
Ideally this would be something like:
```suggestion
{
MakeExample1(/*buffer_size=*/10, default_memory_pool(),
/*raw_read_bound=*/15;
ASSERT_OK(buffered_->Read(10));
ASSERT_OK(buffered_->SetBufferSize(/*new_buffer_size=*/100000));
EXPECT_EQ(5, buffered_->buffer_size());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
