wangyong9999 commented on code in PR #314:
URL: https://github.com/apache/paimon-cpp/pull/314#discussion_r4022451549
##########
src/paimon/format/parquet/page_filtered_row_group_reader.cpp:
##########
@@ -100,6 +100,38 @@ std::optional<DataPageLayout> GetDataPageLayout(
return DataPageLayout{column_chunk_offset, first_data_page_offset};
}
+// Call only after GetDataPageLayout has validated the ordered page row
offsets.
+template <typename Visitor>
+void VisitSelectedPages(const RowRanges& row_ranges,
Review Comment:
Removed in c7579e7c. MakeDataPageReadPlan and ComputePageRanges now match
current main; only the per-leaf direct-plan decision is reused. The
page-selection test additions are also removed.
##########
src/paimon/format/parquet/file_reader_wrapper_test.cpp:
##########
@@ -311,6 +312,99 @@ TEST_F(FileReaderWrapperTest,
PredicateReadsOnlyItsPageIndexesAndKeepsPayloadRea
ASSERT_FALSE(end);
}
+TEST_F(FileReaderWrapperTest, ReusesParsedOffsetIndexesWithinFileReader) {
+ std::string file_path = PathUtil::JoinPath(dir_->Str(),
"parsed-index.parquet");
+ PrepareParquetFile(file_path, /*row_count=*/2000,
/*enable_page_index=*/true);
+ ASSERT_OK_AND_ASSIGN(auto reader, PrepareReaderWrapper(file_path));
+ std::weak_ptr<::parquet::OffsetIndex> released_index;
+ {
+ auto first = reader->GetRowGroupPageIndexReader(0);
+ auto second = reader->GetRowGroupPageIndexReader(1);
+ ASSERT_TRUE(first);
+ ASSERT_TRUE(second);
+ ASSERT_NE(first, second);
+ for (int32_t col = 0; col < 3; ++col) {
+ auto offset = first->GetOffsetIndex(col);
+ auto column = first->GetColumnIndex(col);
+ ASSERT_TRUE(offset);
+ ASSERT_TRUE(column);
+ ASSERT_EQ(offset, first->GetOffsetIndex(col));
+ ASSERT_NE(offset, second->GetOffsetIndex(col));
+ released_index = offset;
+ }
+ ASSERT_THROW(first->GetOffsetIndex(-1), ::parquet::ParquetException);
+ ASSERT_THROW(first->GetColumnIndex(100), ::parquet::ParquetException);
+ ASSERT_TRUE(first->GetOffsetIndex(0));
+ }
+ ASSERT_FALSE(released_index.expired());
+ reader.reset();
+ ASSERT_TRUE(released_index.expired());
+}
+
+TEST_F(FileReaderWrapperTest,
ParsedOffsetIndexesRespectRowGroupRetentionLimit) {
Review Comment:
Removed both tests and the fixture parameter added for the retention-limit
test in c7579e7c. ReusesParsedOffsetIndexesWithinFileReader is the sole new
test. The existing Parquet and read integration suites pass.
--
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]
