gaborkaszab commented on code in PR #7555:
URL: https://github.com/apache/iceberg/pull/7555#discussion_r1195032379
##########
core/src/main/java/org/apache/iceberg/BaseFile.java:
##########
@@ -76,6 +76,9 @@ public PartitionData copy() {
private byte[] keyMetadata = null;
private Integer sortOrderId;
+ private Long dataSequenceNumber = null;
Review Comment:
Yeah, giving it a second thought, they indeed aren't optional. Added them to
the same block as `partitionSpecId`.
##########
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:
Done
##########
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:
I kept consistency in mind with this code as a few other members above are
also added the same way.
##########
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:
Done
--
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]
