kevinjqliu commented on code in PR #11354:
URL: https://github.com/apache/iceberg/pull/11354#discussion_r1818193692
##########
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:
Created a revert PR, #11409.
> 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.
I agree that we want to be more permissive than the spec when reading a
table. However, what should we do about working with a table that is not
compliant with the spec? We would need to add extra logic to handle V2 tables
without the Snapshot `operation` field.
> Is there a historical reason where we expect some tables not to have this
populated?
From looking at the [commit history for
`SnapshotParser.toJson`](https://github.com/apache/iceberg/blame/377cab742adacc18f1c81d372bb4d65a549e180e/core/src/main/java/org/apache/iceberg/SnapshotParser.java#L63-L66),
the `summary` and `operation` fields either both exist or are both missing. So
historically, the Java Iceberg implementation should not produce one without
the other.
Here's my current understanding:
- V1 tables do not have `operation` and `summary` fields.
- V2 tables should have both `operation` and `summary` fields
- Another engine generated a V2 table with the `summary` field but without
the `operation` field. This is technically not compliant with the V2 spec. We
want to be more permissive and still allow reading this non-compliant table.
But we should figure out how to work with this V2 table that is without
Snapshot operation.
--
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
