rdblue commented on code in PR #11354:
URL: https://github.com/apache/iceberg/pull/11354#discussion_r1817898860


##########
core/src/main/java/org/apache/iceberg/SnapshotParser.java:
##########
@@ -129,13 +129,12 @@ static Snapshot fromJson(JsonNode node) {
           "Cannot parse summary from non-object value: %s",
           sNode);
 
+      operation = JsonUtil.getString(OPERATION, sNode);
       ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
       Iterator<String> fields = sNode.fieldNames();
       while (fields.hasNext()) {
         String field = fields.next();
-        if (field.equals(OPERATION)) {
-          operation = JsonUtil.getString(OPERATION, sNode);
-        } else {
+        if (!field.equals(OPERATION)) {

Review Comment:
   I disagree with this change. Although the operation is required by the spec, 
we do not want to fail to read a table if it isn't there. We can be more 
permissive than the spec.
   
   @kevinjqliu can you open a PR to revert this please?



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to