wangyong9999 commented on code in PR #315:
URL: https://github.com/apache/paimon-cpp/pull/315#discussion_r3975004298
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -802,6 +804,73 @@ Status SchemaValidation::ValidateMosaicDataFields(const
TableSchema& schema,
return Status::OK();
}
+Status SchemaValidation::ValidateLanceDataField(const
std::shared_ptr<arrow::Field>& field) {
+ if (VariantTypeUtils::IsVariantField(field)) {
+ return Status::Invalid("Lance file format does not support type
VARIANT");
+ }
+ if (BlobUtils::IsBlobField(field)) {
+ return Status::Invalid("Lance file format does not support type BLOB");
+ }
+
+ const std::shared_ptr<arrow::DataType>& type = field->type();
+ switch (type->id()) {
+ case arrow::Type::BOOL:
+ case arrow::Type::INT8:
+ case arrow::Type::INT16:
+ case arrow::Type::INT32:
+ case arrow::Type::INT64:
+ case arrow::Type::FLOAT:
+ case arrow::Type::DOUBLE:
+ case arrow::Type::DATE32:
+ case arrow::Type::STRING:
+ case arrow::Type::BINARY:
+ case arrow::Type::TIME32:
+ case arrow::Type::DECIMAL128:
+ case arrow::Type::FIXED_SIZE_LIST:
+ return Status::OK();
+ case arrow::Type::TIMESTAMP: {
+ const auto& timestamp_type = checked_cast<const
arrow::TimestampType&>(*type);
+ if (!timestamp_type.timezone().empty()) {
+ return Status::Invalid(
+ "Lance file format does not support type
LOCAL_ZONED_TIMESTAMP");
+ }
+ return Status::OK();
+ }
+ case arrow::Type::LIST:
+ return ValidateLanceDataField(type->field(0));
+ case arrow::Type::STRUCT:
+ for (const std::shared_ptr<arrow::Field>& child : type->fields()) {
+ PAIMON_RETURN_NOT_OK(ValidateLanceDataField(child));
+ }
+ return Status::OK();
Review Comment:
Nullable structs cannot be accepted here. This PR already skips
`TestWriteWithNestedSchema` because Lance 0.39 v2.0 drops the parent validity
bitmap, so a null struct round-trips as a non-null struct with null children
and silently changes the data. Reject nullable `STRUCT` fields recursively, or
use an encoding that preserves parent validity.
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -802,6 +804,73 @@ Status SchemaValidation::ValidateMosaicDataFields(const
TableSchema& schema,
return Status::OK();
}
+Status SchemaValidation::ValidateLanceDataField(const
std::shared_ptr<arrow::Field>& field) {
+ if (VariantTypeUtils::IsVariantField(field)) {
+ return Status::Invalid("Lance file format does not support type
VARIANT");
+ }
+ if (BlobUtils::IsBlobField(field)) {
+ return Status::Invalid("Lance file format does not support type BLOB");
+ }
+
+ const std::shared_ptr<arrow::DataType>& type = field->type();
+ switch (type->id()) {
+ case arrow::Type::BOOL:
+ case arrow::Type::INT8:
+ case arrow::Type::INT16:
+ case arrow::Type::INT32:
+ case arrow::Type::INT64:
+ case arrow::Type::FLOAT:
+ case arrow::Type::DOUBLE:
+ case arrow::Type::DATE32:
+ case arrow::Type::STRING:
+ case arrow::Type::BINARY:
+ case arrow::Type::TIME32:
+ case arrow::Type::DECIMAL128:
+ case arrow::Type::FIXED_SIZE_LIST:
+ return Status::OK();
+ case arrow::Type::TIMESTAMP: {
+ const auto& timestamp_type = checked_cast<const
arrow::TimestampType&>(*type);
+ if (!timestamp_type.timezone().empty()) {
+ return Status::Invalid(
+ "Lance file format does not support type
LOCAL_ZONED_TIMESTAMP");
+ }
+ return Status::OK();
+ }
+ case arrow::Type::LIST:
+ return ValidateLanceDataField(type->field(0));
+ case arrow::Type::STRUCT:
+ for (const std::shared_ptr<arrow::Field>& child : type->fields()) {
+ PAIMON_RETURN_NOT_OK(ValidateLanceDataField(child));
+ }
+ return Status::OK();
+ case arrow::Type::MAP:
+ return Status::Invalid("Lance file format does not support type
MAP");
+ default:
+ break;
+ }
+ return Status::Invalid(
+ fmt::format("Lance file format does not support type {}",
type->ToString()));
+}
+
+Status SchemaValidation::ValidateLanceDataFields(const TableSchema& schema,
+ const CoreOptions& options) {
+ if (StringUtils::ToLowerCase(options.GetFileFormat()->Identifier()) !=
"lance") {
Review Comment:
This only validates the default `file.format`. A table using
`file.format.per.level=...:lance` or `changelog-file.format=lance` can
therefore pass validation with unsupported types such as MAP, then fail later
during compaction or changelog writing. Apply the Lance type checks to every
configured write format.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
