rdblue commented on code in PR #18171:
URL: https://github.com/apache/iceberg/pull/18171#discussion_r4075450957
##########
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:
I'd like to keep inheritance as simple as possible. I think we would only
need to update this so that a null data sequence number is inherited when the
state is `MODIFIED` (and not file sequence number).
It sounds like you're thinking about how to detect when we should override
the sequence number by checking the `last_column_update_snapshot_id`? I don't
think I follow the logic of using the current snapshot ID anywhere. Inheritance
must happen no matter what the current table state is. We _could_ check against
the manifest's `snapshot_id` because the manifest with an updated entry must be
written at the same time. But to me, this is over-complicated compared to
expecting the writer to set the `data_sequence_number` to null in the
`MODIFIED` entry when it needs to be inherited.
--
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]
