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_r333139717
##########
File path: core/src/main/java/org/apache/iceberg/avro/AvroSchemaUtil.java
##########
@@ -196,15 +199,38 @@ static Schema createProjectionMap(String recordName,
return LogicalMap.get().addToSchema(Schema.createArray(keyValueRecord));
}
- private static int getId(Schema schema, String propertyName) {
+
+ static Integer getId(Schema schema, String propertyName) {
+ Integer id = getId(schema, propertyName, null, null);
+ Preconditions.checkNotNull(id, "Missing expected '%s' property",
propertyName);
+ return id;
+ }
+
+ static Integer getId(Schema schema, String propertyName,
+ NameMapping nameMapping, List<String> names) {
if (schema.getType() == UNION) {
- return getId(fromOption(schema), propertyName);
+ return getId(fromOption(schema), propertyName, nameMapping, names);
}
Object id = schema.getObjectProp(propertyName);
- Preconditions.checkNotNull(id, "Missing expected '%s' property",
propertyName);
+ if (id != null) {
+ return toInt(id);
+ } else {
+ Preconditions.checkArgument(nameMapping != null,
+ "Schema does not have field id and name mapping not specified");
Review comment:
I don't think this should thrown an exception. If the name mapping isn't
present or doesn't have the given field, then this should return null so that
the field is pruned.
I think it should be like this:
```java
private static Integer getId(Schema schema, String propertyName,
NameMapping nameMapping, List<String> names) {
if (schema.getType() == UNION) {
return getId(fromOption(schema), propertyName, nameMapping, names);
}
Object id = schema.getObjectProp(propertyName);
if (id != null) {
return toInt(id);
} else if (nameMapping != null) {
MappedField mappedField = nameMapping.find(names);
if (mappedField != null) {
return mappedField.id();
}
}
return null;
}
```
----------------------------------------------------------------
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]
