Skip to content

Commit 6ccfc7e

Browse files
committed
fix comments for paipai
1 parent b83a6c3 commit 6ccfc7e

7 files changed

Lines changed: 189 additions & 23 deletions

File tree

‎src/paimon/core/schema/schema_validation.cpp‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ Status SchemaValidation::ValidateTableSchema(const TableSchema& schema) {
197197

198198
PAIMON_RETURN_NOT_OK(ValidateRowTracking(schema, options));
199199
PAIMON_RETURN_NOT_OK(ValidateBlobFields(schema, options));
200+
PAIMON_RETURN_NOT_OK(ValidateMosaicDataFields(schema, options));
200201
PAIMON_RETURN_NOT_OK(ValidateMapStorageLayout(schema, options));
201202
PAIMON_RETURN_NOT_OK(ValidateVectorFields(schema, options));
202203
return Status::OK();
@@ -561,6 +562,70 @@ Status SchemaValidation::ValidateBlobFields(const TableSchema& schema, const Cor
561562
return Status::OK();
562563
}
563564

565+
Status SchemaValidation::ValidateMosaicDataField(const std::shared_ptr<arrow::Field>& field) {
566+
if (VariantTypeUtils::IsVariantField(field)) {
567+
return Status::Invalid("Mosaic file format does not support type VARIANT");
568+
}
569+
if (BlobUtils::IsBlobField(field)) {
570+
return Status::Invalid("Mosaic file format does not support type BLOB");
571+
}
572+
573+
const std::shared_ptr<arrow::DataType>& type = field->type();
574+
switch (type->id()) {
575+
case arrow::Type::BOOL:
576+
case arrow::Type::INT8:
577+
case arrow::Type::INT16:
578+
case arrow::Type::INT32:
579+
case arrow::Type::INT64:
580+
case arrow::Type::FLOAT:
581+
case arrow::Type::DOUBLE:
582+
case arrow::Type::DATE32:
583+
case arrow::Type::STRING:
584+
case arrow::Type::BINARY:
585+
case arrow::Type::TIME32:
586+
case arrow::Type::TIMESTAMP:
587+
case arrow::Type::DECIMAL128:
588+
return Status::OK();
589+
case arrow::Type::LIST:
590+
return ValidateMosaicDataField(type->field(0));
591+
case arrow::Type::MAP: {
592+
const auto& map_type = checked_cast<const arrow::MapType&>(*type);
593+
PAIMON_RETURN_NOT_OK(ValidateMosaicDataField(map_type.key_field()));
594+
return ValidateMosaicDataField(map_type.item_field());
595+
}
596+
case arrow::Type::FIXED_SIZE_LIST:
597+
return Status::Invalid("Mosaic file format does not support type VECTOR");
598+
case arrow::Type::STRUCT:
599+
return Status::Invalid("Mosaic file format does not support type ROW");
600+
default:
601+
break;
602+
}
603+
return Status::Invalid(
604+
fmt::format("Mosaic file format does not support type {}", type->ToString()));
605+
}
606+
607+
Status SchemaValidation::ValidateMosaicDataFields(const TableSchema& schema,
608+
const CoreOptions& options) {
609+
if (StringUtils::ToLowerCase(options.GetFileFormat()->Identifier()) != "mosaic") {
610+
return Status::OK();
611+
}
612+
613+
const std::vector<std::string> inline_blob_fields = options.GetBlobInlineFields();
614+
const std::set<std::string> inline_blob_field_set(inline_blob_fields.begin(),
615+
inline_blob_fields.end());
616+
// Match Java SchemaValidation by validating only fields stored in the normal data file. C++
617+
// permits BLOB only as a top-level field; descriptor and view fields are inline, so Mosaic
618+
// must reject them here.
619+
for (const DataField& field : schema.Fields()) {
620+
if (BlobUtils::IsBlobField(field.ArrowField()) &&
621+
inline_blob_field_set.count(field.Name()) == 0) {
622+
continue;
623+
}
624+
PAIMON_RETURN_NOT_OK(ValidateMosaicDataField(field.ArrowField()));
625+
}
626+
return Status::OK();
627+
}
628+
564629
Status SchemaValidation::ValidateMapStorageLayout(const TableSchema& schema,
565630
const CoreOptions& options) {
566631
// Extract all field names that have map.storage-layout configured from options

‎src/paimon/core/schema/schema_validation.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ class SchemaValidation {
7373

7474
static Status ValidateBlobFields(const TableSchema& schema, const CoreOptions& options);
7575

76+
static Status ValidateMosaicDataFields(const TableSchema& schema, const CoreOptions& options);
77+
78+
static Status ValidateMosaicDataField(const std::shared_ptr<arrow::Field>& field);
79+
7680
static Status ValidateMapStorageLayout(const TableSchema& schema, const CoreOptions& options);
7781

7882
static Status ValidateVectorFields(const TableSchema& schema, const CoreOptions& options);

‎src/paimon/core/schema/schema_validation_test.cpp‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "arrow/api.h"
2525
#include "gtest/gtest.h"
2626
#include "paimon/common/data/blob_utils.h"
27+
#include "paimon/common/data/variant/variant_type_utils.h"
2728
#include "paimon/core/schema/table_schema.h"
2829
#include "paimon/defs.h"
2930
#include "paimon/testing/utils/testharness.h"
@@ -109,6 +110,74 @@ TEST(SchemaValidationTest, TestVectorType) {
109110
"VECTOR fields in data-evolution tables are not implemented yet.");
110111
}
111112

113+
#ifdef PAIMON_ENABLE_MOSAIC
114+
TEST(SchemaValidationTest, TestMosaicDataTypes) {
115+
std::map<std::string, std::string> options = {{Options::BUCKET, "-1"},
116+
{Options::FILE_FORMAT, "mosaic"}};
117+
arrow::FieldVector supported_fields = {
118+
arrow::field("f0", arrow::boolean()),
119+
arrow::field("f1", arrow::int8()),
120+
arrow::field("f2", arrow::int16()),
121+
arrow::field("f3", arrow::int32()),
122+
arrow::field("f4", arrow::int64()),
123+
arrow::field("f5", arrow::float32()),
124+
arrow::field("f6", arrow::float64()),
125+
arrow::field("f7", arrow::utf8()),
126+
arrow::field("f8", arrow::binary()),
127+
arrow::field("f9", arrow::date32()),
128+
arrow::field("f10", arrow::timestamp(arrow::TimeUnit::NANO)),
129+
arrow::field("f11", arrow::decimal128(38, 2)),
130+
arrow::field("f12", arrow::list(arrow::float32())),
131+
arrow::field("f13", arrow::map(arrow::int8(), arrow::int16())),
132+
};
133+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<TableSchema> table_schema,
134+
TableSchema::Create(/*schema_id=*/0, arrow::schema(supported_fields),
135+
/*partition_keys=*/{}, /*primary_keys=*/{}, options));
136+
ASSERT_OK(SchemaValidation::ValidateTableSchema(*table_schema));
137+
138+
arrow::FieldVector unsupported_fields = {
139+
arrow::field("row", arrow::struct_({arrow::field("value", arrow::int32())})),
140+
VariantTypeUtils::ToArrowField("variant"),
141+
arrow::field("vector", arrow::fixed_size_list(arrow::float32(), 3)),
142+
arrow::field("nested_row",
143+
arrow::list(arrow::struct_({arrow::field("value", arrow::int32())}))),
144+
};
145+
std::vector<std::string> expected_errors = {"type ROW", "type VARIANT", "type VECTOR",
146+
"type ROW"};
147+
for (size_t i = 0; i < unsupported_fields.size(); ++i) {
148+
SCOPED_TRACE("field=" + unsupported_fields[i]->name());
149+
ASSERT_OK_AND_ASSIGN(
150+
table_schema,
151+
TableSchema::Create(/*schema_id=*/0, arrow::schema({unsupported_fields[i]}),
152+
/*partition_keys=*/{}, /*primary_keys=*/{}, options));
153+
ASSERT_NOK_WITH_MSG(SchemaValidation::ValidateTableSchema(*table_schema),
154+
expected_errors[i]);
155+
}
156+
157+
std::shared_ptr<arrow::Field> blob_field = BlobUtils::ToArrowField("blob", false);
158+
std::map<std::string, std::string> blob_options = {
159+
{Options::BUCKET, "-1"},
160+
{Options::FILE_FORMAT, "mosaic"},
161+
{Options::ROW_TRACKING_ENABLED, "true"},
162+
{Options::DATA_EVOLUTION_ENABLED, "true"},
163+
};
164+
ASSERT_OK_AND_ASSIGN(
165+
table_schema,
166+
TableSchema::Create(/*schema_id=*/0,
167+
arrow::schema({arrow::field("id", arrow::int32()), blob_field}),
168+
/*partition_keys=*/{}, /*primary_keys=*/{}, blob_options));
169+
ASSERT_OK(SchemaValidation::ValidateTableSchema(*table_schema));
170+
171+
blob_options[Options::BLOB_DESCRIPTOR_FIELD] = "blob";
172+
ASSERT_OK_AND_ASSIGN(
173+
table_schema,
174+
TableSchema::Create(/*schema_id=*/0,
175+
arrow::schema({arrow::field("id", arrow::int32()), blob_field}),
176+
/*partition_keys=*/{}, /*primary_keys=*/{}, blob_options));
177+
ASSERT_NOK_WITH_MSG(SchemaValidation::ValidateTableSchema(*table_schema), "type BLOB");
178+
}
179+
#endif
180+
112181
TEST(SchemaValidationTest, TestRowTracking) {
113182
auto f0 = arrow::field("f0", arrow::utf8());
114183
auto f1 = arrow::field("f1", arrow::int32());

‎src/paimon/format/mosaic/mosaic_file_batch_reader.cpp‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -72,45 +72,34 @@ Result<std::unique_ptr<MosaicFileBatchReader>> MosaicFileBatchReader::Create(
7272
input_file.ctx = input_context.get();
7373
input_file.read_at_fn = MosaicInputContext::ReadAt;
7474
input_file.length_fn = MosaicInputContext::Length;
75-
MosaicReaderHandle* reader = mosaic_reader_open(input_file);
75+
std::unique_ptr<MosaicReaderHandle, decltype(&mosaic_reader_free)> reader(
76+
mosaic_reader_open(input_file), mosaic_reader_free);
7677
if (reader == nullptr) {
7778
return MosaicFfiError("open Mosaic reader", input_context->GetCallbackStatus());
7879
}
7980

8081
::ArrowSchema ffi_schema = {};
81-
if (mosaic_reader_export_schema(reader, &ffi_schema) != 0) {
82-
Status status = MosaicFfiError("read Mosaic schema", input_context->GetCallbackStatus());
83-
mosaic_reader_free(reader);
84-
return status;
82+
if (mosaic_reader_export_schema(reader.get(), &ffi_schema) != 0) {
83+
return MosaicFfiError("read Mosaic schema", input_context->GetCallbackStatus());
8584
}
86-
arrow::Result<std::shared_ptr<arrow::Schema>> schema_result = arrow::ImportSchema(&ffi_schema);
87-
if (!schema_result.ok()) {
88-
mosaic_reader_free(reader);
89-
return ToPaimonStatus(schema_result.status());
90-
}
91-
std::shared_ptr<arrow::Schema> file_schema = std::move(schema_result).ValueOrDie();
85+
PAIMON_ASSIGN_OR_RAISE_FROM_ARROW(std::shared_ptr<arrow::Schema> file_schema,
86+
arrow::ImportSchema(&ffi_schema));
9287

9388
uint32_t num_row_groups = 0;
94-
if (mosaic_reader_num_row_groups(reader, &num_row_groups) != 0) {
95-
Status status =
96-
MosaicFfiError("read Mosaic row group count", input_context->GetCallbackStatus());
97-
mosaic_reader_free(reader);
98-
return status;
89+
if (mosaic_reader_num_row_groups(reader.get(), &num_row_groups) != 0) {
90+
return MosaicFfiError("read Mosaic row group count", input_context->GetCallbackStatus());
9991
}
10092
uint64_t total_rows = 0;
10193
for (uint32_t row_group = 0; row_group < num_row_groups; ++row_group) {
10294
uint32_t row_count = 0;
103-
if (mosaic_reader_row_group_num_rows(reader, row_group, &row_count) != 0) {
104-
Status status =
105-
MosaicFfiError("read Mosaic row count", input_context->GetCallbackStatus());
106-
mosaic_reader_free(reader);
107-
return status;
95+
if (mosaic_reader_row_group_num_rows(reader.get(), row_group, &row_count) != 0) {
96+
return MosaicFfiError("read Mosaic row count", input_context->GetCallbackStatus());
10897
}
10998
total_rows += row_count;
11099
}
111100
return std::unique_ptr<MosaicFileBatchReader>(
112-
new MosaicFileBatchReader(input, batch_size, std::move(input_context), reader, file_schema,
113-
num_row_groups, total_rows, pool, arrow_pool));
101+
new MosaicFileBatchReader(input, batch_size, std::move(input_context), reader.release(),
102+
file_schema, num_row_groups, total_rows, pool, arrow_pool));
114103
}
115104

116105
MosaicFileBatchReader::~MosaicFileBatchReader() {

‎src/paimon/format/mosaic/mosaic_file_format_test.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,39 @@ TEST_F(MosaicFileFormatTest, WriteThenRead) {
200200
AssertReadWithBatchSizes(path, schema, expected, {1, 2, 3, 5, 8});
201201
}
202202

203+
TEST_F(MosaicFileFormatTest, EmptyProjectionPreservesRowCount) {
204+
std::string path = PathUtil::JoinPath(directory_->Str(), "empty-projection.mosaic");
205+
arrow::FieldVector fields = {arrow::field("id", arrow::int32(), false),
206+
arrow::field("name", arrow::utf8())};
207+
std::shared_ptr<arrow::Schema> schema = arrow::schema(fields);
208+
std::shared_ptr<arrow::Array> data =
209+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields),
210+
R"([[1,"one"],[2,"two"],[3,"three"]])")
211+
.ValueOrDie();
212+
ASSERT_OK(WriteFile(path, schema, data, /*batch_size=*/2));
213+
214+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<ReaderBuilder> reader_builder,
215+
format_->CreateReaderBuilder(/*batch_size=*/2));
216+
reader_builder->WithMemoryPool(pool_);
217+
ASSERT_OK_AND_ASSIGN(std::shared_ptr<InputStream> input, file_system_->Open(path));
218+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<FileBatchReader> reader, reader_builder->Build(input));
219+
::ArrowSchema ffi_schema = {};
220+
ASSERT_TRUE(arrow::ExportSchema(*arrow::schema({}), &ffi_schema).ok());
221+
ASSERT_OK(reader->SetReadSchema(&ffi_schema, /*predicate=*/nullptr,
222+
/*selection_bitmap=*/std::nullopt));
223+
224+
for (int64_t expected_rows : {2, 1}) {
225+
ASSERT_OK_AND_ASSIGN(BatchReader::ReadBatch batch, reader->NextBatch());
226+
ASSERT_FALSE(BatchReader::IsEofBatch(batch));
227+
std::shared_ptr<arrow::RecordBatch> record_batch =
228+
arrow::ImportRecordBatch(batch.first.get(), batch.second.get()).ValueOrDie();
229+
ASSERT_EQ(record_batch->num_columns(), 0);
230+
ASSERT_EQ(record_batch->num_rows(), expected_rows);
231+
}
232+
ASSERT_OK_AND_ASSIGN(BatchReader::ReadBatch eof_batch, reader->NextBatch());
233+
ASSERT_TRUE(BatchReader::IsEofBatch(eof_batch));
234+
}
235+
203236
TEST_F(MosaicFileFormatTest, SetReadSchemaResetsReaderToFirstRow) {
204237
std::string path = PathUtil::JoinPath(directory_->Str(), "reset-reader.mosaic");
205238
arrow::FieldVector fields = {arrow::field("id", arrow::int32(), false),

‎src/paimon/format/mosaic/mosaic_format_writer.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ Result<std::unique_ptr<MosaicFormatWriter>> MosaicFormatWriter::Create(
6666

6767
MosaicFormatWriter::~MosaicFormatWriter() {
6868
if (writer_ != nullptr) {
69+
if (!finished_) {
70+
mosaic_writer_close(writer_);
71+
}
6972
mosaic_writer_free(writer_);
7073
}
7174
}

‎test/inte/blob_table_inte_test.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3426,6 +3426,9 @@ TEST_P(BlobTableInteTest, TestDataEvolutionWithBlobDescriptorField) {
34263426
}
34273427

34283428
TEST_P(BlobTableInteTest, TestBlobDescriptorFieldWriteRawBytesDirectly) {
3429+
if (GetParam() == "mosaic") {
3430+
return;
3431+
}
34293432
// Similar to TestBlobDescriptorField but writes raw bytes directly without converting to
34303433
// descriptor first. Descriptor fields reject values without the descriptor magic header.
34313434
arrow::FieldVector fields = {arrow::field("f0", arrow::int32()),

0 commit comments

Comments
 (0)