stevenzwu commented on code in PR #16408:
URL: https://github.com/apache/iceberg/pull/16408#discussion_r3344799314
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -210,38 +427,85 @@ void testProjectedStructLike() {
}
Review Comment:
While we're on the projection / `set` path: `TrackingStruct.internalSet`
silently ignores unknown ordinals to support reading newer formats (the
`default` branch with the "must be from a newer version" comment). That
forward-compat behavior has no test in any of the three structs
(`TrackingStruct`, `ManifestInfoStruct`, `DeletionVectorStruct`). One
regression test that calls `set` with an out-of-range ordinal and asserts no
throw and no observable state change would protect the contract.
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -175,19 +198,213 @@ void testNoDefaultingWithoutInheritance() {
}
private static Tracking createManifestTracking(long snapshotId, long
sequenceNumber) {
- return TrackingStruct.builder()
- .status(EntryStatus.ADDED)
- .snapshotId(snapshotId)
- .dataSequenceNumber(sequenceNumber)
- .fileSequenceNumber(sequenceNumber)
- .build();
+ TrackingStruct tracking = new TrackingStruct(Tracking.schema());
+ tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
+ tracking.set(SNAPSHOT_ID_ORDINAL, snapshotId);
+ tracking.set(DATA_SEQUENCE_NUMBER_ORDINAL, sequenceNumber);
+ tracking.set(FILE_SEQUENCE_NUMBER_ORDINAL, sequenceNumber);
+ return tracking;
+ }
+
+ @Test
+ void testAddedBuilder() {
+ Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
+
+ assertThat(tracking.status()).isEqualTo(EntryStatus.ADDED);
+ assertThat(tracking.snapshotId()).isEqualTo(42L);
+ assertThat(tracking.dvSnapshotId()).isEqualTo(42L);
+ assertThat(tracking.deletedPositions()).isNull();
+ assertThat(tracking.replacedPositions()).isNull();
+ // sequence numbers and firstRowId remain null; populated by inheritance
+ assertThat(tracking.dataSequenceNumber()).isNull();
+ assertThat(tracking.fileSequenceNumber()).isNull();
+ assertThat(tracking.firstRowId()).isNull();
+ }
+
+ @Test
+ void testExistingBuilderPreservesSourceFields() {
+ Tracking source = sourceTracking();
+
+ Tracking existing = TrackingBuilder.builder(source, 1L).build();
+
+ assertThat(existing.status()).isEqualTo(EntryStatus.EXISTING);
+ assertThat(existing.snapshotId()).isEqualTo(source.snapshotId());
+
assertThat(existing.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(existing.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(existing.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(existing.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testDeleteUpdatesSnapshotIdAndPreservesRest() {
+ Tracking source = sourceTracking();
+
+ Tracking deleted = TrackingBuilder.delete(source, 999L);
+
+ assertThat(deleted.status()).isEqualTo(EntryStatus.DELETED);
+ assertThat(deleted.snapshotId()).isEqualTo(999L);
+
assertThat(deleted.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(deleted.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(deleted.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(deleted.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testReplaceUpdatesSnapshotIdAndPreservesRest() {
+ Tracking source = sourceTracking();
+
+ Tracking replaced = TrackingBuilder.replace(source, 999L);
+
+ assertThat(replaced.status()).isEqualTo(EntryStatus.REPLACED);
+ assertThat(replaced.snapshotId()).isEqualTo(999L);
+
assertThat(replaced.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(replaced.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(replaced.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(replaced.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testSourceDvPositionsAreNotCarriedForward() {
+ TrackingStruct source = sourceTracking();
+ source.set(DELETED_POSITIONS_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2}));
+ source.set(REPLACED_POSITIONS_ORDINAL, ByteBuffer.wrap(new byte[] {3, 4}));
+
+ Tracking existing = TrackingBuilder.builder(source, 1L).build();
+ assertThat(existing.deletedPositions()).isNull();
+ assertThat(existing.replacedPositions()).isNull();
+
+ Tracking deleted = TrackingBuilder.delete(source, 999L);
+ assertThat(deleted.deletedPositions()).isNull();
+ assertThat(deleted.replacedPositions()).isNull();
+
+ Tracking replaced = TrackingBuilder.replace(source, 999L);
+ assertThat(replaced.deletedPositions()).isNull();
+ assertThat(replaced.replacedPositions()).isNull();
+ }
+
+ @Test
+ void testExistingBuilderAllowsDvMutation() {
+ Tracking existing = TrackingBuilder.builder(sourceTracking(),
999L).dvUpdated().build();
+ assertThat(existing.dvSnapshotId()).isEqualTo(999L);
+ }
+
+ @Test
+ void testMdvMutatorsRejectedOnAdded() {
+ assertThatThrownBy(
+ () ->
TrackingBuilder.added(42L).deletedPositions(ByteBuffer.wrap(new byte[] {1})))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot set deleted positions on ADDED entry");
+
+ assertThatThrownBy(
+ () ->
TrackingBuilder.added(42L).replacedPositions(ByteBuffer.wrap(new byte[] {1})))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot set replaced positions on ADDED entry");
+ }
+
+ @Test
+ void testDvSnapshotIdAndMdvPositionsAreMutuallyExclusive() {
+ // sourceTracking has dvSnapshotId=43, inherited by existing(source)
+ assertThatThrownBy(
+ () ->
+ TrackingBuilder.builder(sourceTracking(), 1L)
+ .deletedPositions(ByteBuffer.wrap(new byte[] {1})))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot set deleted positions on a data file entry (DV
snapshot ID is set)");
+
+ assertThatThrownBy(
+ () ->
+ TrackingBuilder.builder(sourceTracking(), 1L)
+ .replacedPositions(ByteBuffer.wrap(new byte[] {1})))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot set replaced positions on a data file entry (DV
snapshot ID is set)");
+
+ // Setting MDV positions first then dvUpdated is also rejected
+ assertThatThrownBy(
+ () ->
+ TrackingBuilder.builder(manifestSourceTracking(), 1L)
+ .deletedPositions(ByteBuffer.wrap(new byte[] {1}))
+ .dvUpdated())
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Cannot mark DV updated on a manifest entry (deleted/replaced
positions are set)");
+ }
+
+ @Test
+ void testBuilderRejectsNullSource() {
+ assertThatThrownBy(() -> TrackingBuilder.builder(null, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid source tracking: null");
}
@Test
- void testBuilderValidation() {
- assertThatThrownBy(() -> TrackingStruct.builder().build())
+ void testSourceBuildersRejectSourceWithoutSequenceNumbers() {
+ Tracking missingBoth = TrackingBuilder.added(42L).build();
+
+ assertThatThrownBy(() -> TrackingBuilder.builder(missingBoth, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid tracking source: data sequence number is null");
+
+ assertThatThrownBy(() -> TrackingBuilder.delete(missingBoth, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid tracking source: data sequence number is null");
+
+ assertThatThrownBy(() -> TrackingBuilder.replace(missingBoth, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid tracking source: data sequence number is null");
+
+ TrackingStruct missingFileSeq = new TrackingStruct(Tracking.schema());
+ missingFileSeq.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
+ missingFileSeq.set(SNAPSHOT_ID_ORDINAL, 42L);
+ missingFileSeq.set(DATA_SEQUENCE_NUMBER_ORDINAL, 10L);
+
+ assertThatThrownBy(() -> TrackingBuilder.builder(missingFileSeq, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid tracking source: file sequence number is null");
+
+ assertThatThrownBy(() -> TrackingBuilder.delete(missingFileSeq, 1L))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Invalid status: null");
+ .hasMessage("Invalid tracking source: file sequence number is null");
+
+ assertThatThrownBy(() -> TrackingBuilder.replace(missingFileSeq, 1L))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid tracking source: file sequence number is null");
+ }
+
+ @Test
+ void testRejectsTransitionsFromTerminalStatus() {
+ Tracking deletedSource = sourceTrackingWithStatus(EntryStatus.DELETED);
+ Tracking replacedSource = sourceTrackingWithStatus(EntryStatus.REPLACED);
+
+ assertThatThrownBy(() -> TrackingBuilder.builder(deletedSource, 1L))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot revive non-live entry with status DELETED");
+
+ assertThatThrownBy(() -> TrackingBuilder.delete(replacedSource, 1L))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot revive non-live entry with status REPLACED");
Review Comment:
Coverage gap on the central correctness invariant. `checkStatus` rejects
every transition out of a terminal source, but this test exercises only 2 of
the 6 forbidden source/factory pairs — `(DELETED, builder)` and `(REPLACED,
delete)` are covered, while `(DELETED, delete)`, `(DELETED, replace)`,
`(REPLACED, builder)`, and `(REPLACED, replace)` are not. A parameterized test
over `{DELETED, REPLACED} × {builder, delete, replace}` would close the matrix
and guard the invariant against regressions when `MODIFIED` is added.
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -329,13 +332,9 @@ void testKryoSerializationRoundTrip() throws IOException {
static TrackedFileStruct createFullTrackedFile() {
TrackingStruct tracking =
- TrackingStruct.builder()
- .status(EntryStatus.ADDED)
- .snapshotId(42L)
- .dataSequenceNumber(10L)
- .build();
+ new TrackingStruct(EntryStatus.ADDED, 42L, null, null, null, null,
null, null);
Review Comment:
nit: use `TrackingBuilder` as line 45 above
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -175,19 +198,213 @@ void testNoDefaultingWithoutInheritance() {
}
Review Comment:
`inheritFrom(null)` is intentionally a no-op (the outer null check on
`manifestTracking` in `TrackingStruct`), but no test exercises it. A small case
here would protect the silent-skip behavior from being refactored away.
##########
core/src/main/java/org/apache/iceberg/ManifestInfoStruct.java:
##########
@@ -252,121 +252,140 @@ public String toString() {
.add("replaced_rows_count", replacedRowsCount)
.add("min_sequence_number", minSequenceNumber)
.add("dv", dv == null ? "null" : "(binary)")
- .add("dv_cardinality", dvCardinality == null ? "null" : dvCardinality)
+ .add("dv_cardinality", dvCardinality)
.toString();
}
static class Builder {
- private int addedFilesCount = -1;
- private int existingFilesCount = -1;
- private int deletedFilesCount = -1;
- private int replacedFilesCount = -1;
- private long addedRowsCount = -1L;
- private long existingRowsCount = -1L;
- private long deletedRowsCount = -1L;
- private long replacedRowsCount = -1L;
- private long minSequenceNumber = -1L;
+ private Integer addedFilesCount = null;
+ private Integer existingFilesCount = null;
+ private Integer deletedFilesCount = null;
+ private Integer replacedFilesCount = null;
+ private Long addedRowsCount = null;
+ private Long existingRowsCount = null;
+ private Long deletedRowsCount = null;
+ private Long replacedRowsCount = null;
+ private Long minSequenceNumber = null;
private byte[] dv = null;
private Long dvCardinality = null;
Builder addedFilesCount(int count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid added files count: %s (must be >= 0)", count);
this.addedFilesCount = count;
return this;
}
Builder existingFilesCount(int count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid existing files count: %s (must be >= 0)",
count);
this.existingFilesCount = count;
return this;
}
Builder deletedFilesCount(int count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid deleted files count: %s (must be >= 0)", count);
this.deletedFilesCount = count;
return this;
}
Builder replacedFilesCount(int count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid replaced files count: %s (must be >= 0)",
count);
this.replacedFilesCount = count;
return this;
}
Builder addedRowsCount(long count) {
+ Preconditions.checkArgument(count >= 0, "Invalid added rows count: %s
(must be >= 0)", count);
this.addedRowsCount = count;
return this;
}
Builder existingRowsCount(long count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid existing rows count: %s (must be >= 0)", count);
this.existingRowsCount = count;
return this;
}
Builder deletedRowsCount(long count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid deleted rows count: %s (must be >= 0)", count);
this.deletedRowsCount = count;
return this;
}
Builder replacedRowsCount(long count) {
+ Preconditions.checkArgument(
+ count >= 0, "Invalid replaced rows count: %s (must be >= 0)", count);
this.replacedRowsCount = count;
return this;
}
Builder minSequenceNumber(long sequenceNumber) {
+ Preconditions.checkArgument(
+ sequenceNumber >= 0, "Invalid min sequence number: %s (must be >=
0)", sequenceNumber);
this.minSequenceNumber = sequenceNumber;
return this;
}
Builder dv(ByteBuffer buffer) {
- this.dv = buffer != null ? ByteBuffers.toByteArray(buffer) : null;
- return this;
- }
-
- Builder dv(byte[] buffer) {
- this.dv = buffer;
+ Preconditions.checkArgument(buffer != null, "Invalid DV: null");
+ this.dv = ByteBuffers.toByteArray(buffer);
return this;
}
- Builder dvCardinality(Long cardinality) {
+ Builder dvCardinality(long cardinality) {
+ Preconditions.checkArgument(
+ cardinality >= 0, "Invalid DV cardinality: %s (must be >= 0)",
cardinality);
this.dvCardinality = cardinality;
return this;
}
ManifestInfoStruct build() {
Preconditions.checkArgument(
- addedFilesCount >= 0, "Invalid added files count: %s (must be >=
0)", addedFilesCount);
+ addedFilesCount != null, "Missing required value: added files
count");
Preconditions.checkArgument(
- existingFilesCount >= 0,
- "Invalid existing files count: %s (must be >= 0)",
- existingFilesCount);
+ existingFilesCount != null, "Missing required value: existing files
count");
Preconditions.checkArgument(
- deletedFilesCount >= 0,
- "Invalid deleted files count: %s (must be >= 0)",
- deletedFilesCount);
+ deletedFilesCount != null, "Missing required value: deleted files
count");
Preconditions.checkArgument(
- replacedFilesCount >= 0,
- "Invalid replaced files count: %s (must be >= 0)",
- replacedFilesCount);
+ replacedFilesCount != null, "Missing required value: replaced files
count");
+ Preconditions.checkArgument(
+ addedRowsCount != null, "Missing required value: added rows count");
+ Preconditions.checkArgument(
+ existingRowsCount != null, "Missing required value: existing rows
count");
+ Preconditions.checkArgument(
+ deletedRowsCount != null, "Missing required value: deleted rows
count");
Preconditions.checkArgument(
- addedRowsCount >= 0, "Invalid added rows count: %s (must be >= 0)",
addedRowsCount);
+ replacedRowsCount != null, "Missing required value: replaced rows
count");
Preconditions.checkArgument(
- existingRowsCount >= 0,
- "Invalid existing rows count: %s (must be >= 0)",
- existingRowsCount);
+ minSequenceNumber != null, "Missing required value: min sequence
number");
Preconditions.checkArgument(
- deletedRowsCount >= 0, "Invalid deleted rows count: %s (must be >=
0)", deletedRowsCount);
+ addedRowsCount == 0 || addedFilesCount > 0,
Review Comment:
ah. I forgot the earlier check in the setter method. will resolve
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -175,19 +198,213 @@ void testNoDefaultingWithoutInheritance() {
}
private static Tracking createManifestTracking(long snapshotId, long
sequenceNumber) {
- return TrackingStruct.builder()
- .status(EntryStatus.ADDED)
- .snapshotId(snapshotId)
- .dataSequenceNumber(sequenceNumber)
- .fileSequenceNumber(sequenceNumber)
- .build();
+ TrackingStruct tracking = new TrackingStruct(Tracking.schema());
+ tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
+ tracking.set(SNAPSHOT_ID_ORDINAL, snapshotId);
+ tracking.set(DATA_SEQUENCE_NUMBER_ORDINAL, sequenceNumber);
+ tracking.set(FILE_SEQUENCE_NUMBER_ORDINAL, sequenceNumber);
+ return tracking;
+ }
+
+ @Test
+ void testAddedBuilder() {
+ Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
+
+ assertThat(tracking.status()).isEqualTo(EntryStatus.ADDED);
+ assertThat(tracking.snapshotId()).isEqualTo(42L);
+ assertThat(tracking.dvSnapshotId()).isEqualTo(42L);
+ assertThat(tracking.deletedPositions()).isNull();
+ assertThat(tracking.replacedPositions()).isNull();
+ // sequence numbers and firstRowId remain null; populated by inheritance
+ assertThat(tracking.dataSequenceNumber()).isNull();
+ assertThat(tracking.fileSequenceNumber()).isNull();
+ assertThat(tracking.firstRowId()).isNull();
+ }
+
+ @Test
+ void testExistingBuilderPreservesSourceFields() {
+ Tracking source = sourceTracking();
+
+ Tracking existing = TrackingBuilder.builder(source, 1L).build();
+
+ assertThat(existing.status()).isEqualTo(EntryStatus.EXISTING);
+ assertThat(existing.snapshotId()).isEqualTo(source.snapshotId());
+
assertThat(existing.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(existing.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(existing.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(existing.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testDeleteUpdatesSnapshotIdAndPreservesRest() {
+ Tracking source = sourceTracking();
+
+ Tracking deleted = TrackingBuilder.deleted(source, 999L);
+
+ assertThat(deleted.status()).isEqualTo(EntryStatus.DELETED);
+ assertThat(deleted.snapshotId()).isEqualTo(999L);
+
assertThat(deleted.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(deleted.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(deleted.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(deleted.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testReplaceUpdatesSnapshotIdAndPreservesRest() {
+ Tracking source = sourceTracking();
+
+ Tracking replaced = TrackingBuilder.replaced(source, 999L);
+
+ assertThat(replaced.status()).isEqualTo(EntryStatus.REPLACED);
+ assertThat(replaced.snapshotId()).isEqualTo(999L);
+
assertThat(replaced.dataSequenceNumber()).isEqualTo(source.dataSequenceNumber());
+
assertThat(replaced.fileSequenceNumber()).isEqualTo(source.fileSequenceNumber());
+ assertThat(replaced.dvSnapshotId()).isEqualTo(source.dvSnapshotId());
+ assertThat(replaced.firstRowId()).isEqualTo(source.firstRowId());
+ }
+
+ @Test
+ void testSourceDvPositionsAreNotCarriedForward() {
+ TrackingStruct source = sourceTracking();
+ source.set(DELETED_POSITIONS_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2}));
+ source.set(REPLACED_POSITIONS_ORDINAL, ByteBuffer.wrap(new byte[] {3, 4}));
+
+ Tracking existing = TrackingBuilder.builder(source, 1L).build();
+ assertThat(existing.deletedPositions()).isNull();
+ assertThat(existing.replacedPositions()).isNull();
+
+ Tracking deleted = TrackingBuilder.deleted(source, 999L);
+ assertThat(deleted.deletedPositions()).isNull();
+ assertThat(deleted.replacedPositions()).isNull();
+
+ Tracking replaced = TrackingBuilder.replaced(source, 999L);
+ assertThat(replaced.deletedPositions()).isNull();
+ assertThat(replaced.replacedPositions()).isNull();
+ }
+
+ @Test
+ void testExistingBuilderAllowsDvMutation() {
+ Tracking existing = TrackingBuilder.builder(sourceTracking(),
999L).dvUpdated().build();
+ assertThat(existing.dvSnapshotId()).isEqualTo(999L);
+ }
+
+ @Test
+ void testMdvMutatorsRejectedOnAdded() {
Review Comment:
nit: `Mdv` acronym is hard to tell what it means. maybe spell it out as
`ManifestDV`. same for the method below
##########
core/src/main/java/org/apache/iceberg/TrackingBuilder.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.nio.ByteBuffer;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.util.ByteBuffers;
+
+class TrackingBuilder {
+ private final EntryStatus status;
+ private final Long snapshotId;
+ private final Long dataSequenceNumber;
+ private final Long fileSequenceNumber;
+ private final Long firstRowId;
+ // ID of the snapshot in which the new Tracking instance will be committed.
+ private final long newSnapshotId;
+ private Long dvSnapshotId;
+ private byte[] deletedPositions;
+ private byte[] replacedPositions;
+
+ /**
+ * Creates a builder for a newly added file.
+ *
+ * @param snapshotId the snapshot ID in which the new tracking instance will
be committed
+ */
+ static TrackingBuilder added(long snapshotId) {
Review Comment:
> If anything, I'd rename the "builder" that is for an existing entry, but
I'm not sure what to call it since it may produce a MODIFIED or an EXISTING
record based on the changes.
maybe `static TrackingBuilder from(Tracking source, long newSnapshotId)`?
--
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]
