wgtmac commented on code in PR #127:
URL: https://github.com/apache/iceberg-cpp/pull/127#discussion_r2218124851
##########
test/avro_schema_test.cc:
##########
@@ -23,9 +23,13 @@
#include <avro/NodeImpl.hh>
#include <avro/Types.hh>
#include <gtest/gtest.h>
+#include <nlohmann/json.hpp>
Review Comment:
```suggestion
```
##########
test/CMakeLists.txt:
##########
@@ -30,8 +30,7 @@ configure_file("${CMAKE_SOURCE_DIR}/test/test_config.h.in"
add_executable(schema_test)
target_sources(schema_test
- PRIVATE name_mapping_test.cc
- schema_test.cc
+ PRIVATE schema_test.cc
Review Comment:
Please revert changes in this file.
##########
test/name_mapping_test.cc:
##########
@@ -24,9 +24,18 @@
#include <unordered_set>
#include <vector>
+#include <avro/Compiler.hh>
+#include <avro/NodeImpl.hh>
+#include <avro/Types.hh>
Review Comment:
```suggestion
```
##########
test/avro_schema_test.cc:
##########
@@ -23,9 +23,13 @@
#include <avro/NodeImpl.hh>
#include <avro/Types.hh>
#include <gtest/gtest.h>
+#include <nlohmann/json.hpp>
+#include "iceberg/avro/avro_reader.h"
#include "iceberg/avro/avro_schema_util_internal.h"
+#include "iceberg/json_internal.h"
Review Comment:
```suggestion
```
##########
test/avro_schema_test.cc:
##########
@@ -1057,4 +1079,383 @@ TEST(AvroSchemaProjectionTest,
ProjectDecimalIncompatible) {
ASSERT_THAT(projection_result, HasErrorMessage("Cannot read"));
}
+// NameMapping tests for Avro schema context
+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) {
+ // 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);
+}
+
+TEST_F(NameMappingAvroSchemaTest, ApplyNameMappingToMap) {
+ // Create an Avro map schema without field IDs
+ std::string avro_schema_json = R"({
+ "type": "record",
+ "name": "test_record",
+ "fields": [
+ {"name": "id", "type": "int"},
+ {"name": "properties", "type": {
+ "type": "map",
+ "values": "string"
+ }}
+ ]
+ })";
+ auto avro_schema = ::avro::compileJsonSchemaFromString(avro_schema_json);
+
+ auto name_mapping = CreateMapNameMapping();
+ 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 map field structure only - don't access any attributes
Review Comment:
Same questions here.
##########
test/avro_schema_test.cc:
##########
@@ -23,9 +23,13 @@
#include <avro/NodeImpl.hh>
#include <avro/Types.hh>
#include <gtest/gtest.h>
+#include <nlohmann/json.hpp>
+#include "iceberg/avro/avro_reader.h"
Review Comment:
```suggestion
```
##########
test/avro_schema_test.cc:
##########
@@ -46,6 +50,24 @@ void CheckFieldIdAt(const ::avro::NodePtr& node, size_t
index, int32_t field_id,
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));
+}
+
Review Comment:
```suggestion
```
I don't see it is used anywhere.
##########
test/avro_schema_test.cc:
##########
@@ -1057,4 +1079,383 @@ TEST(AvroSchemaProjectionTest,
ProjectDecimalIncompatible) {
ASSERT_THAT(projection_result, HasErrorMessage("Cannot read"));
}
+// NameMapping tests for Avro schema context
+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) {
+ // 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
Review Comment:
Why not checking the ids?
##########
src/iceberg/avro/avro_schema_util_internal.h:
##########
@@ -144,4 +145,11 @@ std::string ToString(const ::avro::LogicalType::Type&
logical_type);
/// \return True if the node has a map logical type, false otherwise.
bool HasMapLogicalType(const ::avro::NodePtr& node);
+/// \brief Create a new Avro node with field IDs from name mapping.
+/// \param original_node The original Avro node to copy.
+/// \param mapped_field The mapped field to apply field IDs from.
+/// \return A new Avro node with field IDs applied, or an error.
+Result<::avro::NodePtr> CreateAvroNodeWithFieldIds(const ::avro::NodePtr&
original_node,
Review Comment:
nit: we can add the following convenience function so callers (e.g.
AvroReader and test cases) don't need to convert from NameMapping to
MappedField every time:
```
Result<::avro::NodePtr> CreateAvroNodeWithFieldIds(const ::avro::NodePtr&
original_node,
const NameMapping&
mapping);
```
##########
test/avro_schema_test.cc:
##########
@@ -1057,4 +1079,383 @@ TEST(AvroSchemaProjectionTest,
ProjectDecimalIncompatible) {
ASSERT_THAT(projection_result, HasErrorMessage("Cannot read"));
}
+// NameMapping tests for Avro schema context
+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));
Review Comment:
```suggestion
ASSERT_NO_FATAL_FAILURE(CheckFieldIdAt(node, /*index=*/0, /*field_id=*/1));
```
For better readability. Same for all calls to `CheckFieldIdAt` below.
##########
test/name_mapping_test.cc:
##########
@@ -24,9 +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 "iceberg/avro/avro_reader.h"
+#include "iceberg/avro/avro_schema_util_internal.h"
+#include "iceberg/file_reader.h"
+#include "iceberg/json_internal.h"
+#include "matchers.h"
+
Review Comment:
```suggestion
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
