wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3355222807
##########
src/iceberg/schema.cc:
##########
@@ -35,6 +35,45 @@
namespace iceberg {
+namespace {
+
+Status ValidateUnknownFieldsOptional(const Type& type) {
+ switch (type.type_id()) {
+ case TypeId::kStruct: {
+ const auto& struct_type = static_cast<const StructType&>(type);
+ for (const auto& field : struct_type.fields()) {
+ ICEBERG_PRECHECK(field.optional() || field.type()->type_id() !=
TypeId::kUnknown,
+ "Unknown type field '{}' must be optional",
field.name());
+
ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldsOptional(*field.type()));
+ }
+ return {};
+ }
+ case TypeId::kList: {
+ const auto& list_type = static_cast<const ListType&>(type);
+ const auto& element = list_type.element();
+ ICEBERG_PRECHECK(
+ element.optional() || element.type()->type_id() != TypeId::kUnknown,
+ "Unknown type field '{}' must be optional", element.name());
+ return ValidateUnknownFieldsOptional(*element.type());
+ }
+ case TypeId::kMap: {
+ const auto& map_type = static_cast<const MapType&>(type);
+ const auto& key = map_type.key();
+ const auto& value = map_type.value();
+ ICEBERG_PRECHECK(key.type()->type_id() != TypeId::kUnknown,
+ "Map 'key' cannot be unknown type");
+ ICEBERG_PRECHECK(value.optional() || value.type()->type_id() !=
TypeId::kUnknown,
+ "Unknown type field '{}' must be optional",
value.name());
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldsOptional(*key.type()));
+ return ValidateUnknownFieldsOptional(*value.type());
+ }
+ default:
+ return {};
+ }
+}
Review Comment:
```suggestion
Status ValidateFieldNullability(const Type& type) {
auto validate_field = [&](const SchemaField& field) -> Status {
ICEBERG_PRECHECK(field.optional() || field.type()->type_id() !=
TypeId::kUnknown,
"Unknown type field '{}' must be optional",
field.name());
return ValidateFieldNullability(*field.type());
};
switch (type.type_id()) {
case TypeId::kStruct: {
const auto& struct_type = static_cast<const StructType&>(type);
for (const auto& field : struct_type.fields()) {
ICEBERG_RETURN_UNEXPECTED(validate_field(field));
}
return {};
}
case TypeId::kList: {
const auto& list_type = static_cast<const ListType&>(type);
const auto& element = list_type.element();
return validate_field(element);
}
case TypeId::kMap: {
const auto& map_type = static_cast<const MapType&>(type);
const auto& key = map_type.key();
const auto& value = map_type.value();
ICEBERG_PRECHECK(key.type()->type_id() != TypeId::kUnknown,
"Map 'key' cannot be unknown type");
ICEBERG_RETURN_UNEXPECTED(ValidateFieldNullability(*key.type()));
return validate_field(value);
}
default:
return {};
}
}
```
I'd rather rewriting it as the above. In the next step, we can reuse it for
checking v3 default values.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
