nastra commented on code in PR #6051:
URL: https://github.com/apache/iceberg/pull/6051#discussion_r1004644948
##########
core/src/main/java/org/apache/iceberg/util/JsonUtil.java:
##########
@@ -212,8 +212,7 @@ public static List<String> getStringList(String property,
JsonNode node) {
}
public static Set<String> getStringSet(String property, JsonNode node) {
- Preconditions.checkArgument(
- node.hasNonNull(property), "Cannot parse missing set: %s", property);
+ Preconditions.checkArgument(node.has(property), "Cannot parse missing set:
%s", property);
Review Comment:
the null check is already being handled inside the array handler. Relaxing
this one here allows for being more precise on the error reporting as we don't
want to say `Cannot parse missing set: items` when in fact `items` was given,
but it was null (`{\"items\": null}`). What we'd want to report is `"Cannot
parse from non-array value: items: null"`. I also added some tests for this to
make sure we're reporting the right thing
--
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]
