zjw1111 commented on code in PR #185:
URL: https://github.com/apache/paimon-cpp/pull/185#discussion_r3719542461
##########
test/inte/write_and_read_inte_test.cpp:
##########
@@ -1529,6 +1529,94 @@ TEST_P(WriteAndReadInteTest,
TestAppendWithParquetPageIndexFilter) {
ASSERT_TRUE(expected->Equals(read_result)) << read_result->ToString();
}
+/// Reproduces the prefetch + parquet page-index filter failure: the predicate
keeps only the last
+/// page of RG1, RG2 and RG3, so the prefetch reader ends up seeking to a row
in the middle of a row
+/// group, which FileReaderWrapper::SeekToRow rejects.
+TEST_P(WriteAndReadInteTest, TestAppendWithParquetPageIndexFilterAndPrefetch) {
+ auto [file_format, file_system] = GetParam();
+ if (file_format != "parquet" || file_system != "local") {
+ return;
+ }
+
+ auto test_dir = UniqueTestDirectory::Create("local");
+ arrow::FieldVector fields = {arrow::field("f0", arrow::int32()),
+ arrow::field("f1", arrow::utf8())};
+ auto schema = arrow::schema(fields);
+ std::map<std::string, std::string> options = {
+ {Options::MANIFEST_FORMAT, "orc"},
+ {Options::FILE_FORMAT, "parquet"},
+ {Options::TARGET_FILE_SIZE, "1048576"},
+ {Options::BUCKET, "-1"},
+ {Options::FILE_SYSTEM, "local"},
+ // One row per page (see TestAppendWithParquetPageIndexFilter for why
these three
+ // options are needed together) and 4 rows per row group, so the 16
rows below end up
+ // in 4 row groups of 4 single-row pages.
+ {Options::WRITE_BATCH_SIZE, "1"},
+ {"parquet.page.size", "1"},
+ {"parquet.enable-dictionary", "false"},
+ {"parquet.write.enable-page-index", "true"},
+ {"parquet.write.max-row-group-length", "4"},
+ {"parquet.read.enable-page-index-filter", "true"},
+ };
+ ASSERT_OK_AND_ASSIGN(
+ auto helper, TestHelper::Create(test_dir->Str(), schema,
/*partition_keys=*/{},
+ /*primary_keys=*/{}, options,
/*is_streaming_mode=*/true));
+ std::string table_path = test_dir->Str() + "/foo.db/bar";
+
+ std::string data = R"([
+ [0, "v0"], [1, "v1"], [2, "v2"], [3, "v3"],
+ [4, "v4"], [5, "v5"], [6, "v6"], [7, "v7"],
+ [8, "v8"], [9, "v9"], [10, "v10"], [11, "v11"],
+ [12, "v12"], [13, "v13"], [14, "v14"], [15, "v15"]
+ ])";
+ ASSERT_OK_AND_ASSIGN(std::unique_ptr<RecordBatch> batch,
+ TestHelper::MakeRecordBatch(arrow::struct_(fields),
data,
+ /*partition_map=*/{},
/*bucket=*/0, {}));
+ ASSERT_OK(helper->WriteAndCommit(std::move(batch), /*commit_identifier=*/0,
+
/*expected_commit_messages=*/std::nullopt));
+
+ // Keep only the last row of RG1, RG2 and RG3, so each row group is
partially matched and its
+ // first selected row is 3 rows behind the row group start.
+ auto predicate = PredicateBuilder::In(/*field_index=*/0,
/*field_name=*/"f0", FieldType::INT,
+ {Literal(7), Literal(11),
Literal(15)});
+ ASSERT_TRUE(predicate);
+
+ ScanContextBuilder scan_context_builder(table_path);
+ scan_context_builder.SetOptions(options)
+ .AddOption(Options::SCAN_MODE, StartupMode::LatestFull().ToString())
+ .SetPredicate(predicate);
+ ASSERT_OK_AND_ASSIGN(auto scan_context, scan_context_builder.Finish());
+ ASSERT_OK_AND_ASSIGN(auto table_scan,
TableScan::Create(std::move(scan_context)));
+ ASSERT_OK_AND_ASSIGN(auto result_plan, table_scan->CreatePlan());
+ ASSERT_FALSE(result_plan->Splits().empty());
+
+ // A single prefetch sub reader owns every read range, so it is the one
that has to cross
Review Comment:
This comment is now stale after raising `SetPrefetchMaxParallelNum` to 2:
the read ranges are no longer owned by a single sub reader — they are
dispatched across the two readers. Would it be possible to update the wording,
e.g. "The read ranges are dispatched across 2 sub readers; a reader still has
to cross row group boundaries because its assigned ranges are non-contiguous"?
Purely a comment/code consistency nit, no behavior impact.
--
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]
