rdblue commented on code in PR #18853:
URL: https://github.com/apache/iceberg/pull/18853#discussion_r4543090411


##########
core/src/main/java/org/apache/iceberg/V4Metadata.java:
##########
@@ -541,4 +546,608 @@ public Long pos() {
       return null;
     }
   }
+
+  /** Base wrapper used to write a {@link TrackedFile} using the v4 tracked 
file schema. */
+  private abstract static class TrackedFileWrapper implements StructLike {
+    private static final int SIZE =
+        TrackedFile.schema(Types.StructType.of(), 
Types.StructType.of()).columns().size();
+
+    private final Map<Integer, PartitionSpec> specsById;
+    private final Map<Integer, StructProjection> partitionProjections;
+    private final ContentStatsWrapper contentStats;
+    private final DeletionVectorWrapper deletionVector = new 
DeletionVectorWrapper();
+    private final List<ColumnFileWrapper> columnFileWrappers = 
Lists.newArrayList();
+    private final TrackingWrapper tracking;
+    private final ManifestInfoWrapper manifestInfo;
+
+    private TrackedFile wrapped = null;
+    private Integer specId = null;
+
+    private TrackedFileWrapper(
+        Map<Integer, PartitionSpec> specsById,
+        Types.StructType partitionType,
+        Types.StructType contentStatsType,
+        TrackingWrapper tracking,
+        ManifestInfoWrapper manifestInfo) {
+      validatePartitionType(specsById.values(), partitionType);
+      this.specsById = specsById;
+      this.partitionProjections = partitionProjections(specsById.values(), 
partitionType);
+      this.contentStats = new ContentStatsWrapper(contentStatsType);
+      this.tracking = tracking;
+      this.manifestInfo = manifestInfo;
+    }
+
+    protected TrackingWrapper tracking() {
+      return tracking;
+    }
+
+    protected void wrapFile(TrackedFile file) {
+      Preconditions.checkArgument(file != null, "Invalid file: null");
+
+      FileContent content = file.contentType();
+      if (content == FileContent.DATA) {
+        Preconditions.checkArgument(
+            file.manifestInfo() == null,
+            "Invalid data file, manifest info must be null: %s",
+            file.location());
+      } else if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+        Preconditions.checkArgument(
+            file.manifestInfo() != null,
+            "Invalid manifest, missing manifest info: %s",
+            file.location());
+        Preconditions.checkArgument(
+            file.deletionVector() == null,
+            "Invalid manifest, deletion vector must be null: %s",
+            file.location());
+      } else {
+        throw new IllegalArgumentException(
+            String.format("Cannot write %s to a v4 manifest: %s", content, 
file.location()));
+      }
+
+      Integer fileSpecId = file.specId();
+      PartitionSpec spec = fileSpecId != null ? specsById.get(fileSpecId) : 
null;
+      Preconditions.checkArgument(
+          fileSpecId == null || spec != null,
+          "Cannot find partition spec for spec ID: %s",
+          fileSpecId);
+
+      this.wrapped = file;
+      this.specId = spec != null && !spec.isUnpartitioned() ? fileSpecId : 
null;
+    }
+
+    @Override
+    public int size() {
+      return SIZE;
+    }
+
+    @Override
+    public <T> void set(int pos, T value) {
+      throw new UnsupportedOperationException("Cannot modify 
TrackedFileWrapper wrapper via set");
+    }
+
+    @Override
+    public <T> T get(int pos, Class<T> javaClass) {
+      return javaClass.cast(get(pos));
+    }
+
+    private Object get(int pos) {
+      return switch (pos) {
+        case 0 -> tracking;
+        case 1 -> wrapped.contentType().id();
+        case 2 -> wrapped.location();
+        case 3 -> wrapped.fileFormat().toString();
+        case 4 -> wrapped.recordCount();
+        case 5 -> wrapped.fileSizeInBytes();
+        case 6 -> specId;
+        case 7 -> unionPartition();
+        case 8 -> contentStats.wrap(wrapped.contentStats());
+        case 9 -> wrapped.tightStats();
+        case 10 -> wrapped.sortOrderId();
+        case 11 -> deletionVector.wrap(wrapped.deletionVector());
+        case 12 ->
+            wrapped.manifestInfo() != null ? 
manifestInfo.wrap(wrapped.manifestInfo()) : null;

Review Comment:
   I think this should be null-tolerant like `DeletionVectorWrapper` so that 
this doesn't need to check for null.



-- 
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]

Reply via email to