amogh-jahagirdar commented on code in PR #17035:
URL: https://github.com/apache/iceberg/pull/17035#discussion_r3509741471
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -256,23 +341,6 @@ void testStructLikeSize() {
assertThat(file.size()).isEqualTo(16);
}
- @Test
- void testStructLikeGetSet() {
Review Comment:
Not needed anymore since we have a single test for verifying set and a
single test for verifying get
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -36,83 +36,62 @@ 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 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();
+
+ private static final ContentStats CONTENT_STATS =
Review Comment:
This is still using the builder for some of the nested fields within
TrackedFile but I think that's fine, we're largely concerned about the
explicitness of TrackedFile tests in this change.
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -41,22 +41,13 @@ class TestTrackedFileStruct {
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);
Review Comment:
Removing these ordinals because they're unused now.
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -177,16 +264,14 @@ void partitionAccess() {
@Test
void partitionIsCopied() {
- PartitionData partition = newPartition(5, "books");
TrackedFileStruct file = createFullTrackedFile();
- file.set(PARTITION_ORDINAL, partition);
TrackedFile copy = file.copy();
- assertThat(copy.partition()).isNotNull().isNotSameAs(partition);
Review Comment:
I'm not sure I understand the reason for this assertion, doesn't the next
one imply it?
--
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]
