amogh-jahagirdar commented on code in PR #16263:
URL: https://github.com/apache/iceberg/pull/16263#discussion_r3220256497
##########
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:
Yeah @stevenzwu that's my analysis of all the call points as well.
Fundamentally, I would expect the idTransformer that's set by default on the
ManifestReader should do *no* transformation of the first row ID on the entry.
That implicitly covers the 3 cases you mentioned.
And for the coppyAppendManifest case, if we really want to be defensive, I
think the right solution is that the public writer APIs Iceberg exposes should
suppress the firstRowId assignment on the ADDED entry. The read side should
just consume what's there in my opinion.
--
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]
