yangshangqing95 commented on code in PR #18108:
URL: https://github.com/apache/iceberg/pull/18108#discussion_r4020172617
##########
core/src/test/java/org/apache/iceberg/TestV4ManifestReader.java:
##########
@@ -684,6 +684,38 @@ public void
filterMatchesFilesAcrossDisjointSpecs(FileFormat format) throws IOEx
}
}
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void narrowPartitionProjectionReadsFullUnionTuple(FileFormat format)
throws IOException {
+ PartitionSpec idSpec =
+ PartitionSpec.builderFor(TABLE_SCHEMA)
+ .withSpecId(0)
+ .add(1, 1000, "id", Transforms.identity())
+ .build();
+ PartitionSpec dataSpec =
+ PartitionSpec.builderFor(TABLE_SCHEMA)
+ .withSpecId(1)
+ .add(2, 1001, "data", Transforms.identity())
+ .build();
+ Map<Integer, PartitionSpec> specsById =
+ ImmutableMap.of(idSpec.specId(), idSpec, dataSpec.specId(), dataSpec);
+ Types.StructType unionType =
Partitioning.unionPartitionTypes(specsById.values());
+
+ TrackedFile file =
+ dataFile("by-data.parquet", dataSpec.specId(),
unionPartition(unionType, null, "x"));
+ ManifestFile manifest = writeManifest(format, unionType,
ImmutableList.of(file));
+
+ // select id, which this file's spec does not use; the reader must still
read the whole union
+ // tuple (including data) so partition() can project it onto the file's
data spec
+ try (V4ManifestReader reader =
+ V4ManifestReader.builder(manifest, io, specsById, TABLE_LOCATION)
+ .select("partition.id")
+ .build()) {
+ TrackedFile actual = Iterables.getOnlyElement(reader);
+ assertThat(actual.partition().get(0,
CharSequence.class)).hasToString("x");
+ }
+ }
+
Review Comment:
Nit, non-blocking: would it be worth adding a regression test that goes
through `TrackedFileAdapters` as well?
The current tests cover the projection behavior in `TrackedFileStruct` and
`V4ManifestReader` well, but the original failure shows up downstream when the
partition tuple is interpreted using the file's spec, e.g. during residual
evaluation or partition-constant injection.
A test along the lines of `V4ManifestReader` -> `TrackedFileAdapters` ->
`partition/residual evaluation` would help lock in the user-visible behavior
and guard against the adapter boundary regressing in the future.
--
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]
