Skip to content

Commit d76c2a6

Browse files
authored
fix(compaction): persist keys in lookup SST files while first-row merge function (#250)
1 parent c329090 commit d76c2a6

2 files changed

Lines changed: 41 additions & 12 deletions

File tree

src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter_test.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,45 @@ TEST_F(LookupMergeTreeCompactRewriterTest, TestFirstRowRewrite) {
484484
CheckResult(compact_file_name, table_schema, "orc", expected_array);
485485
}
486486

487+
TEST_F(LookupMergeTreeCompactRewriterTest, TestFirstRowLooksUpExistingKeys) {
488+
std::map<std::string, std::string> options = {{Options::MERGE_ENGINE, "first-row"},
489+
{Options::FILE_FORMAT, "orc"}};
490+
ASSERT_OK_AND_ASSIGN(CoreOptions core_options, CoreOptions::FromMap(options));
491+
ASSERT_OK_AND_ASSIGN(auto table_path, CreateTable(options));
492+
auto schema_manager = std::make_shared<SchemaManager>(fs_, table_path);
493+
ASSERT_OK_AND_ASSIGN(auto table_schema, schema_manager->ReadSchema(0));
494+
495+
ASSERT_OK_AND_ASSIGN(auto level0_file,
496+
NewFiles(/*level=*/0, /*last_sequence_number=*/0, table_path, core_options,
497+
"[[1, 111], [2, 22]]"));
498+
ASSERT_OK_AND_ASSIGN(auto high_level_file, NewFiles(/*level=*/2, /*last_sequence_number=*/-1,
499+
table_path, core_options, "[[1, 11]]"));
500+
auto processor_factory = std::make_shared<PersistEmptyProcessor::Factory>();
501+
ASSERT_OK_AND_ASSIGN(auto lookup_levels,
502+
CreateLookupLevels<bool>(table_path, table_schema, processor_factory,
503+
std::vector<std::shared_ptr<DataFileMeta>>{
504+
level0_file, high_level_file}));
505+
ASSERT_OK_AND_ASSIGN(auto rewriter,
506+
CreateCompactRewriterForFirstRow(table_path, table_schema, core_options,
507+
std::move(lookup_levels)));
508+
ASSERT_OK_AND_ASSIGN(
509+
auto runs, GenerateSortedRuns(std::vector<std::shared_ptr<DataFileMeta>>{level0_file}));
510+
ASSERT_OK_AND_ASSIGN(auto compact_result, rewriter->Rewrite(
511+
/*output_level=*/1, /*drop_delete=*/true, runs));
512+
513+
ASSERT_EQ(1, compact_result.After().size());
514+
ASSERT_EQ(1, compact_result.After()[0]->row_count);
515+
516+
auto type_with_special_fields =
517+
arrow::struct_(SpecialFields::CompleteSequenceAndValueKindField(arrow_schema_)->fields());
518+
std::shared_ptr<arrow::ChunkedArray> expected;
519+
ASSERT_TRUE(arrow::ipc::internal::json::ChunkedArrayFromJSON(type_with_special_fields,
520+
{"[[2, 0, 2, 22]]"}, &expected)
521+
.ok());
522+
CheckResult(table_path + "/bucket-0/" + compact_result.After()[0]->file_name, table_schema,
523+
"orc", expected);
524+
}
525+
487526
TEST_F(LookupMergeTreeCompactRewriterTest, TestFirstRowUpgrade) {
488527
std::map<std::string, std::string> options = {{Options::MERGE_ENGINE, "first-row"},
489528
{Options::FILE_FORMAT, "orc"}};

src/paimon/core/mergetree/lookup_levels.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ LookupLevels<T>::LookupLevels(
162162
lookup_store_factory_(lookup_store_factory),
163163
lookup_file_cache_(lookup_file_cache),
164164
remote_lookup_file_manager_(remote_lookup_file_manager) {
165-
if constexpr (std::is_same_v<T, FilePosition>) {
166-
// if T is FilePosition, only read key fields to create sst file is enough
165+
if constexpr (std::is_same_v<T, FilePosition> || std::is_same_v<T, bool>) {
166+
// FilePosition and first-row lookup do not persist values, so reading key fields is enough.
167167
value_schema_ = key_schema_;
168168
} else {
169169
value_schema_ = DataField::ConvertDataFieldsToArrowSchema(table_schema->Fields());
@@ -332,16 +332,6 @@ std::optional<std::string> LookupLevels<T>::TryToDownloadRemoteSst(
332332
template <typename T>
333333
Status LookupLevels<T>::CreateSstFileFromDataFile(const std::shared_ptr<DataFileMeta>& file,
334334
const std::string& kv_file_path) {
335-
if constexpr (std::is_same_v<T, bool>) {
336-
// Short-circuit logic: if T is bool, just write empty lookup file.
337-
PAIMON_ASSIGN_OR_RAISE(
338-
std::shared_ptr<BloomFilter> bloom_filter,
339-
LookupStoreFactory::BfGenerator(file->row_count, options_, pool_.get()));
340-
PAIMON_ASSIGN_OR_RAISE(
341-
std::unique_ptr<LookupStoreWriter> kv_writer,
342-
lookup_store_factory_->CreateWriter(fs_, kv_file_path, bloom_filter, pool_));
343-
return kv_writer->Close();
344-
}
345335
// Prepare reader to iterate KeyValue
346336
PAIMON_ASSIGN_OR_RAISE(
347337
std::vector<std::unique_ptr<FileBatchReader>> raw_readers,

0 commit comments

Comments
 (0)