rdblue commented on code in PR #18225:
URL: https://github.com/apache/iceberg/pull/18225#discussion_r4097321726
##########
core/src/test/java/org/apache/iceberg/TestV4ManifestReader.java:
##########
@@ -447,6 +485,86 @@ public void inheritanceAddedFileSequenceNumber(FileFormat
format) throws IOExcep
.containsExactly(MANIFEST_SEQ, 500L);
}
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void inheritanceFirstRowId(FileFormat format) throws IOException {
+ Tracking trackingWithoutFirstRowId =
+ new TrackingStruct(EntryStatus.ADDED, SNAPSHOT_ID, null, null, null,
null, null, null);
+ TrackedFile withoutFirstRowId =
+ unpartitionedDataFile(trackingWithoutFirstRowId,
"s3://bucket/table/file-a.parquet");
+ Tracking trackingWithFirstRowId =
+ new TrackingStruct(EntryStatus.EXISTING, SNAPSHOT_ID, null, null,
null, 5_000L, null, null);
+ TrackedFile withFirstRowId =
+ unpartitionedDataFile(trackingWithFirstRowId,
"s3://bucket/table/file-c.parquet");
+
+ ManifestFile manifest =
+ writeManifest(
+ format, UNPARTITIONED_TYPE, ImmutableList.of(withoutFirstRowId,
withFirstRowId));
+ when(manifest.firstRowId()).thenReturn(10_000L);
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
ID_PARTITIONING_SPECS)
+ .metricsConfig(METRICS_CONFIG);
+ List<TrackedFile> actual = read(builder);
+
+ assertThat(actual)
+ .extracting(file -> file.tracking().firstRowId())
+ .containsExactly(10_000L, 5_000L);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void inheritanceUncommittedSkipsFirstRowId(FileFormat format) throws
IOException {
+ Tracking trackingWithoutFirstRowId =
+ new TrackingStruct(EntryStatus.ADDED, SNAPSHOT_ID, null, null, null,
null, null, null);
+ TrackedFile withoutFirstRowId =
+ unpartitionedDataFile(trackingWithoutFirstRowId,
"s3://bucket/table/file-a.parquet");
+ Tracking trackingWithFirstRowId =
+ new TrackingStruct(EntryStatus.EXISTING, SNAPSHOT_ID, null, null,
null, 5_000L, null, null);
+ TrackedFile withFirstRowId =
+ unpartitionedDataFile(trackingWithFirstRowId,
"s3://bucket/table/file-c.parquet");
+
+ ManifestFile manifest =
+ writeManifest(
+ format, UNPARTITIONED_TYPE, ImmutableList.of(withoutFirstRowId,
withFirstRowId));
+ when(manifest.firstRowId()).thenReturn(10_000L);
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.uncommitted(manifest, IO, TABLE_SCHEMA,
ID_PARTITIONING_SPECS)
+ .metricsConfig(METRICS_CONFIG);
+ List<TrackedFile> actual = read(builder);
+
+ assertThat(actual)
+ .extracting(file -> file.tracking().firstRowId())
+ .containsExactly(null, 5_000L);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void inheritanceFirstRowIdNullFromUpgradedTable(FileFormat format)
throws IOException {
+ Tracking trackingWithoutFirstRowId =
+ new TrackingStruct(EntryStatus.ADDED, SNAPSHOT_ID, null, null, null,
null, null, null);
+ TrackedFile withoutFirstRowId =
+ unpartitionedDataFile(trackingWithoutFirstRowId,
"s3://bucket/table/file-a.parquet");
+ Tracking trackingWithFirstRowId =
+ new TrackingStruct(EntryStatus.EXISTING, SNAPSHOT_ID, null, null,
null, 5_000L, null, null);
+ TrackedFile withFirstRowId =
+ unpartitionedDataFile(trackingWithFirstRowId,
"s3://bucket/table/file-c.parquet");
+
+ ManifestFile manifest =
+ writeManifest(
+ format, UNPARTITIONED_TYPE, ImmutableList.of(withoutFirstRowId,
withFirstRowId));
+ // first row ID is null in upgraded tables, which is propagated to all
files
Review Comment:
Yeah, this is basically a violation so it doesn't matter what behavior we
have. But I think it is good to be consistent with the behavior of v3.
--
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]
