HuaHuaY commented on code in PR #37400:
URL: https://github.com/apache/arrow/pull/37400#discussion_r2562992719
##########
cpp/src/parquet/CMakeLists.txt:
##########
@@ -159,6 +159,7 @@ set(PARQUET_SRCS
arrow/variant_internal.cc
arrow/writer.cc
bloom_filter.cc
+ bloom_filter_writer.cc
Review Comment:
We'd better keep the files sorted by filename.
##########
cpp/src/parquet/page_index_test.cc:
##########
@@ -839,28 +841,38 @@ class PageIndexBuilderTest : public ::testing::Test {
protected:
std::unique_ptr<ColumnIndex> ReadColumnIndex(int row_group, int column) {
- auto location =
page_index_location_.column_index_location[row_group][column];
- if (!location.has_value()) {
+ auto row_group_location = column_index_location_.locations.find(row_group);
+ if (row_group_location == column_index_location_.locations.end()) {
+ return nullptr;
+ }
+ auto column_location = row_group_location->second.find(column);
+ if (column_location == row_group_location->second.end()) {
return nullptr;
}
auto properties = default_reader_properties();
- return ColumnIndex::Make(*schema_.Column(column), buffer_->data() +
location->offset,
- static_cast<uint32_t>(location->length),
properties);
+ return ColumnIndex::Make(
+ *schema_.Column(column), buffer_->data() +
column_location->second.offset,
+ static_cast<uint32_t>(column_location->second.length), properties);
}
std::unique_ptr<OffsetIndex> ReadOffsetIndex(int row_group, int column) {
- auto location =
page_index_location_.offset_index_location[row_group][column];
- if (!location.has_value()) {
+ auto row_group_location = offset_index_location_.locations.find(row_group);
+ if (row_group_location == offset_index_location_.locations.end()) {
+ return nullptr;
+ }
+ auto column_location = row_group_location->second.find(column);
+ if (column_location == row_group_location->second.end()) {
return nullptr;
}
auto properties = default_reader_properties();
- return OffsetIndex::Make(buffer_->data() + location->offset,
- static_cast<uint32_t>(location->length),
properties);
+ return OffsetIndex::Make(buffer_->data() + column_location->second.offset,
+
static_cast<uint32_t>(column_location->second.length),
+ properties);
}
SchemaDescriptor schema_;
std::shared_ptr<Buffer> buffer_;
- PageIndexLocation page_index_location_;
+ IndexLocations column_index_location_, offset_index_location_;
Review Comment:
I think defining only one variable at one line is a better style?
##########
cpp/src/parquet/CMakeLists.txt:
##########
@@ -372,8 +373,8 @@ set_source_files_properties(public_api_test.cc PROPERTIES
SKIP_UNITY_BUILD_INCLU
add_parquet_test(internals-test
SOURCES
- bloom_filter_reader_test.cc
bloom_filter_test.cc
+ bloom_filter_reader_writer_test.cc
Review Comment:
ditto
--
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]
