Skip to content

Commit 7ebab5a

Browse files
committed
fix(blob): allow blob files with different schema ids in a bunch
BlobBunch::Add required every file in a bunch to share the same schema id. After schema evolution that adds/drops OTHER columns, a single blob field's contiguous files can carry different schema ids (an old appended file plus a compacted one). MergeRangesAndSort groups them into one bunch by row id, so the check raised a spurious "All files in a blob bunch should have the same schema id." A blob column's on-disk layout is schema-independent and the bunch is read with the first file's schema regardless, so blob files may span schema ids. BlobBunch only ever holds blob files (non-blob files are rejected up front), so the check is removed. This matches Paimon Java, whose SpecialFieldBunch.add guards the same check with if (!isBlobFile(file)). Adds a unit test (TestAddBlobFilesWithDifferentSchemaId) and an integration test (TestBlobBunchSpanningSchemaIds, which uses the existing WriteNextSchema helper to simulate an ALTER TABLE between two appends of the same blob field).
1 parent ffbda4f commit 7ebab5a

3 files changed

Lines changed: 104 additions & 5 deletions

File tree

src/paimon/core/operation/data_evolution_split_read.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,7 @@ Status DataEvolutionSplitRead::BlobBunch::Add(const std::shared_ptr<DataFileMeta
9494
}
9595
}
9696
if (!files_.empty()) {
97-
if (file->schema_id != files_[0]->schema_id) {
98-
return Status::Invalid("All files in a blob bunch should have the same schema id.");
99-
}
97+
// Paimon Java does not check the schema id for blob files in a bunch.
10098
if (file->write_cols != files_[0]->write_cols) {
10199
return Status::Invalid(
102100
"All files in a blob bunch should have the same write columns.");

src/paimon/core/operation/data_evolution_split_read_test.cpp

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ class DataEvolutionSplitReadTest : public ::testing::Test {
8484
std::shared_ptr<DataFileMeta> CreateBlobFile(
8585
const std::string& file_name, int64_t first_row_id, int64_t row_count,
8686
int64_t max_sequence_number,
87-
const std::optional<std::vector<std::string>>& write_cols) const {
87+
const std::optional<std::vector<std::string>>& write_cols, int64_t schema_id = 0) const {
8888
return DataFileMeta::ForAppend(file_name + ".blob", /*file_size=*/row_count,
8989
/*row_count=*/row_count,
9090
/*row_stats=*/SimpleStats::EmptyStats(),
9191
/*min_sequence_number=*/0, max_sequence_number,
92-
/*schema_id=*/0, FileSource::Append(),
92+
schema_id, FileSource::Append(),
9393
/*value_stats_cols=*/std::nullopt,
9494
/*external_path=*/std::nullopt, first_row_id, write_cols)
9595
.value();
@@ -247,6 +247,28 @@ TEST_F(DataEvolutionSplitReadTest, TestAddBlobFileWithDifferentWriteCols) {
247247
"All files in a blob bunch should have the same write columns.");
248248
}
249249

250+
TEST_F(DataEvolutionSplitReadTest, TestAddBlobFilesWithDifferentSchemaId) {
251+
// A blob field written under schema 0 for the first rows and under schema 1 for the
252+
// following rows (after schema evolution that adds/drops OTHER columns) must still be
253+
// groupable into a single bunch: a blob column's on-disk layout is schema-independent.
254+
// Same write cols, contiguous row ids, different schema id -> both added OK.
255+
auto blob_entry =
256+
CreateBlobFile("blob1", /*first_row_id=*/0, /*row_count=*/100,
257+
/*max_sequence_number=*/1,
258+
/*write_cols=*/std::optional<std::vector<std::string>>({"blob_col"}),
259+
/*schema_id=*/0);
260+
auto blob_tail =
261+
CreateBlobFile("blob2", /*first_row_id=*/100, /*row_count=*/200,
262+
/*max_sequence_number=*/1,
263+
/*write_cols=*/std::optional<std::vector<std::string>>({"blob_col"}),
264+
/*schema_id=*/1);
265+
auto blob_bunch = std::make_shared<DataEvolutionSplitRead::BlobBunch>(
266+
INT64_MAX, /*has_row_ids_selection=*/false);
267+
ASSERT_OK(blob_bunch->Add(blob_entry));
268+
ASSERT_OK(blob_bunch->Add(blob_tail));
269+
ASSERT_EQ(blob_bunch->Files().size(), 2);
270+
}
271+
250272
TEST_F(DataEvolutionSplitReadTest, TestRowIdSelectionWithOverlap) {
251273
auto blob_entry =
252274
CreateBlobFile("blob1", /*first_row_id=*/0, /*row_count=*/10,

test/inte/blob_table_inte_test.cpp

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,85 @@ TEST_P(BlobTableInteTest, TestMultipleAppendsDifferentFirstRowIds) {
840840
}
841841
}
842842

843+
TEST_P(BlobTableInteTest, TestBlobBunchSpanningSchemaIds) {
844+
// Regression test: a blob field whose contiguous files were written under different schema
845+
// ids (an ALTER TABLE happens between two appends) must still be groupable into a single
846+
// blob bunch.
847+
//
848+
// BlobBunch::Add only reaches the per-bunch schema/write-cols check when a second blob file is
849+
// *strictly contiguous* with the previous one (first_row_id == expected_next_first_row_id_);
850+
// the same-first-row-id and overlapping cases return early. Two 1-row f1 blobs at [0,0] and
851+
// [1,1] are contiguous, but their ranges do not overlap, so on their own MergeOverlappingRanges
852+
// would split them into different need_merge_files groups and each would form a singleton bunch
853+
// (never hitting the check). We therefore write the non-blob columns f0/f2 as one file spanning
854+
// both rows [0,1]; that file overlaps both f1 blobs and bridges them into a single group, where
855+
// the two contiguous f1 blobs (schema 0 then schema 1) are added to the same bunch.
856+
std::map<std::string, std::string> options = {{Options::MANIFEST_FORMAT, "orc"},
857+
{Options::FILE_FORMAT, GetParam()},
858+
{Options::FILE_SYSTEM, "local"},
859+
{Options::ROW_TRACKING_ENABLED, "true"},
860+
{Options::DATA_EVOLUTION_ENABLED, "true"}};
861+
CreateTable(/*partition_keys=*/{}, options);
862+
std::string table_path = PathUtil::JoinPath(dir_->Str(), "foo.db/bar");
863+
auto schema = arrow::schema(fields_);
864+
865+
// First commit under schema 0. The non-blob columns f0/f2 are written as one file spanning
866+
// both rows [0,1] -- this is the file that bridges the two f1 blobs into one group. The f1
867+
// blob covers only row 0 -> [0,0], schema_id 0.
868+
std::vector<std::string> write_cols_base = {"f0", "f2"};
869+
auto base_array = std::dynamic_pointer_cast<arrow::StructArray>(
870+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[0], fields_[2]}), R"([
871+
[1, "b"],
872+
[2, "d"]
873+
])")
874+
.ValueOrDie());
875+
ASSERT_OK_AND_ASSIGN(auto commit_msgs_base, WriteArray(table_path, {}, write_cols_base, {base_array}));
876+
SetFirstRowId(0, commit_msgs_base);
877+
878+
std::vector<std::string> write_cols_f1a = {"f1"};
879+
auto f1a_array = std::dynamic_pointer_cast<arrow::StructArray>(
880+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[1]}), R"([
881+
["a"]
882+
])")
883+
.ValueOrDie());
884+
ASSERT_OK_AND_ASSIGN(auto commit_msgs_f1a, WriteArray(table_path, {}, write_cols_f1a, {f1a_array}));
885+
SetFirstRowId(0, commit_msgs_f1a);
886+
887+
std::vector<std::shared_ptr<CommitMessage>> total_msgs;
888+
total_msgs.insert(total_msgs.end(), commit_msgs_base.begin(), commit_msgs_base.end());
889+
total_msgs.insert(total_msgs.end(), commit_msgs_f1a.begin(), commit_msgs_f1a.end());
890+
ASSERT_OK(Commit(table_path, total_msgs));
891+
892+
// ALTER TABLE ADD COLUMN f3 -> schema 1. Files written afterwards carry schema_id 1.
893+
ASSERT_OK(WriteNextSchema(
894+
table_path,
895+
{DataField(0, fields_[0]), DataField(1, fields_[1]), DataField(2, fields_[2]),
896+
DataField(3, arrow::field("f3", arrow::utf8()))},
897+
/*highest_field_id=*/3, options));
898+
899+
// Second commit under schema 1: the f1 blob for row 1 -> [1,1], schema_id 1. It is strictly
900+
// contiguous with the schema_id 0 f1 blob above ([0,0]), so both land in the same blob bunch.
901+
std::vector<std::string> write_cols_f1b = {"f1"};
902+
auto f1b_array = std::dynamic_pointer_cast<arrow::StructArray>(
903+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[1]}), R"([
904+
["c"]
905+
])")
906+
.ValueOrDie());
907+
ASSERT_OK_AND_ASSIGN(auto commit_msgs_f1b, WriteArray(table_path, {}, write_cols_f1b, {f1b_array}));
908+
SetFirstRowId(1, commit_msgs_f1b);
909+
ASSERT_OK(Commit(table_path, commit_msgs_f1b));
910+
911+
// Without the fix the read fails with
912+
// "All files in a blob bunch should have the same schema id."; with it both rows read.
913+
auto expected_array = std::dynamic_pointer_cast<arrow::StructArray>(
914+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields_), R"([
915+
[1, "a", "b"],
916+
[2, "c", "d"]
917+
])")
918+
.ValueOrDie());
919+
ASSERT_OK(ScanAndRead(table_path, schema->field_names(), expected_array));
920+
}
921+
843922
TEST_P(BlobTableInteTest, TestMoreDataWithDataEvolution) {
844923
CreateTable();
845924
std::string table_path = PathUtil::JoinPath(dir_->Str(), "foo.db/bar");

0 commit comments

Comments
 (0)