Skip to content

Commit c479ac7

Browse files
lxy-9602lxy-9602
authored
refactor(blob): refactor BlobFileContext to honor the actual schema when classifying BLOB fields (alibaba#332)
1 parent c1b5a27 commit c479ac7

3 files changed

Lines changed: 67 additions & 83 deletions

File tree

‎src/paimon/core/operation/blob_file_context.cpp‎

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -39,51 +39,61 @@ BlobFileContext::BlobFileContext(std::set<std::string> descriptor_fields,
3939

4040
std::unique_ptr<BlobFileContext> BlobFileContext::Create(
4141
const std::shared_ptr<arrow::Schema>& schema, const CoreOptions& options) {
42-
// Check if there are any BLOB fields in the schema
43-
bool has_blob = false;
42+
// Collect the BLOB field names that are present in the given schema. The schema may
43+
// only contain a subset of the table columns (e.g. a projected read/write schema), so all
44+
// field categories below must be derived from this set rather than from the options
45+
// alone, which describe the full table.
46+
std::set<std::string> schema_blob_fields;
4447
for (int i = 0; i < schema->num_fields(); ++i) {
45-
if (BlobUtils::IsBlobField(schema->field(i))) {
46-
has_blob = true;
47-
break;
48+
const auto& field = schema->field(i);
49+
if (BlobUtils::IsBlobField(field)) {
50+
schema_blob_fields.insert(field->name());
4851
}
4952
}
50-
if (!has_blob) {
53+
if (schema_blob_fields.empty()) {
5154
return nullptr;
5255
}
5356

5457
// Populate descriptor fields
5558
std::set<std::string> descriptor_fields;
5659
for (const auto& name : options.GetBlobDescriptorFields()) {
57-
descriptor_fields.insert(name);
60+
if (schema_blob_fields.count(name) > 0) {
61+
descriptor_fields.insert(name);
62+
}
5863
}
5964

6065
// Populate view fields
6166
std::set<std::string> view_fields;
6267
for (const auto& name : options.GetBlobViewFields()) {
63-
view_fields.insert(name);
68+
if (schema_blob_fields.count(name) > 0) {
69+
view_fields.insert(name);
70+
}
6471
}
6572

6673
// Populate inline fields from options (descriptor ∪ view)
6774
std::set<std::string> inline_fields;
6875
for (const auto& name : options.GetBlobInlineFields()) {
69-
inline_fields.insert(name);
76+
if (schema_blob_fields.count(name) > 0) {
77+
inline_fields.insert(name);
78+
}
7079
}
7180

7281
// Populate external storage fields
7382
std::set<std::string> external_storage_fields;
7483
for (const auto& name : options.GetBlobExternalStorageFields()) {
75-
external_storage_fields.insert(name);
84+
if (schema_blob_fields.count(name) > 0) {
85+
external_storage_fields.insert(name);
86+
}
7687
}
7788

7889
// Populate external storage path
7990
std::optional<std::string> external_storage_path = options.GetBlobExternalStoragePath();
8091

81-
// Determine blob_file_fields: BLOB fields that are NOT inline
92+
// Determine blob_file_fields: schema BLOB fields that are NOT inline
8293
std::set<std::string> blob_file_fields;
83-
for (int i = 0; i < schema->num_fields(); ++i) {
84-
const auto& field = schema->field(i);
85-
if (BlobUtils::IsBlobField(field) && inline_fields.count(field->name()) == 0) {
86-
blob_file_fields.insert(field->name());
94+
for (const auto& name : schema_blob_fields) {
95+
if (inline_fields.count(name) == 0) {
96+
blob_file_fields.insert(name);
8797
}
8898
}
8999

@@ -93,26 +103,6 @@ std::unique_ptr<BlobFileContext> BlobFileContext::Create(
93103
std::move(blob_file_fields), std::move(external_storage_path)));
94104
}
95105

96-
bool BlobFileContext::IsInlineField(const std::string& field_name) const {
97-
return inline_fields_.count(field_name) > 0;
98-
}
99-
100-
bool BlobFileContext::IsBlobFileField(const std::string& field_name) const {
101-
return blob_file_fields_.count(field_name) > 0;
102-
}
103-
104-
bool BlobFileContext::IsDescriptorField(const std::string& field_name) const {
105-
return descriptor_fields_.count(field_name) > 0;
106-
}
107-
108-
bool BlobFileContext::IsViewField(const std::string& field_name) const {
109-
return view_fields_.count(field_name) > 0;
110-
}
111-
112-
bool BlobFileContext::IsExternalStorageField(const std::string& field_name) const {
113-
return external_storage_fields_.count(field_name) > 0;
114-
}
115-
116106
bool BlobFileContext::RequireBlobFileWriter() const {
117107
return !blob_file_fields_.empty();
118108
}

‎src/paimon/core/operation/blob_file_context.h‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,22 +46,6 @@ class BlobFileContext {
4646
static std::unique_ptr<BlobFileContext> Create(const std::shared_ptr<arrow::Schema>& schema,
4747
const CoreOptions& options);
4848

49-
/// Returns true if the given field should be stored inline in the main data file
50-
/// (either as descriptor bytes or view bytes).
51-
bool IsInlineField(const std::string& field_name) const;
52-
53-
/// Returns true if the given field should be written to a separate .blob file.
54-
bool IsBlobFileField(const std::string& field_name) const;
55-
56-
/// Returns true if the given field is a descriptor field.
57-
bool IsDescriptorField(const std::string& field_name) const;
58-
59-
/// Returns true if the given field is a view field.
60-
bool IsViewField(const std::string& field_name) const;
61-
62-
/// Returns true if the given field should be written to external storage.
63-
bool IsExternalStorageField(const std::string& field_name) const;
64-
6549
/// Returns true if there are any BLOB fields that need a .blob file writer.
6650
bool RequireBlobFileWriter() const;
6751

‎src/paimon/core/operation/blob_file_context_test.cpp‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -91,18 +91,6 @@ TEST_F(BlobFileContextTest, MixedInlineAndBlobFile) {
9191
// blob file fields = non-inline blob fields
9292
ASSERT_EQ(context->GetBlobFileFields(), std::set<std::string>({"video", "audio"}));
9393

94-
// Query methods
95-
ASSERT_TRUE(context->IsInlineField("image"));
96-
ASSERT_TRUE(context->IsDescriptorField("image"));
97-
ASSERT_FALSE(context->IsViewField("image"));
98-
ASSERT_FALSE(context->IsBlobFileField("image"));
99-
100-
ASSERT_FALSE(context->IsInlineField("video"));
101-
ASSERT_TRUE(context->IsBlobFileField("video"));
102-
103-
ASSERT_FALSE(context->IsInlineField("audio"));
104-
ASSERT_TRUE(context->IsBlobFileField("audio"));
105-
10694
// Requires blob file writer for video and audio
10795
ASSERT_TRUE(context->RequireBlobFileWriter());
10896
ASSERT_FALSE(context->RequireExternalStorageWriter());
@@ -126,9 +114,6 @@ TEST_F(BlobFileContextTest, ExternalStorageFields) {
126114
ASSERT_EQ(context->GetExternalStoragePath(), "oss://bucket/blob/");
127115
ASSERT_TRUE(context->GetBlobFileFields().empty());
128116

129-
ASSERT_TRUE(context->IsExternalStorageField("image"));
130-
ASSERT_FALSE(context->IsExternalStorageField("video"));
131-
132117
ASSERT_FALSE(context->RequireBlobFileWriter());
133118
ASSERT_TRUE(context->RequireExternalStorageWriter());
134119
}
@@ -148,10 +133,6 @@ TEST_F(BlobFileContextTest, ViewFields) {
148133
ASSERT_EQ(context->GetInlineFields(), std::set<std::string>({"ref_image"}));
149134
ASSERT_EQ(context->GetBlobFileFields(), std::set<std::string>({"raw_blob"}));
150135

151-
ASSERT_TRUE(context->IsInlineField("ref_image"));
152-
ASSERT_TRUE(context->IsViewField("ref_image"));
153-
ASSERT_FALSE(context->IsDescriptorField("ref_image"));
154-
155136
ASSERT_TRUE(context->RequireBlobFileWriter());
156137
ASSERT_FALSE(context->RequireExternalStorageWriter());
157138
}
@@ -176,24 +157,53 @@ TEST_F(BlobFileContextTest, DescriptorAndViewTogether) {
176157
ASSERT_EQ(context->GetExternalStoragePath(), "/tmp/ext/");
177158
ASSERT_EQ(context->GetBlobFileFields(), std::set<std::string>({"normal_blob"}));
178159

179-
ASSERT_TRUE(context->IsDescriptorField("desc_blob"));
180-
ASSERT_TRUE(context->IsExternalStorageField("desc_blob"));
181-
ASSERT_TRUE(context->IsInlineField("desc_blob"));
182-
ASSERT_FALSE(context->IsBlobFileField("desc_blob"));
160+
ASSERT_TRUE(context->RequireBlobFileWriter());
161+
ASSERT_TRUE(context->RequireExternalStorageWriter());
162+
}
183163

184-
ASSERT_TRUE(context->IsViewField("view_blob"));
185-
ASSERT_TRUE(context->IsInlineField("view_blob"));
186-
ASSERT_FALSE(context->IsDescriptorField("view_blob"));
164+
TEST_F(BlobFileContextTest, PartialSchemaIgnoresAbsentFields) {
165+
// Schema only carries "image"; "video" and "audio" are not part of this write schema.
166+
auto schema = MakeSchema({"id"}, {"image"});
167+
std::map<std::string, std::string> opts_map = {
168+
{Options::BLOB_DESCRIPTOR_FIELD, "image,audio"},
169+
{Options::BLOB_VIEW_FIELD, "video"},
170+
{Options::BLOB_EXTERNAL_STORAGE_FIELD, "image,video"},
171+
{Options::BLOB_EXTERNAL_STORAGE_PATH, "oss://bucket/blob/"},
172+
};
173+
ASSERT_OK_AND_ASSIGN(auto options, CoreOptions::FromMap(opts_map));
174+
auto context = BlobFileContext::Create(schema, options);
175+
ASSERT_TRUE(context);
187176

188-
ASSERT_FALSE(context->IsInlineField("normal_blob"));
189-
ASSERT_TRUE(context->IsBlobFileField("normal_blob"));
177+
// Only "image" survives filtering; "audio" / "video" are not in the schema.
178+
ASSERT_EQ(context->GetDescriptorFields(), std::set<std::string>({"image"}));
179+
ASSERT_TRUE(context->GetViewFields().empty());
180+
ASSERT_EQ(context->GetInlineFields(), std::set<std::string>({"image"}));
181+
ASSERT_EQ(context->GetExternalStorageFields(), std::set<std::string>({"image"}));
190182

191-
// Non-existent field
192-
ASSERT_FALSE(context->IsInlineField("not_exist"));
193-
ASSERT_FALSE(context->IsBlobFileField("not_exist"));
183+
// No non-inline blob field remains in the schema.
184+
ASSERT_TRUE(context->GetBlobFileFields().empty());
194185

195-
ASSERT_TRUE(context->RequireBlobFileWriter());
186+
ASSERT_FALSE(context->RequireBlobFileWriter());
196187
ASSERT_TRUE(context->RequireExternalStorageWriter());
197188
}
198189

190+
TEST_F(BlobFileContextTest, PartialSchemaWithOnlyBlobFileField) {
191+
auto schema = MakeSchema({"id"}, {"audio"});
192+
std::map<std::string, std::string> opts_map = {
193+
{Options::BLOB_DESCRIPTOR_FIELD, "image"}, {Options::BLOB_VIEW_FIELD, "video"},
194+
// "audio" is not configured as inline -> goes to .blob file
195+
};
196+
ASSERT_OK_AND_ASSIGN(auto options, CoreOptions::FromMap(opts_map));
197+
auto context = BlobFileContext::Create(schema, options);
198+
ASSERT_TRUE(context);
199+
200+
ASSERT_TRUE(context->GetDescriptorFields().empty());
201+
ASSERT_TRUE(context->GetViewFields().empty());
202+
ASSERT_TRUE(context->GetInlineFields().empty());
203+
ASSERT_EQ(context->GetBlobFileFields(), std::set<std::string>({"audio"}));
204+
205+
ASSERT_TRUE(context->RequireBlobFileWriter());
206+
ASSERT_FALSE(context->RequireExternalStorageWriter());
207+
}
208+
199209
} // namespace paimon

0 commit comments

Comments
 (0)