rdblue commented on a change in pull request #199:
URL: https://github.com/apache/iceberg/pull/199#discussion_r433563926
##########
File path: orc/src/main/java/org/apache/iceberg/orc/ORCSchemaUtil.java
##########
@@ -377,15 +377,15 @@ private static boolean isSameType(TypeDescription
orcType, Type icebergType) {
}
}
- private static Optional<Integer> icebergID(TypeDescription orcType) {
+ static Optional<Integer> icebergID(TypeDescription orcType) {
return Optional.ofNullable(orcType.getAttributeValue(ICEBERG_ID_ATTRIBUTE))
.map(Integer::parseInt);
}
static int fieldId(TypeDescription orcType) {
- String idStr = orcType.getAttributeValue(ICEBERG_ID_ATTRIBUTE);
- Preconditions.checkNotNull(idStr, "Missing expected '%s' property",
ICEBERG_ID_ATTRIBUTE);
- return Integer.parseInt(idStr);
+ return icebergID(orcType)
+ .orElseThrow(() -> new NullPointerException(
+ String.format("Missing expected '%s' property",
ICEBERG_ID_ATTRIBUTE)));
Review comment:
What's the value of this change? It seems like unnecessary code churn to
me since the behavior is the same. It also causes an `Optional` to be
instantiated and discarded. Not a huge problem, but it doesn't support the idea
of changing this.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
