rdblue commented on code in PR #16936:
URL: https://github.com/apache/iceberg/pull/16936#discussion_r4021065743
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -572,4 +1134,48 @@ private static PartitionSpec resolveSpec(
throw new IllegalArgumentException(
"Cannot find unpartitioned spec in specs: " + specsById.keySet());
}
+
+ /**
+ * Projects the file's per-spec partition tuple into the target partition
schema by field ID.
+ * Fields present in the target but not in the file's spec land as null.
+ */
+ private static StructProjection projectPartition(
+ ContentFile<?> file, Types.StructType partitionType) {
+ StructLike partition = file.partition();
+ Types.StructType sourceType;
+ if (partition instanceof PartitionData) {
+ sourceType = ((PartitionData) partition).getPartitionType();
+ } else if (partition == null || partition.size() == 0) {
+ sourceType = Types.StructType.of();
+ } else {
+ throw new IllegalArgumentException(
+ String.format(
+ "Cannot project partition for %s: partition type is unavailable
for %s",
+ file.location(), partition));
+ }
+
+ return StructProjection.createAllowMissing(sourceType,
partitionType).wrap(partition);
+ }
+
+ /** Resolves record_count for a manifest-reference row from its per-status
file counts. */
+ private static long resolveRecordCount(ManifestFile manifest) {
Review Comment:
The record count is the count of records in the file. For manifests, that is
the number of rows (`TrackedFile` instances) in the manifest. I think that
should be stated more directly in comments.
This also isn't correct. v1 didn't require a manifest list and had only a
list of manifest locations, so these counts were optional in v1. They were made
required in v2, but we can't assume that these are defined. If one isn't
defined, then this will produce an incorrect record count.
I think that the right behavior is to fail rather than produce incorrect
data. These checks should all throw `IllegalStateException` if any count is
missing. In addition, because this is wrapping a v3 or earlier file, it should
also assert that `replacedFilesCount` is 0.
--
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]
