amogh-jahagirdar commented on code in PR #11354:
URL: https://github.com/apache/iceberg/pull/11354#discussion_r1817912195


##########
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 went through https://github.com/apache/iceberg-python/issues/1106, seems 
like another engine is now producing the metadata without the operation key but 
that just doesn't seem spec compliant. 
   I guess there's the argument that we want to prevent needless failures but I 
actually think this may be needed (the validation example I mentioned above, 
unless I'm missing something?)



-- 
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