MisterRaindrop commented on code in PR #127:
URL: https://github.com/apache/iceberg-cpp/pull/127#discussion_r2177655020


##########
src/iceberg/avro/avro_reader.cc:
##########
@@ -96,11 +99,17 @@ class AvroBatchReader::Impl {
     // 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) {
+        
ICEBERG_RETURN_UNEXPECTED(ApplyFieldIdsFromNameMapping(*options.name_mapping,

Review Comment:
   When the verification of verify_visitor.AllHaveIds() fails, should we print 
detailed information such as…
   ```
   return InvalidSchema("Not all fields have field IDs after applying name 
mapping. {}", 
                                 verify_visitor.GetMissingFieldIdsInfo());
   ```
   get detailed information or simple 
   ```
   return InvalidSchema("Not all fields have field IDs after applying name 
mapping")
   ```
   
   



-- 
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]

Reply via email to