amogh-jahagirdar commented on code in PR #18171:
URL: https://github.com/apache/iceberg/pull/18171#discussion_r4075636065
##########
core/src/main/java/org/apache/iceberg/TrackingStruct.java:
##########
@@ -116,34 +116,20 @@ private TrackingStruct(TrackingStruct toCopy) {
this.replacedPositions = replacedPositions;
}
- void inheritFrom(Tracking manifestTracking) {
- if (manifestTracking != null) {
- if (snapshotId == null) {
- this.snapshotId = manifestTracking.snapshotId();
- }
-
- // manifests do not distinguish between data and file sequence numbers
- Preconditions.checkArgument(
- Objects.equals(
- manifestTracking.dataSequenceNumber(),
manifestTracking.fileSequenceNumber()),
- "Manifest data and file sequence numbers must be equal, got %s and
%s",
- manifestTracking.dataSequenceNumber(),
- manifestTracking.fileSequenceNumber());
+ void inherit(long manifestSnapshotId, long manifestSeqNumber) {
+ if (null == snapshotId) {
+ this.snapshotId = manifestSnapshotId;
+ }
- if (status == EntryStatus.ADDED) {
- if (dataSequenceNumber == null) {
- this.dataSequenceNumber = manifestTracking.fileSequenceNumber();
- }
+ boolean isAdded = status == EntryStatus.ADDED;
Review Comment:
Yeah you're right that file sequence number should not be inherited for that
case. "Current" may have been a poor choice of words but I just meant the
snapshot that's being read at that time, not neccessarily the latest main
state. But I think ultimately you're right that we neccessarily need to be
writing out a new manifest if we're doing a column update anyways so the
manifest snapshot ID may be used. Personally, I'd prefer to take that
complexity over having the expectation that writers set it to be null but at
the same time it's in the writers own benefit to set it null anyways (that's
arguably an easier thing for them to do). Anyways don't want to get ahead of
ourselves, that's a later problem, for now this makes sense to me!
--
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]
