rdblue commented on code in PR #17041:
URL: https://github.com/apache/iceberg/pull/17041#discussion_r3539434552
##########
core/src/test/java/org/apache/iceberg/TestTrackingStruct.java:
##########
@@ -514,84 +308,44 @@ void testProjectedStructLike() {
assertThat(tracking.get(1, Long.class)).isEqualTo(1000L);
}
- @Test
- void testJavaSerializationRoundTripForDataFile() throws IOException,
ClassNotFoundException {
- Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
-
- Tracking deserialized = TestHelpers.roundTripSerialize(tracking);
-
- assertThat(deserialized.status()).isEqualTo(EntryStatus.ADDED);
- assertThat(deserialized.snapshotId()).isEqualTo(42L);
- assertThat(deserialized.dvSnapshotId()).isEqualTo(42L);
- assertThat(deserialized.deletedPositions()).isNull();
- assertThat(deserialized.replacedPositions()).isNull();
- }
-
- @Test
- void testJavaSerializationRoundTripForManifest() throws IOException,
ClassNotFoundException {
+ @ParameterizedTest
+ @MethodSource("org.apache.iceberg.TestHelpers#serializers")
+ void testSerializationRoundTrip(TestHelpers.RoundTripSerializer<Tracking>
roundTripSerializer)
+ throws IOException, ClassNotFoundException {
Tracking tracking =
- TrackingBuilder.from(manifestSourceTracking(), 1L)
- .deletedPositions(ByteBuffer.wrap(new byte[] {1, 2}))
- .replacedPositions(ByteBuffer.wrap(new byte[] {3, 4}))
- .build();
-
- Tracking deserialized = TestHelpers.roundTripSerialize(tracking);
+ new TrackingStruct(
+ EntryStatus.MODIFIED,
+ 42L,
+ 10L,
+ 11L,
+ 43L,
+ 1000L,
+ DELETED_POSITIONS,
+ REPLACED_POSITIONS,
+ "manifest-location",
+ 7L);
+
+ Tracking deserialized = roundTripSerializer.apply(tracking);
assertThat(deserialized.status()).isEqualTo(EntryStatus.MODIFIED);
- assertThat(deserialized.dvSnapshotId()).isEqualTo(1L);
- assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(new
byte[] {1, 2}));
- assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(new
byte[] {3, 4}));
- }
-
- @Test
- void testKryoSerializationRoundTripForDataFile() throws IOException {
- Tracking tracking = TrackingBuilder.added(42L).dvUpdated().build();
-
- Tracking deserialized =
TestHelpers.KryoHelpers.roundTripSerialize(tracking);
-
- assertThat(deserialized.status()).isEqualTo(EntryStatus.ADDED);
assertThat(deserialized.snapshotId()).isEqualTo(42L);
- assertThat(deserialized.dvSnapshotId()).isEqualTo(42L);
- assertThat(deserialized.deletedPositions()).isNull();
- assertThat(deserialized.replacedPositions()).isNull();
- }
-
- @Test
- void testKryoSerializationRoundTripForManifest() throws IOException {
- Tracking tracking =
- TrackingBuilder.from(manifestSourceTracking(), 1L)
- .deletedPositions(ByteBuffer.wrap(new byte[] {1, 2}))
- .replacedPositions(ByteBuffer.wrap(new byte[] {3, 4}))
- .build();
-
- Tracking deserialized =
TestHelpers.KryoHelpers.roundTripSerialize(tracking);
-
- assertThat(deserialized.status()).isEqualTo(EntryStatus.MODIFIED);
- assertThat(deserialized.dvSnapshotId()).isEqualTo(1L);
- assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(new
byte[] {1, 2}));
- assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(new
byte[] {3, 4}));
- }
-
- private static TrackingStruct sourceTracking() {
- TrackingStruct tracking = new TrackingStruct(Tracking.schema());
- tracking.set(STATUS_ORDINAL, EntryStatus.ADDED.id());
- tracking.set(SNAPSHOT_ID_ORDINAL, 42L);
- tracking.set(DATA_SEQUENCE_NUMBER_ORDINAL, 10L);
- tracking.set(FILE_SEQUENCE_NUMBER_ORDINAL, 10L);
- tracking.set(DV_SNAPSHOT_ID_ORDINAL, 43L);
- tracking.set(FIRST_ROW_ID_ORDINAL, 1000L);
- return tracking;
- }
-
- private static TrackingStruct sourceTrackingWithStatus(EntryStatus status) {
- TrackingStruct tracking = sourceTracking();
- tracking.set(STATUS_ORDINAL, status.id());
- return tracking;
- }
-
- private static TrackingStruct manifestSourceTracking() {
- TrackingStruct tracking = sourceTracking();
- tracking.set(DV_SNAPSHOT_ID_ORDINAL, null);
- return tracking;
+ assertThat(deserialized.dataSequenceNumber()).isEqualTo(10L);
+ assertThat(deserialized.fileSequenceNumber()).isEqualTo(11L);
+ assertThat(deserialized.dvSnapshotId()).isEqualTo(43L);
+ assertThat(deserialized.firstRowId()).isEqualTo(1000L);
+
assertThat(deserialized.deletedPositions()).isEqualTo(ByteBuffer.wrap(DELETED_POSITIONS));
+
assertThat(deserialized.replacedPositions()).isEqualTo(ByteBuffer.wrap(REPLACED_POSITIONS));
+ assertThat(deserialized.manifestLocation()).isEqualTo("manifest-location");
+ assertThat(deserialized.manifestPos()).isEqualTo(7L);
+ }
+
+ private static int ordinalByFieldName(String fieldName) {
+ List<Types.NestedField> fields = TrackingStruct.BASE_TYPE.fields();
Review Comment:
This should use `Tracking.schema()` to find positions instead. `BASE_TYPE`
should remain private rather than sharing it here.
`BASE_TYPE` is the type that the struct passes to `SupportsIndexProjection`
and it must match the positions in `get` and `set` exactly. That's why it is
private and located along with the struct implementation (so that they can
match). But the validation we want to do here is against the type we actually
use to read and write, which is `Tracking.schema()`.
--
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]
