wgtmac commented on code in PR #127:
URL: https://github.com/apache/iceberg-cpp/pull/127#discussion_r2203784355
##########
test/avro_schema_test.cc:
##########
@@ -22,10 +22,15 @@
#include <avro/Compiler.hh>
#include <avro/NodeImpl.hh>
#include <avro/Types.hh>
+#include <gmock/gmock.h>
Review Comment:
Please revert (unrelated) changes in this file.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
Review Comment:
```suggestion
if (i >= original_node->names()) {
return InvalidSchema(...);
}
const std::string& field_name = original_node->nameAt(i);
```
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
Review Comment:
```suggestion
new_record_node->addCustomAttributesForField(attributes);
```
This check is unnecessary.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
+ }
+ }
+
+ return new_record_node;
+}
+
+Result<::avro::NodePtr> CreateArrayNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 1) {
+ return InvalidSchema("Array type must have exactly one leaf");
+ }
+
+ auto new_array_node = std::make_shared<::avro::NodeArray>();
+
+ // Check if this is a map represented as array
+ if (HasMapLogicalType(original_node)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto new_element_node,
+
CreateAvroNodeWithFieldIds(original_node->leafAt(0), field));
+ new_array_node->addLeaf(new_element_node);
+ return new_array_node;
+ }
+
+ // For regular arrays, try to find element field ID from nested mapping
+ const MappedField* element_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(std::string(kElement)) != f.names.end()) {
+ element_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (element_field) {
+ // Check if field_id is present
+ if (!element_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for element field in array");
+ }
+
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto new_element_node,
+ CreateAvroNodeWithFieldIds(original_node->leafAt(0), *element_field));
+ new_array_node->addLeaf(new_element_node);
Review Comment:
Shouldn't we add `element_field->field_id` to the custom attributes?
##########
test/name_mapping_test.cc:
##########
@@ -299,3 +361,436 @@ TEST(CreateMappingTest, ListSchemaToMapping) {
}
} // namespace iceberg
+
+// NameMapping tests for Avro schema context
+namespace iceberg::avro {
Review Comment:
Ah, test cases below this line should be in the avro_schema_test.cc.
##########
src/iceberg/avro/avro_reader.cc:
##########
@@ -91,16 +92,43 @@ class AvroBatchReader::Impl {
// Create a base reader without setting reader schema to enable projection.
auto base_reader =
std::make_unique<::avro::DataFileReaderBase>(std::move(input_stream));
- const ::avro::ValidSchema& file_schema = base_reader->dataSchema();
+ ::avro::ValidSchema file_schema = base_reader->dataSchema();
// Validate field ids in the file schema.
HasIdVisitor has_id_visitor;
ICEBERG_RETURN_UNEXPECTED(has_id_visitor.Visit(file_schema));
+
if (has_id_visitor.HasNoIds()) {
- // TODO(gangwu): support applying field-ids based on name mapping
- return NotImplemented("Avro file schema has no field IDs");
- }
- if (!has_id_visitor.AllHaveIds()) {
+ // Apply field IDs based on name mapping if available
+ if (options.name_mapping) {
+ MappedField mapped_field;
+ // Convert NameMapping to MappedFields for nested mapping
+ mapped_field.nested_mapping =
+
std::make_shared<MappedFields>(options.name_mapping->AsMappedFields());
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto new_root_node,
+ CreateAvroNodeWithFieldIds(file_schema.root(), mapped_field));
+
+ // Create a new schema with the updated root node
+ auto new_schema = ::avro::ValidSchema(new_root_node);
+
+ // Verify that all fields now have IDs after applying the name mapping
+ HasIdVisitor verify_visitor;
+ ICEBERG_RETURN_UNEXPECTED(verify_visitor.Visit(new_schema));
+ if (!verify_visitor.AllHaveIds()) {
Review Comment:
I think we can remove this extra check now. Your implementation can
guarantee all fields are processed.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
Review Comment:
```suggestion
attributes.addAttribute(attr_pair.first, attr_pair.second,
/*addQuote=*/false);
```
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
Review Comment:
```suggestion
if (i >= original_node->leaves()) {
return InvalidSchema(...);
}
::avro::NodePtr field_node = original_node->leafAt(i);
```
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
+ }
+ }
+
+ return new_record_node;
+}
+
+Result<::avro::NodePtr> CreateArrayNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 1) {
+ return InvalidSchema("Array type must have exactly one leaf");
+ }
+
+ auto new_array_node = std::make_shared<::avro::NodeArray>();
+
+ // Check if this is a map represented as array
+ if (HasMapLogicalType(original_node)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto new_element_node,
+
CreateAvroNodeWithFieldIds(original_node->leafAt(0), field));
+ new_array_node->addLeaf(new_element_node);
+ return new_array_node;
+ }
+
+ // For regular arrays, try to find element field ID from nested mapping
+ const MappedField* element_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(std::string(kElement)) != f.names.end()) {
Review Comment:
Do we actually need to find the field name of a map or array? IIUC, the
field name mapping should only apply to struct fields. We can blindly use the
child nested_mapping.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
Review Comment:
```suggestion
```
I don't think we need to check this. I meant to add suggested changes below.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
Review Comment:
```suggestion
const MappedField* nested_field = nullptr;
auto field_id_opt = field.Id(field_name);
if (field_id_opt.has_value()) {
nested_field = &(field.Field(field_id_opt.value()).value().get());
}
```
This is not a required suggestion but I just found that the refactoring
above does the same thing. Perhaps we should extend `MappedFields` by adding a
new function to return a `std::optional<MappedFieldConstRef>` by finding a
field name.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
Review Comment:
```suggestion
for (const auto& attr_pair :
original_node->customAttributesAt(i).attributes()) {
```
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
Review Comment:
It is much better to return error immediately after we fail to find the
field id. Then we don't need the large if branch at line 817.
##########
test/name_mapping_test.cc:
##########
@@ -24,8 +24,18 @@
#include <unordered_set>
#include <vector>
+#include <avro/Compiler.hh>
+#include <avro/NodeImpl.hh>
+#include <avro/Types.hh>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <nlohmann/json.hpp>
Review Comment:
We don't need this, isn't it?
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
+ }
+ }
+
+ return new_record_node;
+}
+
+Result<::avro::NodePtr> CreateArrayNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 1) {
+ return InvalidSchema("Array type must have exactly one leaf");
+ }
+
+ auto new_array_node = std::make_shared<::avro::NodeArray>();
+
+ // Check if this is a map represented as array
+ if (HasMapLogicalType(original_node)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto new_element_node,
+
CreateAvroNodeWithFieldIds(original_node->leafAt(0), field));
+ new_array_node->addLeaf(new_element_node);
+ return new_array_node;
+ }
+
+ // For regular arrays, try to find element field ID from nested mapping
+ const MappedField* element_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(std::string(kElement)) != f.names.end()) {
+ element_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (element_field) {
+ // Check if field_id is present
+ if (!element_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for element field in array");
+ }
+
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto new_element_node,
+ CreateAvroNodeWithFieldIds(original_node->leafAt(0), *element_field));
+ new_array_node->addLeaf(new_element_node);
+ } else {
+ // If no element field found, this is an error
+ return InvalidSchema("Element field not found in nested mapping for
array");
Review Comment:
Ditto
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
+ }
+ }
+
+ return new_record_node;
+}
+
+Result<::avro::NodePtr> CreateArrayNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 1) {
+ return InvalidSchema("Array type must have exactly one leaf");
+ }
+
+ auto new_array_node = std::make_shared<::avro::NodeArray>();
+
+ // Check if this is a map represented as array
+ if (HasMapLogicalType(original_node)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto new_element_node,
+
CreateAvroNodeWithFieldIds(original_node->leafAt(0), field));
+ new_array_node->addLeaf(new_element_node);
+ return new_array_node;
+ }
+
+ // For regular arrays, try to find element field ID from nested mapping
+ const MappedField* element_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(std::string(kElement)) != f.names.end()) {
+ element_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (element_field) {
+ // Check if field_id is present
+ if (!element_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for element field in array");
+ }
+
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto new_element_node,
+ CreateAvroNodeWithFieldIds(original_node->leafAt(0), *element_field));
+ new_array_node->addLeaf(new_element_node);
+ } else {
+ // If no element field found, this is an error
+ return InvalidSchema("Element field not found in nested mapping for
array");
+ }
+
+ return new_array_node;
+}
+
+Result<::avro::NodePtr> CreateMapNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 2) {
+ return InvalidSchema("Map type must have exactly two leaves");
+ }
+
+ auto new_map_node = std::make_shared<::avro::NodeMap>();
+
+ // Try to find key and value fields from nested mapping
+ const MappedField* key_field = nullptr;
+ const MappedField* value_field = nullptr;
+ if (field.nested_mapping) {
Review Comment:
ditto, I think we can blindly get field ids for key and value fields.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
Review Comment:
For better readability.
##########
test/name_mapping_test.cc:
##########
@@ -299,3 +361,436 @@ TEST(CreateMappingTest, ListSchemaToMapping) {
}
} // namespace iceberg
+
+// NameMapping tests for Avro schema context
+namespace iceberg::avro {
+
+namespace {
+
+void CheckFieldIdAt(const ::avro::NodePtr& node, size_t index, int32_t
field_id,
+ const std::string& key = "field-id") {
+ ASSERT_LT(index, node->customAttributes());
+ const auto& attrs = node->customAttributesAt(index);
+ ASSERT_EQ(attrs.getAttribute(key),
std::make_optional(std::to_string(field_id)));
+}
+
+// Helper function to create a test name mapping
+std::unique_ptr<NameMapping> CreateTestNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+ fields.emplace_back(MappedField{.names = {"name"}, .field_id = 2});
+
+ // Create nested mapping for the data field
+ std::vector<MappedField> nested_fields;
+ nested_fields.emplace_back(MappedField{.names = {"value"}, .field_id = 3});
+ nested_fields.emplace_back(MappedField{.names = {"description"}, .field_id =
4});
+ auto nested_mapping = MappedFields::Make(std::move(nested_fields));
+
+ fields.emplace_back(MappedField{
+ .names = {"data"}, .field_id = 5, .nested_mapping =
std::move(nested_mapping)});
+
+ return NameMapping::Make(std::move(fields));
+}
+
+} // namespace
+
+class NameMappingAvroSchemaTest : public ::testing::Test {
+ protected:
+ // Helper function to create a simple name mapping
+ std::unique_ptr<NameMapping> CreateSimpleNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+ fields.emplace_back(MappedField{.names = {"name"}, .field_id = 2});
+ fields.emplace_back(MappedField{.names = {"age"}, .field_id = 3});
+ return NameMapping::Make(std::move(fields));
+ }
+
+ // Helper function to create a nested name mapping
+ std::unique_ptr<NameMapping> CreateNestedNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+
+ // Nested mapping for address
+ std::vector<MappedField> address_fields;
+ address_fields.emplace_back(MappedField{.names = {"street"}, .field_id =
10});
+ address_fields.emplace_back(MappedField{.names = {"city"}, .field_id =
11});
+ address_fields.emplace_back(MappedField{.names = {"zip"}, .field_id = 12});
+ auto address_mapping = MappedFields::Make(std::move(address_fields));
+
+ fields.emplace_back(MappedField{.names = {"address"},
+ .field_id = 2,
+ .nested_mapping =
std::move(address_mapping)});
+
+ return NameMapping::Make(std::move(fields));
+ }
+
+ // Helper function to create a name mapping for array types
+ std::unique_ptr<NameMapping> CreateArrayNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+
+ // Nested mapping for array element
+ std::vector<MappedField> element_fields;
+ element_fields.emplace_back(MappedField{.names = {"element"}, .field_id =
20});
+ auto element_mapping = MappedFields::Make(std::move(element_fields));
+
+ fields.emplace_back(MappedField{
+ .names = {"items"}, .field_id = 2, .nested_mapping =
std::move(element_mapping)});
+
+ return NameMapping::Make(std::move(fields));
+ }
+
+ // Helper function to create a name mapping for map types
+ std::unique_ptr<NameMapping> CreateMapNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+
+ // Nested mapping for map key-value
+ std::vector<MappedField> map_fields;
+ map_fields.emplace_back(MappedField{.names = {"key"}, .field_id = 30});
+ map_fields.emplace_back(MappedField{.names = {"value"}, .field_id = 31});
+ auto map_mapping = MappedFields::Make(std::move(map_fields));
+
+ fields.emplace_back(MappedField{.names = {"properties"},
+ .field_id = 2,
+ .nested_mapping = std::move(map_mapping)});
+
+ return NameMapping::Make(std::move(fields));
+ }
+
+ // Helper function to create a name mapping for union types
+ std::unique_ptr<NameMapping> CreateUnionNameMapping() {
+ std::vector<MappedField> fields;
+ fields.emplace_back(MappedField{.names = {"id"}, .field_id = 1});
+ fields.emplace_back(MappedField{.names = {"data"}, .field_id = 2});
+ return NameMapping::Make(std::move(fields));
+ }
+};
+
+TEST_F(NameMappingAvroSchemaTest, ApplyNameMappingToRecord) {
+ // Create a simple Avro record schema without field IDs
+ std::string avro_schema_json = R"({
+ "type": "record",
+ "name": "test_record",
+ "fields": [
+ {"name": "id", "type": "int"},
+ {"name": "name", "type": "string"},
+ {"name": "age", "type": "int"}
+ ]
+ })";
+ auto avro_schema = ::avro::compileJsonSchemaFromString(avro_schema_json);
+
+ auto name_mapping = CreateSimpleNameMapping();
+ MappedField mapped_field;
+ mapped_field.nested_mapping =
+ std::make_shared<MappedFields>(name_mapping->AsMappedFields());
+
+ auto result = CreateAvroNodeWithFieldIds(avro_schema.root(), mapped_field);
+ ASSERT_THAT(result, IsOk());
+
+ const auto& new_node = *result;
+ EXPECT_EQ(new_node->type(), ::avro::AVRO_RECORD);
+ EXPECT_EQ(new_node->names(), 3);
+ EXPECT_EQ(new_node->leaves(), 3);
+
+ // Check that field IDs are properly applied
+ ASSERT_EQ(new_node->customAttributes(), 3);
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(new_node, 0, 1));
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(new_node, 1, 2));
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(new_node, 2, 3));
+}
+
+TEST_F(NameMappingAvroSchemaTest, ApplyNameMappingToNestedRecord) {
+ // Create a nested Avro record schema without field IDs
+ std::string avro_schema_json = R"({
+ "type": "record",
+ "name": "test_record",
+ "fields": [
+ {"name": "id", "type": "int"},
+ {"name": "address", "type": {
+ "type": "record",
+ "name": "address",
+ "fields": [
+ {"name": "street", "type": "string"},
+ {"name": "city", "type": "string"},
+ {"name": "zip", "type": "string"}
+ ]
+ }}
+ ]
+ })";
+ auto avro_schema = ::avro::compileJsonSchemaFromString(avro_schema_json);
+
+ auto name_mapping = CreateNestedNameMapping();
+ MappedField mapped_field;
+ mapped_field.nested_mapping =
+ std::make_shared<MappedFields>(name_mapping->AsMappedFields());
+
+ auto result = CreateAvroNodeWithFieldIds(avro_schema.root(), mapped_field);
+ ASSERT_THAT(result, IsOk());
+
+ const auto& new_node = *result;
+ EXPECT_EQ(new_node->type(), ::avro::AVRO_RECORD);
+ EXPECT_EQ(new_node->names(), 2);
+ EXPECT_EQ(new_node->leaves(), 2);
+
+ // Check that field IDs are properly applied to top-level fields
+ ASSERT_EQ(new_node->customAttributes(), 2);
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(new_node, 0, 1));
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(new_node, 1, 2));
+
+ // Check nested record
+ const auto& address_node = new_node->leafAt(1);
+ EXPECT_EQ(address_node->type(), ::avro::AVRO_RECORD);
+ EXPECT_EQ(address_node->names(), 3);
+ EXPECT_EQ(address_node->leaves(), 3);
+
+ // Check that field IDs are properly applied to nested fields
+ ASSERT_EQ(address_node->customAttributes(), 3);
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(address_node, 0, 10));
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(address_node, 1, 11));
+ ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(address_node, 2, 12));
+}
+
+TEST_F(NameMappingAvroSchemaTest, ApplyNameMappingToArray) {
+ try {
+ // Create an Avro array schema without field IDs
+ std::string avro_schema_json = R"({
+ "type": "record",
+ "name": "test_record",
+ "fields": [
+ {"name": "id", "type": "int"},
+ {"name": "items", "type": {
+ "type": "array",
+ "items": "string"
+ }}
+ ]
+ })";
+ auto avro_schema = ::avro::compileJsonSchemaFromString(avro_schema_json);
+
+ auto name_mapping = CreateArrayNameMapping();
+ MappedField mapped_field;
+ mapped_field.nested_mapping =
+ std::make_shared<MappedFields>(name_mapping->AsMappedFields());
+
+ auto result = CreateAvroNodeWithFieldIds(avro_schema.root(), mapped_field);
+ ASSERT_THAT(result, IsOk());
+
+ const auto& new_node = *result;
+ EXPECT_EQ(new_node->type(), ::avro::AVRO_RECORD);
+ EXPECT_EQ(new_node->names(), 2);
+ EXPECT_EQ(new_node->leaves(), 2);
+
+ // Check array field structure only - don't access any attributes
+ const auto& array_node = new_node->leafAt(1);
+ EXPECT_EQ(array_node->type(), ::avro::AVRO_ARRAY);
+ EXPECT_EQ(array_node->leaves(), 1);
+
+ // Note: Array nodes don't support custom attributes in Avro C++
+ // We only verify the structure is correct, not the attributes
+ } catch (const std::exception& e) {
+ // If we get an exception about attributes not being supported, that's
expected
+ // for array nodes in Avro C++
+ EXPECT_TRUE(std::string(e.what()).find("This type does not have
attribute") !=
Review Comment:
Please fix test cases like this.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -783,4 +785,237 @@ Result<SchemaProjection> Project(const Schema&
expected_schema,
return SchemaProjection{std::move(field_projection.children)};
}
+namespace {
+
+Result<::avro::NodePtr> CreateRecordNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field)
{
+ auto new_record_node = std::make_shared<::avro::NodeRecord>();
+ new_record_node->setName(original_node->name());
+
+ if (original_node->leaves() > original_node->names()) {
+ return InvalidSchema("Node has {} leaves but only {} names",
original_node->leaves(),
+ original_node->names());
+ }
+
+ for (size_t i = 0; i < original_node->leaves(); ++i) {
+ const std::string& field_name = original_node->nameAt(i);
+ ::avro::NodePtr field_node = original_node->leafAt(i);
+
+ // TODO(liuxiaoyu): Add support for case sensitivity in name matching.
+ // Try to find nested field by name
+ const MappedField* nested_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(field_name) != f.names.end()) {
+ nested_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (nested_field) {
+ // Check if field_id is present
+ if (!nested_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for field '{}' in nested
mapping",
+ field_name);
+ }
+
+ // Preserve existing custom attributes for this field
+ ::avro::CustomAttributes attributes;
+ if (i < original_node->customAttributes()) {
+ // Copy all existing attributes from the original node
+ const auto& original_attrs = original_node->customAttributesAt(i);
+ const auto& existing_attrs = original_attrs.attributes();
+ for (const auto& attr_pair : existing_attrs) {
+ attributes.addAttribute(attr_pair.first, attr_pair.second, false);
+ }
+ }
+
+ // Add field ID attribute to the new node (preserving existing
attributes)
+ attributes.addAttribute(std::string(kFieldIdProp),
+ std::to_string(nested_field->field_id.value()),
false);
+
+ if (!attributes.attributes().empty()) {
+ new_record_node->addCustomAttributesForField(attributes);
+ }
+
+ // Recursively apply field IDs to nested fields
+ ICEBERG_ASSIGN_OR_RAISE(auto new_nested_node,
+ CreateAvroNodeWithFieldIds(field_node,
*nested_field));
+ new_record_node->addName(field_name);
+ new_record_node->addLeaf(new_nested_node);
+ } else {
+ // If no nested field found, this is an error
+ return InvalidSchema("Field '{}' not found in nested mapping",
field_name);
+ }
+ }
+
+ return new_record_node;
+}
+
+Result<::avro::NodePtr> CreateArrayNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 1) {
+ return InvalidSchema("Array type must have exactly one leaf");
+ }
+
+ auto new_array_node = std::make_shared<::avro::NodeArray>();
+
+ // Check if this is a map represented as array
+ if (HasMapLogicalType(original_node)) {
+ ICEBERG_ASSIGN_OR_RAISE(auto new_element_node,
+
CreateAvroNodeWithFieldIds(original_node->leafAt(0), field));
+ new_array_node->addLeaf(new_element_node);
+ return new_array_node;
+ }
+
+ // For regular arrays, try to find element field ID from nested mapping
+ const MappedField* element_field = nullptr;
+ if (field.nested_mapping) {
+ auto fields_span = field.nested_mapping->fields();
+ for (const auto& f : fields_span) {
+ if (f.names.find(std::string(kElement)) != f.names.end()) {
+ element_field = &f;
+ break;
+ }
+ }
+ }
+
+ if (element_field) {
+ // Check if field_id is present
+ if (!element_field->field_id.has_value()) {
+ return InvalidSchema("Field ID is missing for element field in array");
+ }
+
+ ICEBERG_ASSIGN_OR_RAISE(
+ auto new_element_node,
+ CreateAvroNodeWithFieldIds(original_node->leafAt(0), *element_field));
+ new_array_node->addLeaf(new_element_node);
+ } else {
+ // If no element field found, this is an error
+ return InvalidSchema("Element field not found in nested mapping for
array");
+ }
+
+ return new_array_node;
+}
+
+Result<::avro::NodePtr> CreateMapNodeWithFieldIds(const ::avro::NodePtr&
original_node,
+ const MappedField& field) {
+ if (original_node->leaves() != 2) {
+ return InvalidSchema("Map type must have exactly two leaves");
+ }
+
+ auto new_map_node = std::make_shared<::avro::NodeMap>();
+
+ // Try to find key and value fields from nested mapping
+ const MappedField* key_field = nullptr;
+ const MappedField* value_field = nullptr;
+ if (field.nested_mapping) {
Review Comment:
BTW, these ids are not added to the attributes below.
--
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]
