Skip to content

Commit 6366ca6

Browse files
authored
fix(blob): support blob-only writes and reject nested blob fields (alibaba#429)
1 parent 2fff022 commit 6366ca6

12 files changed

Lines changed: 275 additions & 89 deletions

src/paimon/common/data/blob_utils.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,12 @@ Result<BlobUtils::SeparatedStructArrays> BlobUtils::SeparateBlobArray(
8282
return Status::Invalid(
8383
"SeparateBlobArray expects at least one non-inline blob field, but got none.");
8484
}
85-
if (main_fields.empty()) {
86-
return Status::Invalid("SeparateBlobArray expects at least one main field, but got none.");
87-
}
8885

8986
SeparatedStructArrays result;
90-
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(result.main_array,
91-
arrow::StructArray::Make(main_arrays, main_fields));
87+
if (!main_fields.empty()) {
88+
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(result.main_array,
89+
arrow::StructArray::Make(main_arrays, main_fields));
90+
}
9291
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(result.blob_array,
9392
arrow::StructArray::Make(blob_arrays, blob_fields));
9493
return result;

src/paimon/common/data/blob_utils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ class PAIMON_EXPORT BlobUtils {
5151
};
5252

5353
struct SeparatedStructArrays {
54-
/// Non-blob fields (includes inline blob fields when inline_fields is provided)
54+
/// Non-blob fields (includes inline blob fields when inline_fields is provided).
55+
/// nullptr when all fields are stored in blob files.
5556
std::shared_ptr<arrow::StructArray> main_array;
5657
/// Blob fields that go to separate .blob files
5758
std::shared_ptr<arrow::StructArray> blob_array;

src/paimon/common/data/blob_utils_test.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,13 @@ TEST_F(BlobUtilsTest, SeparateBlobArray) {
182182
BlobUtils::SeparateBlobArray(struct_array, /*inline_fields=*/{"f2_blob"}),
183183
"SeparateBlobArray expects at least one non-inline blob field, but got none.");
184184

185-
// All fields are blob with no inline -> no main field -> should return error
185+
// All fields are blob with no inline -> no main array is needed
186186
auto all_blob_struct = arrow::StructArray::Make({blob_array_data}, {blob_field}).ValueOrDie();
187187
auto all_blob_sa = std::dynamic_pointer_cast<arrow::StructArray>(all_blob_struct);
188-
ASSERT_NOK_WITH_MSG(BlobUtils::SeparateBlobArray(all_blob_sa, /*inline_fields=*/{}),
189-
"SeparateBlobArray expects at least one main field, but got none.");
188+
ASSERT_OK_AND_ASSIGN(auto all_blob_separated,
189+
BlobUtils::SeparateBlobArray(all_blob_sa, /*inline_fields=*/{}));
190+
ASSERT_EQ(nullptr, all_blob_separated.main_array);
191+
ASSERT_TRUE(all_blob_separated.blob_array->Equals(*all_blob_sa));
190192
}
191193

192194
TEST_F(BlobUtilsTest, SeparateBlobArrayWithPartialInline) {

src/paimon/core/append/append_only_writer.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,14 @@ AppendOnlyWriter::RollingFileWriterResult AppendOnlyWriter::CreateRollingBlobWri
238238
options_.GetBlobTargetFileSize(), single_blob_file_writer_factory);
239239
};
240240

241+
WriterFactory main_writer_factory;
242+
if (schemas.main_schema->num_fields() > 0) {
243+
main_writer_factory =
244+
GetDataFileWriterFactory(schemas.main_schema, schemas.main_schema->field_names());
245+
}
241246
return std::make_unique<RollingBlobFileWriter>(
242-
options_.GetTargetFileSize(/*has_primary_key=*/false),
243-
GetDataFileWriterFactory(schemas.main_schema, schemas.main_schema->field_names()),
244-
blob_schema, blob_writer_creator, arrow::struct_(write_schema_->fields()), inline_fields);
247+
options_.GetTargetFileSize(/*has_primary_key=*/false), main_writer_factory, blob_schema,
248+
blob_writer_creator, arrow::struct_(write_schema_->fields()), inline_fields);
245249
}
246250

247251
Status AppendOnlyWriter::Sync() {

src/paimon/core/append/append_only_writer_test.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,53 @@ TEST_F(AppendOnlyWriterTest, TestWriteWithSingleBlobField) {
741741
ASSERT_OK(writer->Close());
742742
}
743743

744+
TEST_F(AppendOnlyWriterTest, TestWriteWithOnlyBlobField) {
745+
auto options =
746+
CreateOptions({{Options::FILE_FORMAT, "orc"}, {Options::MANIFEST_FORMAT, "orc"}});
747+
auto dir = UniqueTestDirectory::Create();
748+
ASSERT_TRUE(dir);
749+
auto path_factory = CreatePathFactory(dir->Str(), "orc", options);
750+
751+
auto blob_field = BlobUtils::ToArrowField("blob", false);
752+
auto schema = arrow::schema({blob_field});
753+
ASSERT_OK_AND_ASSIGN(auto writer,
754+
CreateAppendOnlyWriter(options, /*schema_id=*/0, schema,
755+
/*write_cols=*/std::vector<std::string>{"blob"},
756+
/*max_sequence_number=*/-1, path_factory,
757+
compact_manager_, memory_pool_));
758+
759+
arrow::LargeBinaryBuilder blob_builder;
760+
ASSERT_TRUE(blob_builder.Append("a", 1).ok());
761+
ASSERT_TRUE(blob_builder.Append("bb", 2).ok());
762+
auto blob_array = blob_builder.Finish().ValueOrDie();
763+
764+
ASSERT_OK(writer->Write(CreateStructBatch(schema, {blob_array})));
765+
ASSERT_OK_AND_ASSIGN(CommitIncrement inc, writer->PrepareCommit(/*wait_compaction=*/true));
766+
ASSERT_OK(writer->Close());
767+
768+
const auto& new_files = inc.GetNewFilesIncrement().NewFiles();
769+
ASSERT_EQ(new_files.size(), 1);
770+
ASSERT_TRUE(BlobUtils::IsBlobFile(new_files[0]->file_name));
771+
ASSERT_EQ(new_files[0]->row_count, 2);
772+
ASSERT_TRUE(new_files[0]->write_cols.has_value());
773+
ASSERT_EQ(new_files[0]->write_cols.value(), std::vector<std::string>({"blob"}));
774+
std::string blob_file_path = path_factory->ToPath(new_files[0]->file_name);
775+
ASSERT_TRUE(options.GetFileSystem()->Exists(blob_file_path).value());
776+
777+
auto blob_reader = OpenFormatReader(blob_file_path, "blob");
778+
::ArrowSchema c_blob_schema;
779+
ASSERT_TRUE(arrow::ExportSchema(*schema, &c_blob_schema).ok());
780+
ASSERT_OK(blob_reader->SetReadSchema(&c_blob_schema, /*predicate=*/nullptr,
781+
/*selection_bitmap=*/std::nullopt));
782+
ASSERT_OK_AND_ASSIGN(auto actual_array, ReadResultCollector::CollectResult(blob_reader.get()));
783+
auto expected_struct_array =
784+
arrow::StructArray::Make({blob_array}, {blob_field->name()}).ValueOrDie();
785+
auto expected_array = std::make_shared<arrow::ChunkedArray>(expected_struct_array);
786+
ASSERT_TRUE(expected_array->Equals(actual_array)) << "Expected:\n"
787+
<< expected_array->ToString() << "\nActual:\n"
788+
<< actual_array->ToString();
789+
}
790+
744791
TEST_F(AppendOnlyWriterTest, TestWriteWithMultipleBlobFields) {
745792
auto options =
746793
CreateOptions({{Options::FILE_FORMAT, "orc"}, {Options::MANIFEST_FORMAT, "orc"}});

src/paimon/core/io/rolling_blob_file_writer.cpp

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "paimon/core/io/rolling_blob_file_writer.h"
1818

19+
#include <map>
1920
#include <memory>
2021
#include <string>
2122
#include <utility>
@@ -27,7 +28,6 @@
2728
#include "arrow/c/bridge.h"
2829
#include "arrow/c/helpers.h"
2930
#include "fmt/format.h"
30-
#include "fmt/ranges.h"
3131
#include "paimon/common/data/blob_utils.h"
3232
#include "paimon/common/utils/arrow/status_utils.h"
3333
#include "paimon/common/utils/scope_guard.h"
@@ -55,7 +55,7 @@ RollingBlobFileWriter::RollingBlobFileWriter(
5555
Status RollingBlobFileWriter::Write(::ArrowArray* record) {
5656
ScopeGuard guard([this]() -> void { this->Abort(); });
5757
// Open the current writer if write the first record or roll over happen before.
58-
if (PAIMON_UNLIKELY(current_writer_ == nullptr)) {
58+
if (writer_factory_ != nullptr && PAIMON_UNLIKELY(current_writer_ == nullptr)) {
5959
PAIMON_RETURN_NOT_OK(OpenCurrentWriter());
6060
}
6161
if (PAIMON_UNLIKELY(blob_writer_ == nullptr)) {
@@ -69,12 +69,14 @@ Status RollingBlobFileWriter::Write(::ArrowArray* record) {
6969
PAIMON_ASSIGN_OR_RAISE(BlobUtils::SeparatedStructArrays separated_arrays,
7070
BlobUtils::SeparateBlobArray(struct_array, inline_fields_));
7171
// Write main (non-blob) data
72-
::ArrowArray c_main_array;
73-
PAIMON_RETURN_NOT_OK_FROM_ARROW(
74-
arrow::ExportArray(*separated_arrays.main_array, &c_main_array));
75-
ScopeGuard array_lifecycle_guard(
76-
[&c_main_array]() -> void { ArrowArrayRelease(&c_main_array); });
77-
PAIMON_RETURN_NOT_OK(current_writer_->Write(&c_main_array));
72+
if (current_writer_ != nullptr) {
73+
::ArrowArray c_main_array;
74+
PAIMON_RETURN_NOT_OK_FROM_ARROW(
75+
arrow::ExportArray(*separated_arrays.main_array, &c_main_array));
76+
ScopeGuard array_lifecycle_guard(
77+
[&c_main_array]() -> void { ArrowArrayRelease(&c_main_array); });
78+
PAIMON_RETURN_NOT_OK(current_writer_->Write(&c_main_array));
79+
}
7880

7981
// Write blob data via MultipleBlobFileWriter (each blob field independently)
8082
::ArrowArray c_blob_array;
@@ -84,28 +86,31 @@ Status RollingBlobFileWriter::Write(::ArrowArray* record) {
8486
PAIMON_RETURN_NOT_OK(blob_writer_->Write(&c_blob_array));
8587

8688
record_count_ += record_count;
87-
PAIMON_ASSIGN_OR_RAISE(bool need_rolling_file, NeedRollingFile());
88-
if (need_rolling_file) {
89-
PAIMON_RETURN_NOT_OK(CloseCurrentWriter());
89+
if (current_writer_ != nullptr) {
90+
PAIMON_ASSIGN_OR_RAISE(bool need_rolling_file, NeedRollingFile());
91+
if (need_rolling_file) {
92+
PAIMON_RETURN_NOT_OK(CloseCurrentWriter());
93+
}
9094
}
9195
guard.Release();
9296
return Status::OK();
9397
}
9498

9599
Status RollingBlobFileWriter::CloseCurrentWriter() {
96-
if (current_writer_ == nullptr) {
97-
return Status::OK();
98-
}
99100
if (blob_writer_ == nullptr) {
100101
return Status::OK();
101102
}
102-
PAIMON_ASSIGN_OR_RAISE(std::shared_ptr<DataFileMeta> main_data_file_meta, CloseMainWriter());
103+
std::shared_ptr<DataFileMeta> main_data_file_meta;
104+
if (current_writer_ != nullptr) {
105+
PAIMON_ASSIGN_OR_RAISE(main_data_file_meta, CloseMainWriter());
106+
}
103107
PAIMON_ASSIGN_OR_RAISE(std::vector<std::shared_ptr<DataFileMeta>> blob_metas,
104108
CloseBlobWriter());
105-
PAIMON_RETURN_NOT_OK(
106-
ValidateFileConsistency(main_data_file_meta, blob_metas, blob_schema_->num_fields()));
107109

108-
results_.push_back(main_data_file_meta);
110+
if (main_data_file_meta != nullptr) {
111+
PAIMON_RETURN_NOT_OK(ValidateFileConsistency(main_data_file_meta, blob_metas));
112+
results_.push_back(main_data_file_meta);
113+
}
109114
results_.insert(results_.end(), blob_metas.begin(), blob_metas.end());
110115

111116
current_writer_.reset();
@@ -137,29 +142,25 @@ Result<std::vector<std::shared_ptr<DataFileMeta>>> RollingBlobFileWriter::CloseB
137142

138143
Status RollingBlobFileWriter::ValidateFileConsistency(
139144
const std::shared_ptr<DataFileMeta>& main_data_file_meta,
140-
const std::vector<std::shared_ptr<DataFileMeta>>& blob_tagged_metas, int32_t blob_field_count) {
141-
if (blob_tagged_metas.empty()) {
142-
return Status::OK();
143-
}
144-
// With multiple blob fields, each blob field produces its own set of files.
145-
// total_blob_row_count should be exactly main_row_count * blob_field_count.
146-
int64_t main_row_count = main_data_file_meta->row_count;
147-
int64_t expected_blob_row_count = main_row_count * blob_field_count;
148-
int64_t total_blob_row_count = 0;
145+
const std::vector<std::shared_ptr<DataFileMeta>>& blob_tagged_metas) {
146+
std::map<std::string, int64_t> blob_field_row_counts;
149147
for (const auto& blob_tagged_meta : blob_tagged_metas) {
150-
total_blob_row_count += blob_tagged_meta->row_count;
148+
if (!blob_tagged_meta->write_cols || blob_tagged_meta->write_cols->empty()) {
149+
return Status::Invalid(
150+
fmt::format("This is a bug: Blob file {} must contain a write column.",
151+
blob_tagged_meta->file_name));
152+
}
153+
blob_field_row_counts[blob_tagged_meta->write_cols->at(0)] += blob_tagged_meta->row_count;
151154
}
152-
if (total_blob_row_count != expected_blob_row_count) {
153-
std::vector<std::string> blob_file_names;
154-
for (const auto& blob_tagged_meta : blob_tagged_metas) {
155-
blob_file_names.push_back(blob_tagged_meta->file_name);
155+
156+
int64_t main_row_count = main_data_file_meta->row_count;
157+
for (const auto& [field_name, row_count] : blob_field_row_counts) {
158+
if (row_count != main_row_count) {
159+
return Status::Invalid(fmt::format(
160+
"This is a bug: The row count of main file and blob file does not match. Main "
161+
"file: {} (row count: {}), blob field name: {} (row count: {})",
162+
main_data_file_meta->file_name, main_row_count, field_name, row_count));
156163
}
157-
return Status::Invalid(fmt::format(
158-
"This is a bug: The row count of main file and blob files does not match. "
159-
"Main file: {} (row count: {}), blob field count: {}, "
160-
"expected blob row count: {}, blob files: {} (actual total row count: {})",
161-
main_data_file_meta->file_name, main_row_count, blob_field_count,
162-
expected_blob_row_count, fmt::join(blob_file_names, ", "), total_blob_row_count));
163164
}
164165
return Status::OK();
165166
}

src/paimon/core/io/rolling_blob_file_writer.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ namespace paimon {
3838
/// between them.
3939
///
4040
/// Multiple blob fields are supported. Each blob field is written to its own set of blob files
41-
/// independently via MultipleBlobFileWriter.
41+
/// independently via MultipleBlobFileWriter. For blob-only writes, the main writer factory may be
42+
/// nullptr and only blob files are produced.
4243
///
4344
/// <pre>
4445
/// For example,
@@ -76,8 +77,7 @@ class RollingBlobFileWriter
7677
private:
7778
static Status ValidateFileConsistency(
7879
const std::shared_ptr<DataFileMeta>& main_data_file_meta,
79-
const std::vector<std::shared_ptr<DataFileMeta>>& blob_tagged_metas,
80-
int32_t blob_field_count);
80+
const std::vector<std::shared_ptr<DataFileMeta>>& blob_tagged_metas);
8181

8282
Status CloseCurrentWriter();
8383

src/paimon/core/io/rolling_blob_file_writer_test.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,15 @@ TEST_F(RollingBlobFileWriterTest, ValidateFileConsistency) {
8282
/*delete_row_count=*/0, /*embedded_index=*/nullptr, FileSource::Append(),
8383
/*value_stats_cols=*/std::nullopt, /*external_path=*/std::nullopt, /*first_row_id=*/3,
8484
/*write_cols=*/std::vector<std::string>({"blob"}));
85-
ASSERT_OK(RollingBlobFileWriter::ValidateFileConsistency(file_meta1, {file_meta2, file_meta3},
86-
/*blob_field_count=*/1));
87-
ASSERT_NOK_WITH_MSG(RollingBlobFileWriter::ValidateFileConsistency(file_meta1, {file_meta2},
88-
/*blob_field_count=*/2),
89-
"This is a bug: The row count of main file and blob files does not match.");
85+
ASSERT_OK(RollingBlobFileWriter::ValidateFileConsistency(file_meta1, {file_meta2, file_meta3}));
86+
ASSERT_NOK_WITH_MSG(RollingBlobFileWriter::ValidateFileConsistency(file_meta1, {file_meta2}),
87+
"This is a bug: The row count of main file and blob file does not match.");
88+
89+
file_meta2->write_cols = std::vector<std::string>({"blob1"});
90+
file_meta3->write_cols = std::vector<std::string>({"blob2"});
91+
ASSERT_NOK_WITH_MSG(
92+
RollingBlobFileWriter::ValidateFileConsistency(file_meta1, {file_meta2, file_meta3}),
93+
"This is a bug: The row count of main file and blob file does not match.");
9094
}
9195

9296
} // namespace paimon::test

0 commit comments

Comments
 (0)