amogh-jahagirdar commented on code in PR #17035:
URL: https://github.com/apache/iceberg/pull/17035#discussion_r3538292354
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -36,254 +36,203 @@ class TestTrackedFileStruct {
Types.NestedField.optional(1000, "id_bucket",
Types.IntegerType.get()),
Types.NestedField.optional(1001, "category",
Types.StringType.get()));
- // Ordinals looked up from the TrackedFile schema so tests don't hard-code
positions.
- private static final List<Types.NestedField> SCHEMA_FIELDS =
- TrackedFile.schemaWithContentStats(Types.StructType.of(),
Types.StructType.of()).fields();
- private static final int TRACKING_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.TRACKING);
- private static final int CONTENT_TYPE_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.CONTENT_TYPE);
- private static final int FORMAT_VERSION_ORDINAL =
- SCHEMA_FIELDS.indexOf(TrackedFile.FORMAT_VERSION);
- private static final int LOCATION_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.LOCATION);
- private static final int FILE_FORMAT_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.FILE_FORMAT);
- private static final int RECORD_COUNT_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.RECORD_COUNT);
- private static final int FILE_SIZE_IN_BYTES_ORDINAL =
- SCHEMA_FIELDS.indexOf(TrackedFile.FILE_SIZE_IN_BYTES);
- private static final int SPEC_ID_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.SPEC_ID);
- private static final int PARTITION_ORDINAL =
ordinalOf(TrackedFile.PARTITION_ID);
- private static final int SORT_ORDER_ID_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.SORT_ORDER_ID);
- private static final int DELETION_VECTOR_ORDINAL =
- SCHEMA_FIELDS.indexOf(TrackedFile.DELETION_VECTOR);
- private static final int MANIFEST_INFO_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.MANIFEST_INFO);
- private static final int KEY_METADATA_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.KEY_METADATA);
- private static final int SPLIT_OFFSETS_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.SPLIT_OFFSETS);
- private static final int EQUALITY_IDS_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.EQUALITY_IDS);
-
- // Ordinal of MetadataColumns.ROW_POSITION within TrackingStruct's BASE_TYPE,
- // which appends ROW_POSITION after the Tracking schema fields.
- private static final int MANIFEST_POS_ORDINAL =
Tracking.schema().fields().size();
-
- private static int ordinalOf(int fieldId) {
- for (int i = 0; i < SCHEMA_FIELDS.size(); i++) {
- if (SCHEMA_FIELDS.get(i).fieldId() == fieldId) {
- return i;
- }
- }
- throw new IllegalArgumentException("Field not found in TrackedFile schema:
" + fieldId);
- }
+ private static final List<Types.NestedField> FIELDS =
+ TrackedFile.schemaWithContentStats(PARTITION_TYPE,
Types.StructType.of()).fields();
+
+ private static final Tracking TRACKING =
+ new TrackingStruct(EntryStatus.ADDED, 42L, 10L, 10L, 43L, 1000L, null,
null);
+
+ private static final PartitionData PARTITION = newPartition(7, "music");
+
+ private static final DeletionVectorStruct DELETION_VECTOR =
+ DeletionVectorStruct.builder()
+ .location("s3://bucket/dv.puffin")
+ .offset(100L)
+ .sizeInBytes(50L)
+ .cardinality(5L)
+ .build();
+
+ private static final ManifestInfoStruct MANIFEST_INFO =
+ ManifestInfoStruct.builder()
+ .addedFilesCount(10)
+ .existingFilesCount(20)
+ .deletedFilesCount(3)
+ .replacedFilesCount(2)
+ .addedRowsCount(1000L)
+ .existingRowsCount(2000L)
+ .deletedRowsCount(300L)
+ .replacedRowsCount(200L)
+ .minSequenceNumber(5L)
+ .build();
@Test
- void testFieldAccess() {
- TrackedFileStruct file = new TrackedFileStruct();
- Tracking tracking = TrackingBuilder.added(42L).build();
- DeletionVectorStruct dv =
- DeletionVectorStruct.builder()
- .location("s3://bucket/dv.puffin")
- .offset(100L)
- .sizeInBytes(50L)
- .cardinality(5L)
- .build();
- ManifestInfoStruct info =
- ManifestInfoStruct.builder()
- .addedFilesCount(10)
- .existingFilesCount(20)
- .deletedFilesCount(3)
- .replacedFilesCount(2)
- .addedRowsCount(1000L)
- .existingRowsCount(2000L)
- .deletedRowsCount(300L)
- .replacedRowsCount(200L)
- .minSequenceNumber(5L)
- .build();
-
- file.set(TRACKING_ORDINAL, tracking);
- file.set(CONTENT_TYPE_ORDINAL, FileContent.EQUALITY_DELETES.id());
- file.set(FORMAT_VERSION_ORDINAL, FORMAT_VERSION_V4);
- file.set(LOCATION_ORDINAL, "s3://bucket/data/eq-delete.avro");
- file.set(FILE_FORMAT_ORDINAL, "avro");
- file.set(RECORD_COUNT_ORDINAL, 50L);
- file.set(FILE_SIZE_IN_BYTES_ORDINAL, 512L);
- file.set(SPEC_ID_ORDINAL, 1);
- file.set(SORT_ORDER_ID_ORDINAL, 5);
- file.set(DELETION_VECTOR_ORDINAL, dv);
- file.set(MANIFEST_INFO_ORDINAL, info);
- file.set(KEY_METADATA_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2, 3}));
- file.set(SPLIT_OFFSETS_ORDINAL, ImmutableList.of(100L, 200L));
- file.set(EQUALITY_IDS_ORDINAL, ImmutableList.of(1, 2, 3));
-
- assertThat(file.tracking()).isNotNull();
- assertThat(file.tracking().status()).isEqualTo(EntryStatus.ADDED);
- assertThat(file.tracking().snapshotId()).isEqualTo(42L);
- assertThat(file.contentType()).isEqualTo(FileContent.EQUALITY_DELETES);
+ void fieldAccess() {
+ TrackedFileStruct file =
+ new TrackedFileStruct(
+ TRACKING,
+ FileContent.DATA,
+ FORMAT_VERSION_V4,
+ "s3://bucket/data/00000-0-file.parquet",
+ FileFormat.PARQUET,
+ PARTITION,
+ 50L,
+ 512L,
+ 1,
+ null,
+ 5,
+ DELETION_VECTOR,
+ MANIFEST_INFO,
+ ByteBuffer.wrap(new byte[] {1, 2, 3}),
+ ImmutableList.of(100L, 200L),
+ ImmutableList.of(1, 2, 3));
+
+ assertThat(file.tracking()).isSameAs(TRACKING);
+ assertThat(file.contentType()).isEqualTo(FileContent.DATA);
assertThat(file.formatVersion()).isEqualTo(FORMAT_VERSION_V4);
- assertThat(file.location()).isEqualTo("s3://bucket/data/eq-delete.avro");
- assertThat(file.fileFormat()).isEqualTo(FileFormat.AVRO);
+
assertThat(file.location()).isEqualTo("s3://bucket/data/00000-0-file.parquet");
+ assertThat(file.fileFormat()).isEqualTo(FileFormat.PARQUET);
+ assertThat(file.partition()).isSameAs(PARTITION);
assertThat(file.recordCount()).isEqualTo(50L);
assertThat(file.fileSizeInBytes()).isEqualTo(512L);
assertThat(file.specId()).isEqualTo(1);
assertThat(file.sortOrderId()).isEqualTo(5);
- assertThat(file.deletionVector()).isSameAs(dv);
- assertThat(file.manifestInfo()).isSameAs(info);
+ assertThat(file.deletionVector()).isSameAs(DELETION_VECTOR);
+ assertThat(file.manifestInfo()).isSameAs(MANIFEST_INFO);
assertThat(file.keyMetadata()).isEqualTo(ByteBuffer.wrap(new byte[] {1, 2,
3}));
assertThat(file.splitOffsets()).containsExactly(100L, 200L);
assertThat(file.equalityIds()).containsExactly(1, 2, 3);
- // should return EMPTY_PARTITION_DATA
- assertThat(file.partition()).isNotNull();
- assertThat(file.partition().size()).isEqualTo(0);
}
@Test
- void testReaderSideFields() {
+ void setByPosition() {
TrackedFileStruct file = new TrackedFileStruct();
-
- TrackingStruct tracking = new TrackingStruct();
- tracking.setManifestLocation("s3://bucket/metadata/manifest.avro");
- tracking.set(MANIFEST_POS_ORDINAL, 7L);
-
- file.set(TRACKING_ORDINAL, tracking);
- file.set(CONTENT_TYPE_ORDINAL, FileContent.DATA.id());
- file.set(LOCATION_ORDINAL, "test");
- file.set(FILE_FORMAT_ORDINAL, "parquet");
- file.set(RECORD_COUNT_ORDINAL, 0L);
- file.set(FILE_SIZE_IN_BYTES_ORDINAL, 0L);
-
-
assertThat(file.tracking().manifestLocation()).isEqualTo("s3://bucket/metadata/manifest.avro");
- assertThat(file.tracking().manifestPos()).isEqualTo(7L);
- }
-
- @Test
- void projectionWithoutPartition() {
- // project only location (field ID 100) and file_size_in_bytes (field ID
104)
- Types.StructType projection =
- Types.StructType.of(TrackedFile.LOCATION,
TrackedFile.FILE_SIZE_IN_BYTES);
-
- TrackedFileStruct file = new TrackedFileStruct(projection);
- assertThat(file.size()).isEqualTo(2);
- // should return EMPTY_PARTITION_DATA
- assertThat(file.partition()).isNotNull();
- assertThat(file.partition().size()).isEqualTo(0);
+ file.set(pos("tracking"), TRACKING);
+ file.set(pos("content_type"), FileContent.DATA.id());
+ file.set(pos("format_version"), FORMAT_VERSION_V4);
+ file.set(pos("location"), "s3://bucket/data/00000-0-file.parquet");
+ file.set(pos("file_format"), "parquet");
+ file.set(pos("record_count"), 50L);
+ file.set(pos("file_size_in_bytes"), 512L);
+ file.set(pos("spec_id"), 1);
Review Comment:
ContentStats is being restructured, but we only wanted to remove the
copyWithoutStats test and any of the other tests which depend on richer stats
specific APIs. We still need to test the actual setter/getter of content stats
which somehow got dropped! I added those back
--
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]
