wgtmac commented on code in PR #36191:
URL: https://github.com/apache/arrow/pull/36191#discussion_r1269117825
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
+
+ protected:
+ const char* GetDataFile() const { return
"stream_reader_multirowgroup_test.parquet"; }
+
+ void SetUp() {
+ CreateTestFile();
+ PARQUET_ASSIGN_OR_THROW(auto infile,
::arrow::io::ReadableFile::Open(GetDataFile()));
+ auto file_reader = parquet::ParquetFileReader::Open(infile);
+ reader_ = StreamReader{std::move(file_reader)};
+ }
+
+ void TearDown() { reader_ = StreamReader{}; }
Review Comment:
```suggestion
void TearDown() override { reader_ = StreamReader{}; }
```
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
+
+ protected:
+ const char* GetDataFile() const { return
"stream_reader_multirowgroup_test.parquet"; }
+
+ void SetUp() {
Review Comment:
BTW, SetUp and TearDown are public by default.
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
+
+ protected:
+ const char* GetDataFile() const { return
"stream_reader_multirowgroup_test.parquet"; }
+
+ void SetUp() {
+ CreateTestFile();
+ PARQUET_ASSIGN_OR_THROW(auto infile,
::arrow::io::ReadableFile::Open(GetDataFile()));
+ auto file_reader = parquet::ParquetFileReader::Open(infile);
+ reader_ = StreamReader{std::move(file_reader)};
+ }
+
+ void TearDown() { reader_ = StreamReader{}; }
+
+ std::shared_ptr<schema::GroupNode> GetSchema() {
+ schema::NodeVector fields;
+ fields.push_back(schema::PrimitiveNode::Make("row_group_number",
Repetition::REQUIRED,
+ Type::INT32,
ConvertedType::UINT_16));
+
+ fields.push_back(schema::PrimitiveNode::Make("row_number",
Repetition::REQUIRED,
+ Type::INT64,
ConvertedType::UINT_64));
+
+ return std::static_pointer_cast<schema::GroupNode>(
+ schema::GroupNode::Make("schema", Repetition::REQUIRED, fields));
+ }
+
+ void CreateTestFile() {
+ PARQUET_ASSIGN_OR_THROW(auto outfile,
+
::arrow::io::FileOutputStream::Open(GetDataFile()));
+
+ auto file_writer = ParquetFileWriter::Open(outfile, GetSchema());
+
+ StreamWriter os{std::move(file_writer)};
+
+ int nrows = 0;
+ for (auto group = 0; group < kNumGroups; ++group) {
+ for (auto i = 0; i < kNumRowsPerGroup; ++i) {
+ os << static_cast<uint16_t>(group);
+ os << static_cast<uint64_t>(nrows);
+ os << EndRow;
+ nrows++;
+ }
+ os.EndRowGroup();
+ }
+ }
+
+ void ReadRowAndAssertPosition(uint64_t expected_row_num) {
+ const uint16_t expected_group_num = ((uint16_t)expected_row_num) /
kNumRowsPerGroup;
Review Comment:
```suggestion
auto expected_group_num = static_cast<uint16_t>(expected_row_num /
kNumRowsPerGroup);
```
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
+
+ protected:
+ const char* GetDataFile() const { return
"stream_reader_multirowgroup_test.parquet"; }
+
+ void SetUp() {
+ CreateTestFile();
+ PARQUET_ASSIGN_OR_THROW(auto infile,
::arrow::io::ReadableFile::Open(GetDataFile()));
+ auto file_reader = parquet::ParquetFileReader::Open(infile);
+ reader_ = StreamReader{std::move(file_reader)};
+ }
+
+ void TearDown() { reader_ = StreamReader{}; }
Review Comment:
It seems TearDown() is not required to implement.
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
Review Comment:
This seems redundant. Could you please remove it?
##########
cpp/src/parquet/stream_reader_test.cc:
##########
@@ -947,5 +947,98 @@ TEST_F(TestReadingDataFiles, ByteArrayDecimal) {
EXPECT_EQ(i, 25);
}
+class TestMultiRowGroupStreamReader : public ::testing::Test {
+ public:
+ TestMultiRowGroupStreamReader() {}
+
+ protected:
+ const char* GetDataFile() const { return
"stream_reader_multirowgroup_test.parquet"; }
+
+ void SetUp() {
Review Comment:
```suggestion
void SetUp() override {
```
--
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]
