Skip to content

Commit 41044f1

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 41044f1

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. Same shape as TestMultipleAppendsDifferentFirstRowIds, with a WriteNextSchema()
847+
// inserted between the two commits so the second f1 blob carries schema_id 1 while the first
848+
// keeps schema_id 0.
849+
std::map<std::string, std::string> options = {{Options::MANIFEST_FORMAT, "orc"},
850+
{Options::FILE_FORMAT, GetParam()},
851+
{Options::FILE_SYSTEM, "local"},
852+
{Options::ROW_TRACKING_ENABLED, "true"},
853+
{Options::DATA_EVOLUTION_ENABLED, "true"}};
854+
CreateTable(/*partition_keys=*/{}, options);
855+
std::string table_path = PathUtil::JoinPath(dir_->Str(), "foo.db/bar");
856+
auto schema = arrow::schema(fields_);
857+
858+
// First commit under schema 0, firstRowId = 0.
859+
std::vector<std::string> write_cols1 = {"f0", "f1"};
860+
auto src_array1 = std::dynamic_pointer_cast<arrow::StructArray>(
861+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[0], fields_[1]}), R"([
862+
[1, "a"]
863+
])")
864+
.ValueOrDie());
865+
ASSERT_OK_AND_ASSIGN(auto commit_msgs1, WriteArray(table_path, {}, write_cols1, {src_array1}));
866+
SetFirstRowId(0, commit_msgs1);
867+
868+
std::vector<std::string> write_cols2 = {"f2"};
869+
auto src_array2 = std::dynamic_pointer_cast<arrow::StructArray>(
870+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[2]}), R"([
871+
["b"]
872+
])")
873+
.ValueOrDie());
874+
ASSERT_OK_AND_ASSIGN(auto commit_msgs2, WriteArray(table_path, {}, write_cols2, {src_array2}));
875+
SetFirstRowId(0, commit_msgs2);
876+
877+
std::vector<std::shared_ptr<CommitMessage>> total_msgs;
878+
total_msgs.insert(total_msgs.end(), commit_msgs1.begin(), commit_msgs1.end());
879+
total_msgs.insert(total_msgs.end(), commit_msgs2.begin(), commit_msgs2.end());
880+
ASSERT_OK(Commit(table_path, total_msgs));
881+
882+
// ALTER TABLE ADD COLUMN f3 -> schema 1. Files written afterwards carry schema_id 1.
883+
ASSERT_OK(WriteNextSchema(
884+
table_path,
885+
{DataField(0, fields_[0]), DataField(1, fields_[1]), DataField(2, fields_[2]),
886+
DataField(3, arrow::field("f3", arrow::utf8()))},
887+
/*highest_field_id=*/3, options));
888+
889+
// Second commit under schema 1, firstRowId = 1. The f1 blob here has schema_id 1; it is
890+
// contiguous with the schema_id 0 f1 blob above, so on read both land in one blob bunch.
891+
std::vector<std::string> write_cols3 = {"f0", "f1"};
892+
auto src_array3 = std::dynamic_pointer_cast<arrow::StructArray>(
893+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[0], fields_[1]}), R"([
894+
[2, "c"]
895+
])")
896+
.ValueOrDie());
897+
ASSERT_OK_AND_ASSIGN(auto commit_msgs3, WriteArray(table_path, {}, write_cols3, {src_array3}));
898+
SetFirstRowId(1, commit_msgs3);
899+
ASSERT_OK(Commit(table_path, commit_msgs3));
900+
901+
std::vector<std::string> write_cols4 = {"f2"};
902+
auto src_array4 = std::dynamic_pointer_cast<arrow::StructArray>(
903+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[2]}), R"([
904+
["d"]
905+
])")
906+
.ValueOrDie());
907+
ASSERT_OK_AND_ASSIGN(auto commit_msgs4, WriteArray(table_path, {}, write_cols4, {src_array4}));
908+
SetFirstRowId(1, commit_msgs4);
909+
ASSERT_OK(Commit(table_path, commit_msgs4));
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)