rdblue commented on code in PR #18171:
URL: https://github.com/apache/iceberg/pull/18171#discussion_r4076753246
##########
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 prefer to take that complexity over having the expectation that
writers set it to be null
Another result of the snapshot ID check to override is that the writer
wouldn't have control over inheritance. For example, what if a writer doesn't
modify data and instead combines column overlays? In that case, a writer that
isn't changing modifying rows should be able to preserve the sequence number if
it chooses to but the snapshot ID rule would override. On the other hand, does
it matter if it isn't being inherited to the row level?
Overall, I think it's best to rely on a consistent and simple rule that a
writer uses null to set the data sequence number to the current commit's
sequence number when it is assigned.
--
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]
