anoopj commented on code in PR #16958:
URL: https://github.com/apache/iceberg/pull/16958#discussion_r3521129139


##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(

Review Comment:
   I think the spec needs to be a bit clear on this. I have a slight preference 
to make it optional for data/delete files as well, for truly unpartitioned 
cases. I will leave this thread open till we resolve this. 



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {

Review Comment:
   Yes, row lineage isn't wired up in this reader yet.



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())
+            .setRootType(TrackedFileStruct.class)
+            .setCustomType(TrackedFile.TRACKING.fieldId(), 
TrackingStruct.class)
+            .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), 
DeletionVectorStruct.class)
+            .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), 
ManifestInfoStruct.class)
+            .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class)
+            .reuseContainers()
+            .build();
+    addCloseable(reader);
+    return reader;
+  }
+
+  private TrackedFile prepare(TrackedFile trackedFile) {
+    Tracking tracking = trackedFile.tracking();
+    // manifestLocation is not stored in the manifest; the reader fills it 
from the file location.
+    // manifestPos is filled from ROW_POSITION while reading the tracking 
struct.
+    if (tracking instanceof TrackingStruct) {
+      ((TrackingStruct) tracking).setManifestLocation(file.location());
+    }
+
+    return trackedFile;
+  }
+
+  private Schema readSchema() {
+    // content_stats is not projected yet, so build the schema with an empty 
stats type

Review Comment:
   Removed



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())

Review Comment:
   That makes sense.  Added content_type as always projected.



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,276 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())
+            .setRootType(TrackedFileStruct.class)
+            .setCustomType(TrackedFile.TRACKING.fieldId(), 
TrackingStruct.class)
+            .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), 
DeletionVectorStruct.class)
+            .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), 
ManifestInfoStruct.class)
+            .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class)
+            .reuseContainers()
+            .build();
+    addCloseable(reader);
+    return reader;
+  }
+
+  private TrackedFile prepare(TrackedFile trackedFile) {
+    Tracking tracking = trackedFile.tracking();
+    // manifestLocation is not stored in the manifest; the reader fills it 
from the file location.
+    // manifestPos is filled from ROW_POSITION while reading the tracking 
struct.
+    if (tracking instanceof TrackingStruct) {
+      ((TrackingStruct) tracking).setManifestLocation(file.location());
+    }
+
+    return trackedFile;
+  }
+
+  private Schema readSchema() {
+    // content_stats is not projected yet, so build the schema with an empty 
stats type
+    Types.StructType fullType =
+        TrackedFile.schemaWithContentStats(partitionType, 
Types.StructType.of());
+    boolean unpartitioned = partitionType.fields().isEmpty();
+
+    Set<Integer> projectedIds = null;
+    if (fileProjection != null) {
+      projectedIds = Sets.newHashSet();
+      for (Types.NestedField field : fileProjection.asStruct().fields()) {
+        projectedIds.add(field.fieldId());
+      }
+
+      // tracking carries the status used to filter live files and is always 
projected
+      projectedIds.add(TrackedFile.TRACKING.fieldId());

Review Comment:
   Yes, please see my comment above.



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())
+            .setRootType(TrackedFileStruct.class)
+            .setCustomType(TrackedFile.TRACKING.fieldId(), 
TrackingStruct.class)
+            .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), 
DeletionVectorStruct.class)
+            .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), 
ManifestInfoStruct.class)
+            .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class)
+            .reuseContainers()
+            .build();
+    addCloseable(reader);
+    return reader;
+  }
+
+  private TrackedFile prepare(TrackedFile trackedFile) {
+    Tracking tracking = trackedFile.tracking();
+    // manifestLocation is not stored in the manifest; the reader fills it 
from the file location.
+    // manifestPos is filled from ROW_POSITION while reading the tracking 
struct.
+    if (tracking instanceof TrackingStruct) {
+      ((TrackingStruct) tracking).setManifestLocation(file.location());
+    }
+
+    return trackedFile;
+  }
+
+  private Schema readSchema() {
+    // content_stats is not projected yet, so build the schema with an empty 
stats type
+    Types.StructType fullType =
+        TrackedFile.schemaWithContentStats(partitionType, 
Types.StructType.of());
+    boolean unpartitioned = partitionType.fields().isEmpty();
+
+    Set<Integer> projectedIds = null;
+    boolean fullTracking = true;
+    if (fileProjection != null) {
+      projectedIds =
+          fileProjection.asStruct().fields().stream()
+              .map(Types.NestedField::fieldId)
+              .collect(Collectors.toCollection(Sets::newHashSet));
+
+      // read the full tracking struct only when the caller requests it; 
otherwise force-add a

Review Comment:
   I initially left them out so that inheritance can be a separate PR. I think 
it makes sense to add them here. 
   
   But I think we should probably not always project out the whole tracking. 
`dv_snapshot_id`, `replaced/deleted_positions` are required for CDF readers.  
Especially the latter two fields are fairly large bitmaps.  We should probably 
not project them on the scan path. 
   
   I have updated the PR such that we always project out the fields we need for 
scan but not for CDF. 



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());

Review Comment:
   Changed. I was trying to be consistent in error messages with the current 
reader. 



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())
+            .setRootType(TrackedFileStruct.class)
+            .setCustomType(TrackedFile.TRACKING.fieldId(), 
TrackingStruct.class)
+            .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), 
DeletionVectorStruct.class)
+            .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), 
ManifestInfoStruct.class)
+            .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class)
+            .reuseContainers()
+            .build();
+    addCloseable(reader);
+    return reader;
+  }
+
+  private TrackedFile prepare(TrackedFile trackedFile) {
+    Tracking tracking = trackedFile.tracking();
+    // manifestLocation is not stored in the manifest; the reader fills it 
from the file location.
+    // manifestPos is filled from ROW_POSITION while reading the tracking 
struct.
+    if (tracking instanceof TrackingStruct) {
+      ((TrackingStruct) tracking).setManifestLocation(file.location());
+    }
+
+    return trackedFile;
+  }
+
+  private Schema readSchema() {
+    // content_stats is not projected yet, so build the schema with an empty 
stats type
+    Types.StructType fullType =
+        TrackedFile.schemaWithContentStats(partitionType, 
Types.StructType.of());
+    boolean unpartitioned = partitionType.fields().isEmpty();
+
+    Set<Integer> projectedIds = null;
+    boolean fullTracking = true;
+    if (fileProjection != null) {
+      projectedIds =
+          fileProjection.asStruct().fields().stream()
+              .map(Types.NestedField::fieldId)
+              .collect(Collectors.toCollection(Sets::newHashSet));
+
+      // read the full tracking struct only when the caller requests it; 
otherwise force-add a
+      // minimal tracking carrying just the status used to filter live files
+      fullTracking = projectedIds.contains(TrackedFile.TRACKING.fieldId());
+      projectedIds.add(TrackedFile.TRACKING.fieldId());
+
+      // project spec_id for partition filtering
+      if (!partitionEvaluators.isEmpty()) {
+        projectedIds.add(TrackedFile.SPEC_ID.fieldId());
+      }
+    }
+
+    List<Types.NestedField> fields = Lists.newArrayList();
+    for (Types.NestedField field : fullType.fields()) {
+      if (projectedIds != null && !projectedIds.contains(field.fieldId())) {
+        continue;
+      }
+
+      if (field.fieldId() == TrackedFile.TRACKING.fieldId()) {
+        fields.add(
+            Types.NestedField.required(
+                TrackedFile.TRACKING.fieldId(),
+                TrackedFile.TRACKING.name(),
+                fullTracking ? TrackingStruct.BASE_TYPE : STATUS_TRACKING,
+                TrackedFile.TRACKING.doc()));
+      } else if (field.fieldId() == TrackedFile.CONTENT_STATS_ID) {

Review Comment:
   I do agree: we will be projecting only the field IDs we need. This is just a 
placeholder for now, as the interfaces for content stats are being revamped.



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),
+        specId,
+        partitionEvaluators.keySet(),
+        file.location());
+
+    boolean matches = evaluator.eval(projection.wrap(trackedFile.partition()));
+    if (!matches) {
+      if (content == FileContent.DATA) {
+        scanMetrics.skippedDataFiles().increment();
+      } else {
+        scanMetrics.skippedDeleteFiles().increment();
+      }
+    }
+
+    return matches;
+  }
+
+  private CloseableIterable<TrackedFile> open() {
+    FileFormat format = FileFormat.fromFileName(file.location());
+    Preconditions.checkArgument(
+        format != null, "Unable to determine format of manifest: %s", 
file.location());
+
+    CloseableIterable<TrackedFile> reader =
+        InternalData.read(format, file)
+            .project(readSchema())
+            .setRootType(TrackedFileStruct.class)
+            .setCustomType(TrackedFile.TRACKING.fieldId(), 
TrackingStruct.class)
+            .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), 
DeletionVectorStruct.class)
+            .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), 
ManifestInfoStruct.class)
+            .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class)
+            .reuseContainers()
+            .build();
+    addCloseable(reader);
+    return reader;
+  }
+
+  private TrackedFile prepare(TrackedFile trackedFile) {
+    Tracking tracking = trackedFile.tracking();
+    // manifestLocation is not stored in the manifest; the reader fills it 
from the file location.
+    // manifestPos is filled from ROW_POSITION while reading the tracking 
struct.
+    if (tracking instanceof TrackingStruct) {
+      ((TrackingStruct) tracking).setManifestLocation(file.location());
+    }
+
+    return trackedFile;
+  }
+
+  private Schema readSchema() {
+    // content_stats is not projected yet, so build the schema with an empty 
stats type
+    Types.StructType fullType =
+        TrackedFile.schemaWithContentStats(partitionType, 
Types.StructType.of());
+    boolean unpartitioned = partitionType.fields().isEmpty();
+
+    Set<Integer> projectedIds = null;
+    boolean fullTracking = true;
+    if (fileProjection != null) {
+      projectedIds =
+          fileProjection.asStruct().fields().stream()
+              .map(Types.NestedField::fieldId)
+              .collect(Collectors.toCollection(Sets::newHashSet));
+
+      // read the full tracking struct only when the caller requests it; 
otherwise force-add a
+      // minimal tracking carrying just the status used to filter live files
+      fullTracking = projectedIds.contains(TrackedFile.TRACKING.fieldId());
+      projectedIds.add(TrackedFile.TRACKING.fieldId());
+
+      // project spec_id for partition filtering
+      if (!partitionEvaluators.isEmpty()) {

Review Comment:
   Good catch, fixed such that we will be force-projecting partition (alongside 
spec_id) when a filter is active. Also added 
testPartitionFilterForceProjectsFilterFields() as a test



##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -0,0 +1,275 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.iceberg.expressions.Evaluator;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.expressions.Expressions;
+import org.apache.iceberg.expressions.Projections;
+import org.apache.iceberg.io.CloseableGroup;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.CloseableIterator;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.metrics.ScanMetrics;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.StructProjection;
+
+/** Reader that reads a v4+ manifest file as {@link TrackedFile}s. */
+class V4ManifestReader extends CloseableGroup implements 
CloseableIterable<TrackedFile> {
+  // minimal tracking projection used when the caller does not request tracking
+  private static final Types.StructType STATUS_TRACKING = 
Types.StructType.of(Tracking.STATUS);
+
+  private final InputFile file;
+  private final Types.StructType partitionType;
+  private final Schema fileProjection;
+  private final ScanMetrics scanMetrics;
+
+  // partition pruning state, keyed by spec ID; empty when no filtering is 
required
+  private final Map<Integer, Evaluator> partitionEvaluators;
+  private final Map<Integer, StructProjection> partitionProjections;
+
+  private V4ManifestReader(
+      InputFile file,
+      Types.StructType partitionType,
+      Map<Integer, Evaluator> partitionEvaluators,
+      Map<Integer, StructProjection> partitionProjections,
+      Schema fileProjection,
+      ScanMetrics scanMetrics) {
+    this.file = file;
+    this.partitionType = partitionType;
+    this.partitionEvaluators = partitionEvaluators;
+    this.partitionProjections = partitionProjections;
+    this.fileProjection = fileProjection;
+    this.scanMetrics = scanMetrics;
+  }
+
+  static Builder builder(
+      InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> 
specsById) {
+    return new Builder(file, tableSchema, specsById);
+  }
+
+  /** Returns all tracked files in this manifest, regardless of status. */
+  CloseableIterable<TrackedFile> allFiles() {
+    return files(false /* all files */);
+  }
+
+  /** Returns tracked files whose tracking {@link Tracking#isLive() is live}. 
*/
+  CloseableIterable<TrackedFile> liveFiles() {
+    return files(true /* only live files */);
+  }
+
+  /** Returns live tracked files, each as an independent copy. */
+  @Override
+  public CloseableIterator<TrackedFile> iterator() {
+    return CloseableIterable.transform(liveFiles(), 
TrackedFile::copy).iterator();
+  }
+
+  private CloseableIterable<TrackedFile> files(boolean onlyLive) {
+    CloseableIterable<TrackedFile> entries = 
CloseableIterable.transform(open(), this::prepare);
+    if (!partitionEvaluators.isEmpty()) {
+      entries = CloseableIterable.filter(entries, this::matchesPartition);
+    }
+
+    if (onlyLive) {
+      entries = CloseableIterable.filter(entries, entry -> 
entry.tracking().isLive());
+    }
+
+    return entries;
+  }
+
+  private boolean matchesPartition(TrackedFile trackedFile) {
+    FileContent content = trackedFile.contentType();
+    if (content == FileContent.DATA_MANIFEST || content == 
FileContent.DELETE_MANIFEST) {
+      // manifest references are expanded later and are not pruned by the 
partition filter
+      return true;
+    }
+
+    Integer specId = trackedFile.specId();
+    Evaluator evaluator = specId != null ? partitionEvaluators.get(specId) : 
null;
+    StructProjection projection = specId != null ? 
partitionProjections.get(specId) : null;
+    Preconditions.checkState(
+        evaluator != null && projection != null,
+        "Cannot apply partition filter: file %s has spec ID %s, not one of the 
known specs %s "
+            + "in manifest %s",
+        trackedFile.location(),

Review Comment:
   Good point. Changed to `file.location()`



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