amogh-jahagirdar commented on code in PR #7555:
URL: https://github.com/apache/iceberg/pull/7555#discussion_r1191780054
##########
core/src/test/java/org/apache/iceberg/TestSnapshot.java:
##########
@@ -170,4 +170,120 @@ public void testCachedDeleteFiles() {
Assert.assertEquals(
"Partition must match", thirdSnapshotDeleteFile.partition(),
addedDeleteFile.partition());
}
+
+ @Test
+ public void testSequenceNumbersInAddedDataFiles() {
+ long expectedSequenceNumber = 0L;
+ if (formatVersion >= 2) {
+ expectedSequenceNumber = 1L;
+ }
Review Comment:
Nit: could we double check style throughout the test class, if blocks should
have a newline after
##########
core/src/test/java/org/apache/iceberg/TableTestBase.java:
##########
@@ -562,6 +553,44 @@ void validateDeleteManifest(
Assert.assertFalse("Should find all files in the manifest",
expectedFiles.hasNext());
}
+ private <T extends ContentFile<T>> void validateManifestSequenceNumbers(
+ ManifestEntry<T> entry, Iterator<Long> dataSeqs, Iterator<Long>
fileSeqs) {
+ if (dataSeqs != null) {
+ V1Assert.assertEquals(
+ "Data sequence number should default to 0", 0,
entry.dataSequenceNumber().longValue());
+ V1Assert.assertEquals(
+ "Data sequence number should default to 0",
+ 0,
+ entry.file().dataSequenceNumber().longValue());
+
+ Long expectedSequenceNumber = dataSeqs.next();
+ V2Assert.assertEquals(
+ "Data sequence number should match expected",
+ expectedSequenceNumber,
+ entry.dataSequenceNumber());
+ V2Assert.assertEquals(
+ "Data sequence number should match expected",
+ expectedSequenceNumber,
+ entry.file().dataSequenceNumber());
+ }
Review Comment:
Nit: newline after if
##########
core/src/main/java/org/apache/iceberg/BaseFile.java:
##########
@@ -478,6 +500,8 @@ public String toString() {
.add("split_offsets", splitOffsets == null ? "null" : splitOffsets())
.add("equality_ids", equalityIds == null ? "null" : equalityFieldIds())
.add("sort_order_id", sortOrderId)
+ .add("data_sequence_number", dataSequenceNumber == null ? "null" :
dataSequenceNumber)
+ .add("file_sequence_number", fileSequenceNumber == null ? "null" :
fileSequenceNumber)
Review Comment:
Nit: I suppose this is fine, but I think add will already take care of
converting the null for us (looking at this
[link](https://guava.dev/releases/19.0/api/docs/com/google/common/base/MoreObjects.ToStringHelper.html#add(java.lang.String,%20long))
--
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]
