rdblue commented on a change in pull request #207: Add external schema mappings 
for files written with name-based schemas #40
URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r333141603
 
 

 ##########
 File path: core/src/main/java/org/apache/iceberg/avro/AvroSchemaUtil.java
 ##########
 @@ -213,23 +239,66 @@ public static int getKeyId(Schema schema) {
     return getId(schema, KEY_ID_PROP);
   }
 
+  static Integer getKeyId(Schema schema, NameMapping nameMapping, 
Iterable<String> parentFieldNames) {
+    Preconditions.checkArgument(schema.getType() == MAP,
+        "Cannot get map key id for non-map schema: %s", schema);
+    List<String> names = Lists.newArrayList(parentFieldNames);
+    names.add("key");
+    return getId(schema, KEY_ID_PROP, nameMapping, names);
+  }
+
   public static int getValueId(Schema schema) {
     Preconditions.checkArgument(schema.getType() == MAP,
         "Cannot get map value id for non-map schema: %s", schema);
     return getId(schema, VALUE_ID_PROP);
   }
 
+  static Integer getValueId(Schema schema, NameMapping nameMapping, 
Iterable<String> parentFieldNames) {
+    Preconditions.checkArgument(schema.getType() == MAP,
+        "Cannot get map value id for non-map schema: %s", schema);
+    List<String> names = Lists.newArrayList(parentFieldNames);
+    names.add("value");
+    return getId(schema, VALUE_ID_PROP, nameMapping, names);
+  }
+
   public static int getElementId(Schema schema) {
     Preconditions.checkArgument(schema.getType() == ARRAY,
         "Cannot get array element id for non-array schema: %s", schema);
     return getId(schema, ELEMENT_ID_PROP);
   }
 
+  static Integer getElementId(Schema schema, NameMapping nameMapping, 
Iterable<String> parentFieldNames) {
+    Preconditions.checkArgument(schema.getType() == ARRAY,
+        "Cannot get array element id for non-array schema: %s", schema);
+    List<String> names = Lists.newArrayList(parentFieldNames);
+    names.add("element");
+    return getId(schema, ELEMENT_ID_PROP, nameMapping, names);
+  }
+
   public static int getFieldId(Schema.Field field) {
-    Object id = field.getObjectProp(FIELD_ID_PROP);
+    Integer id = getFieldId(field, null, null);
     Preconditions.checkNotNull(id, "Missing expected '%s' property", 
FIELD_ID_PROP);
+    return id;
+  }
+
+  static Integer getFieldId(Schema.Field field, NameMapping nameMapping, 
Iterable<String> parentFieldNames) {
+    Object id = field.getObjectProp(FIELD_ID_PROP);
+    if (id != null) {
+      return toInt(id);
+    } else {
+      Preconditions.checkArgument(nameMapping != null, "Field schema does not 
have id and name mapping not specified");
 
 Review comment:
   I think the same logic applies here, so this should not throw an exception. 
Instead, use `else if (nameMapping != null)` and return null if the object 
property is null or if no mapping ID is available.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to