amogh-jahagirdar commented on code in PR #16263:
URL: https://github.com/apache/iceberg/pull/16263#discussion_r3213456577
##########
core/src/main/java/org/apache/iceberg/ManifestReader.java:
##########
@@ -417,14 +417,8 @@ public ManifestEntry<F> apply(ManifestEntry<F> entry) {
}
};
} else {
- // data file's first_row_id is null when the manifest's first_row_id is
null
Review Comment:
I'm still checking if changing idAssigner like this for all cases is really
the right way to fix this or if during manifest merging we should actually pass
in our own id assignment transformer that preserves the existing case.
Here we're changing the manifest reader because
```
// data file's first_row_id is null when the manifest's first_row_id is null
```
isn't true in the context of reading a newly merged manifest. The manfiest
first row ID is null, but the merged content could contain an existing entry
with an already assigned first row ID.
--
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]
