Core: Add V4 scan task planner by anoopj · Pull Request #17541 · apache/iceberg

anoopj · 2026-08-06T18:30:15Z

Adds the planner that walks a v4 adaptive metadata tree and emits FileScanTasks. It reads the root manifest, emits a task per live DATA entry, and expands DATA_MANIFEST entries into their leaf manifests. Leaves are read lazily and, when a worker pool is supplied, expanded in parallel.

Keeping it package-private and not wired into any scan yet.

Support for the following features will be added in followup PRs.

Inherited tracking: sequence numbers, snapshot id, and first-row-id aren't applied
Manifest-level pruning: e.g. using the manifest_info count based short-circuit, stats/partition pruning
Delete content, manifest deletion vectors, encryption, and non-v4 leaves
Content cache + eager-fetch of manifests

anoopj · 2026-08-06T18:56:30Z

cc @danielcweeks @@amogh-jahagirdar @stevenzwu for review.

anoopj · 2026-08-06T19:23:12Z

+
+ CloseableIterable<FileScanTask> planFiles() {
+ List<CloseableIterable<FileScanTask>> taskGroups = Lists.newArrayList();
+ List<TrackedFile> dataFiles = Lists.newArrayList();


Note to reviewers: we are buffering the data files in the root in memory (leaves are lazily expanded). I think it should be okay and not worth making it lazy, but open to revisiting this.

Rename to rootDataFiles?

danielcweeks · 2026-08-06T20:21:29Z

+ * must be applied before this planner is wired into a scan or the delete-manifest matching path,
+ * since delete scoping compares a data file's sequence number against the delete's.
+ */
+class ManifestExpander {


Naming feels wrong here. Maybe ManifestScanner? Expand doesn't really make sense in terms of processing metadata and producing tasks.

Yeah agree with the naming concern, just tossing another one TaskPlanner?

Why wouldn't this be implemented within the DataTableScan -> BaseTableScan -> SnapshotScan hierarchy?

Actually, it's hard to see if this abstraction is right without seeing how it's consumed, do we have a PR that shows how this is used in the scan.planFiles() flow?
Edit: Sorry I was reviewing around the same time @danielcweeks was apparently, but effectively same question as he had ^^.

This is a great question. The intent here is that this is the V4 analog of the existing package-private ManifestGroup helper, and not a new node in the scan hierarchy.
Today DataTableScan.doPlanFiles() builds a ManifestGroup and calls .planFiles() to turn manifests into FileScanTasks. ManifestExpander plays the same role for the V4 metadata tree: doPlanFiles() (or a doPlanFilesV4() branch, dispatched on format version) will build one and call planFiles(), exactly mirroring the current flow.
So it lives below the scan hierarchy and is consumed by rather than being implemented within DataTableScan/BaseTableScan/SnapshotScan themselves.
You can see how this could be used in the prototype PR here.

Naming feels wrong here.
Expansion is a terminology we used in a past system that I worked on. I realize it's not standard database terminology, so we can change it. Looking for more suggestions from others.

I would be good with some name along the line of planner, like ScanTaskPlanner.

I liked ScanTaskPlanner the best. Renamed.

amogh-jahagirdar · 2026-08-06T20:29:03Z

+ * must be applied before this planner is wired into a scan or the delete-manifest matching path,
+ * since delete scoping compares a data file's sequence number against the delete's.
+ */
+class ManifestExpander {


Yeah agree with the naming concern, just tossing another one TaskPlanner?

amogh-jahagirdar · 2026-08-06T20:29:40Z

+ private final FileIO io;
+ private final InputFile rootManifest;
+ private final Map<Integer, PartitionSpec> specsById;
+ private final String tableLocation;


Is this needed for handling relative paths?

Yes, we need pipe it to the reader so that the path resolution happens.

amogh-jahagirdar · 2026-08-06T20:35:26Z

+ // delete content is only produced by upgraded trees; the 2-phase path is not built
+ // yet.


I'm not sure I follow this comment? This abstraction is for V4 entries only I thought, but for those we wouldn't need to two-phase plan right?

Ok but a V4 entry could be V3 leaf data manifests or V3 delete manifests , so I think this comment is just saying we're not quite handling it here in this code yet.

Correct. Improved the comment a bit.

amogh-jahagirdar · 2026-08-06T20:36:06Z

+ * must be applied before this planner is wired into a scan or the delete-manifest matching path,
+ * since delete scoping compares a data file's sequence number against the delete's.
+ */
+class ManifestExpander {


Actually, it's hard to see if this abstraction is right without seeing how it's consumed, do we have a PR that shows how this is used in the scan.planFiles() flow?
Edit: Sorry I was reviewing around the same time @danielcweeks was apparently, but effectively same question as he had ^^.

danielcweeks · 2026-08-06T20:37:38Z

+ boolean caseSensitive,
+ ScanMetrics scanMetrics,
+ ExecutorService executorService) {
+ this.io = io;


I think we also want to include a reference to the Table being scanned (that's consistent with other existing implementations). Keeping the FileIO separate is necessary for scan/planning which doesn't use the table's FileIO/credentials. I believe the table reference is also used extensively for things like converting to distributed plan and metadata tables. (This would be required with the existing scan heirerarchy).

I kept the surface minimal here to mirror ManifestGroup since this PR is just the standalone planner. You're right that the Table reference becomes necessary for the pieces that consume it e.g. distributed plan conversion, but those live in the scan-wiring layer that isn't in this PR yet.
I'd prefer to add the Table reference when that wiring lands and actually uses it.

I think this is good for now. I generally try to avoid passing Table around if we don't need it.

danielcweeks · 2026-08-06T20:48:09Z

+ if (!dataFiles.isEmpty()) {
+ taskGroups.add(
+ CloseableIterable.transform(
+ CloseableIterable.withNoopClose(dataFiles), this::createTask));
+ }
+
+ for (TrackedFile leaf : leafManifests) {
+ taskGroups.add(createLeafTasks(leaf));
+ }
+
+ if (executorService != null) {
+ return new ParallelIterable<>(taskGroups, executorService);
+ }


This stacks the immediately available root tasks with the lazy leaf processing in the same parallel backend. I would think you would compose this with the root tasks not going to the parallel backend followed by the parallel iterable.

That makes sense. Done.

danielcweeks · 2026-08-06T21:17:27Z

+ this.caseSensitive = caseSensitive;
+ this.scanMetrics = scanMetrics;
+ this.executorService = executorService;
+ this.taskContextCache = Caffeine.newBuilder().build(this::newTaskContext);


Why would we use caffine for this? There's no eviction/expiry/size and a tables really don't have that many specs. Just a map would be better here.

You are correct: Caffeine is an overkill. Changed to a map. I modeled this after ManifestGroup, which does the exact same thing. But probably not worth fixing ManifestGroup to avoid potential regressions.

danielcweeks · 2026-08-06T21:20:24Z

+ + leaf.location());
+ }
+
+ // applying manifest DV during expansion is not supported yet, so reject leaf with manifest DVs.


These comments aren't really helpful and just restate code. I'd drop the obvious ones like this.

Dropped them

danielcweeks · 2026-08-06T21:21:40Z

+ // the reader is opened and the leaf counted as scanned lazily, only when the result is
+ // iterated: a plan that is built and closed without iterating reads nothing, and each leaf
+ // increments scannedDataManifests exactly once.


This is unnecessary commentary.

stevenzwu · 2026-08-07T04:03:15Z

+ * assigned the data/file sequence numbers, snapshot id, and first-row-id they inherit from their
+ * parent, so {@code data_sequence_number}, {@code file_sequence_number}, and row-lineage columns
+ * ({@code _row_id}, {@code _last_updated_sequence_number}) read null for added files. Inheritance
+ * must be applied before this planner is wired into a scan or the delete-manifest matching path,


I assume distributed scan planning will be implemented in another class for v4.

I think yes, it will be in another class. similar to what we do currently.

stevenzwu · 2026-08-07T04:13:12Z

+ }
+ }
+
+ private FileScanTask createTaskFromLeafEntry(TrackedFile entry) {


nit: we often use the term "leaf manifest". should we call this createTaskFromDataFileEntry?

stevenzwu · 2026-08-07T04:33:11Z

+ * must be applied before this planner is wired into a scan or the delete-manifest matching path,
+ * since delete scoping compares a data file's sequence number against the delete's.
+ */
+class ManifestExpander {


I would be good with some name along the line of planner, like ScanTaskPlanner.

stevenzwu · 2026-08-07T20:40:54Z

+ * since delete scoping compares a data file's sequence number against the delete's.
+ */
+class ScanTaskPlanner {
+ private static final int FORMAT_VERSION = 4;


I would move this constant to TableMetadata and rename it to sth like in my PR.
https://github.com/apache/iceberg/pull/16936/changes#diff-c540a31e66b157a8f080433c82a29a070096d0e08c6578a0099153f1229bdb7a

Moved it to TableMetadata as MIN_FORMAT_VERSION_ADAPTIVE_MANIFEST_TREE = 4, matching your name in #16936 . Since #16936 isn't merged yet, the constant is defined in both PRs for now. Depending on which PR lands first, the other will have to rebase.

stevenzwu · 2026-08-07T21:20:23Z

+ CloseableIterable<FileScanTask> rootTasks =
+ CloseableIterable.transform(CloseableIterable.withNoopClose(dataFiles), this::createTask);
+
+ List<CloseableIterable<FileScanTask>> leafTasks = Lists.newArrayList();


nit: I would rename leafTasks to leafManifestTasks to make it more clear.

stevenzwu · 2026-08-07T21:25:43Z

+
+ private CloseableIterable<FileScanTask> createLeafTasks(TrackedFile leaf) {
+ // an upgraded tree can reference a legacy-format leaf
+ if (leaf.formatVersion() != FORMAT_VERSION) {


nit: switch to Preconditions.checkArguement. same for other validations below.

I switched the nested manifest check to Preconditions.checkArgument because it's a real validation . I left the others as UnsupportedOperationException on purpose: format-version, manifest-DV, because checkArgument would throw IllegalArgumentException and change that meaning.
Happy to reconsider if you'd prefer them as argument checks, but I think the UOE better signals that the input is valid, but the implementation can't support it yet.

stevenzwu · 2026-08-07T21:32:25Z

+
+ private FileScanTask createTaskFromDataFileEntry(TrackedFile entry) {
+ // the tree is at most two levels, so a leaf holds only DATA entries
+ if (entry.contentType() == FileContent.DATA_MANIFEST) {


nit: similar switch to Preconditions.checkArgument

stevenzwu · 2026-08-07T21:35:48Z

+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ void rootWithDataManifestExpandsLeaf(FileFormat format) throws IOException {


nit: rootWithLeafManifestEntriesOnly

stevenzwu · 2026-08-07T21:55:39Z

+ // without iterating scans only the root, not the leaf
+ planner.planFiles().close();
+
+ assertThat(metrics.scannedDataManifests().value()).isEqualTo(1L);


this remind me that we should probably add scannedDataManifests check for other normal read tests. e.g. leaf manifest read should have value 2.

Added a scannedDataManifests assertion to rootWithLeafManifestEntriesOnly

stevenzwu · 2026-08-08T03:16:45Z

+ writeManifest(
+ FileFormat.AVRO,
+ EMPTY_PARTITION,
+ ImmutableList.of(dataFile("data-from-avro-leaf.parquet", EMPTY_PARTITION_DATA)));


nit: suffix should be .avro

parquet is actually correct since this is the file name of the data file (which happens to come from an Avro manifest

stevenzwu · 2026-08-08T03:18:29Z

+ /* contentType= */ FileContent.DATA_MANIFEST,
+ /* formatVersion= */ formatVersion,
+ /* location= */ location,
+ /* fileFormat= */ FileFormat.PARQUET,


leaf manifest file is hardcoded to Parquet here. The Avro leaf test is not actually applied properly.

Done. They derive file format from location now.

stevenzwu · 2026-08-08T04:09:11Z

+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ void scannedManifestsCountsRootAndLeaves(FileFormat format) throws IOException {


if other tests check scanned metrics, we don't need to repeat this scenario here.

Yes, it is redundant. Removed scannedManifestsCountsRootAndLeaves

rdblue · 2026-08-14T21:06:57Z

+ * their leaf manifests. A data entry's colocated deletion vector is attached to its task as a
+ * {@link DeleteFile}.
+ *
+ * <p>Emitted tasks do not yet carry inherited tracking: leaf and root {@code DATA} entries are not


I don't think that this comment is helpful Javadoc. It is not this class's responsibility to handle inheritance and filling in values like first-row-id, that's the responsibility of readers. We don't need documentation that states what a class isn't responsible for.
If the purpose of this is to note that this isn't currently done and it is relevant that this is missing, then it should be in the PR description, or could be noted in non-Javadoc comments where it is relevant.

Removed . The gap is already in the PR description.

rdblue · 2026-08-14T21:09:14Z

+
+ static Builder builder(
+ FileIO io,
+ InputFile rootManifest,


It's strange to pass both FileIO and an InputFile instance. I would expect a ManifestFile reference instead for this. The io can be used to create an input file.

Changed it such that he planner takes the root manifest location now and opens it with the provided FileIO. I'll switch it to ManifestFile in a follow-up once the root encryption/keyMetadata path is sorted.

rdblue · 2026-08-14T21:09:50Z

+ this.executorService = executorService;
+ }
+
+ static Builder builder(


What criteria did you use to decide what is passed to create the builder vs passed to configure the builder?

I added factor args for the ones without any sensible default. This is also aligned to the builder in V4ManifestReader

I'm removing tableLocation from the manifest reader builder, so I think we should remove it here as well. It seems okay to pass specs, but I think we could default that to null. We can remove it later if we want to.

rdblue · 2026-08-14T21:11:49Z

+ leafManifests.add(entry);
+ break;
+ default:
+ // delete content appears only on upgraded trees


What does this mean? Is this to simplify and not support upgraded tables until later?

Yes, this only supports fully native v4 trees. Clarified the comment

gaborkaszab · 2026-08-24T13:58:43Z

+ }
+
 V4ManifestReader build() {
+ if (manifestDv != null) {


What's the point of the changes in this file? I guess it's setting the stage for some follow-up change, but at this point I don't think it adds any value.

Until we implement MDV, the rejection has to be done somewhere. The initial version of the PR had it done in the scan task planner: this is moving the rejection to the correct place (reader)

gaborkaszab · 2026-08-24T14:50:02Z

+ * their leaf manifests. A data entry's colocated deletion vector is attached to its task as a
+ * {@link DeleteFile}.
+ */
+class ScanTaskPlanner {


Just for my benefit to see the bigger picture: Is this meant to replace the functionality in SnapshotScan.doPlanFiles() and in ManifestGroup.planFiles() and such? I'd like to understand how this fits into the flow that is executed when a user runs table.newScan().planFiles(). Is there going to be an if on the table version to branch which code path is run?

That is correct. We will probably have a version specific branch in table scan layer. e.g. DataTableScan

I general comment about this class after taking a second look: I think this serves 2 different purposes merged into one.
Provide a "root manifest reader" that takes the location of the root and reads all the TrackedFiles including the ones in the leaves.
Do scan planning on top of the TrackedFile iterable we receive from the "root manifest reader"
I'd bet that we want to read the manifest tree for multiple purposes not just to perform "scan task planning" so I'm wondering if we can split the root reader part into some other class that can be reused ?
Another general, maybe unrelated question is if want to still support Snapshot's allManifest(), dataManifests(), deleteManifests() functionality for V4.

gaborkaszab · 2026-08-24T15:08:36Z

+ // out; direct DATA entries are buffered too, bounded by how many data files a tree keeps
+ // directly in the root. Leaf tasks stay lazy: planLeaf opens no reader until iterated.
+ scanMetrics.scannedDataManifests().increment();
+ try (CloseableIterable<TrackedFile> rootEntries =


Root manifest file can be encrypted. I see an exception throws for encrypted leaf manifests, for consistency shouldn't we Thor one here, or let the reader fail anyway because it's unable to parse the file?

We will add encryption support soon as a followup. Until then would it be reasonable to just let the reader fail?

I don't think we handle the lack of encryption support consistently. For root manifest we leave the reader/parser to fail, while for leaf manifests we have a check and throw an exception ourselves.

gaborkaszab · 2026-08-24T15:15:33Z

+ private final ExecutorService executorService;
+ private final Map<Integer, TaskContext> taskContextsBySpec = Maps.newConcurrentMap();
+
+ private ScanTaskPlanner(


Trying to wrap my head around encrypted root manifests. For that we'd need a couple of more inputs here. If I'm not mistaken, it would go through a similar mechanism as the pre-V4 manifest list location: it'd require a key-id from the snapshot, the encryption-keys from TableMetadata and maybe other stuff.
I'm wondering if we want to pass the proposed new interface for "files encrypted with key-id" (PR here) instead of rootManifestLocation

The PR doesn't handle encryption yet. In a followup, we will be converting this into a new ManifestFIle-like class that supports encryption. I think the abstraction you are adding in #17545 will be used here.

@gaborkaszab, we have been moving toward passing files to methods in EncryptingFileIO and I think that the root metadata would be handled similarly. EncryptingFileIO sets up the correct InputFile that carries the key metadata. We should continue to use this strategy to avoid needing to handle EncryptionManager in lots of different places.
For the root metadata file specifically, we will need to create a ManifestFile that represents it. I think that when we create that ManifestFile is when we will unwrap and resolve the key-id to key metadata.
Your PR is also on my TODO list for reviews, so I apologize if you've already thought through this and I'm repeating what you already know.

You're right, passing the right EncryptingFileIO to this class will take care of decrypting the root manifest file.

gaborkaszab · 2026-08-24T16:59:03Z

+ leafDataEntries.add(planLeaf(leaf));
+ }
+
+ CloseableIterable<TrackedFile> expandedLeafEntries =


If I'm not mistaken, this is able to read a 2-level metadata tree: root and leaves. I recall there was a discussion that theoretically we can have more levels in the tree. I'm not sure if there is anything in the proposed spec or anywhere that prevents writers doing that, but if not, I'm wondering we should be flexible enough on the read path to support such a layout.

V4 we intentionally are keeping it at two levels. For tables with multi-billion files, we will need to get to three levels or more, but that is outside the scope of v4.

I've just checked the spec proposal, and while we don't explicitly sat the tree was to have 2 levels, the way how we describe root manifest and data/delete manifests implies that there can't be more level ATM.

rdblue · 2026-08-27T23:33:14Z

+ * their leaf manifests. A data entry's colocated deletion vector is attached to its task as a
+ * {@link DeleteFile}.
+ */
+class ScanTaskPlanner {


Naming: I don't think this should identify ScanTask specifically. That's the root interface of the task hierarchy, which includes changelog task types, combined task types, and the simplest FileScanTask.
The purpose of this class is to produce matching FileScanTask instances for a specific metadata tree root. Maybe a better name for this would be FilePlanner since this implements planFiles.

Makes sense. An alternative isFileScanTaskPlanner that implements planFileScanTasks()? I have a slight preference for this name.

rdblue · 2026-08-27T23:48:02Z

+
+ // root is drained into these lists. Leaf references must be buffered so they can be fanned
+ // out; direct DATA entries are buffered too, bounded by how many data files a tree keeps
+ // directly in the root. Leaf tasks stay lazy: planLeaf opens no reader until iterated.


This comment applies to the lists above and the try block, but not the statement that it precedes.
It is also too long. It's obvious from the code that the data files and leaf manifests are kept in a list. It's also pretty obvious that the leaf manifests are going to be read as tasks and that we aren't going to emit the data files immediately. This comment doesn't seem very insightful or helpful to me.

rdblue · 2026-08-27T23:51:49Z

+ leafManifests.add(entry);
+ break;
+ default:
+ // delete manifests appear only on upgraded v2/v3 tables; that 2-phase path is not
+ // supported yet, so a natively-written v4 tree is the only supported input for now
+ throw new UnsupportedOperationException(
+ "Cannot plan content type in root manifest: " + entry.contentType());


"Unsupported file type in root manifest: %s"

rdblue · 2026-08-27T23:52:50Z

+ break;
+ default:
+ // delete manifests appear only on upgraded v2/v3 tables; that 2-phase path is not
+ // supported yet, so a natively-written v4 tree is the only supported input for now


Avoid making statements about this point in time because they tend to become stale. It would be easy for someone to add case DELETE_MANIFEST: above and not edit this comment.
I think a better approach is to add a case that specifically calls out what you're only stating in a comment here:
caseDELETE_MANIFEST: thrownewUnsupportedOperationException("v3 and earlier deletes are not yet supported"); default: thrownewUnsupportedOperationException("Unsupported file type in root manifest: " + ...);

rdblue · 2026-08-27T23:56:55Z

+ }
+
+ // read each leaf's data entries lazily; only leaf reads are worth the parallel backend, so the
+ // already-in-hand root data entries are concatenated in directly


worth the parallel backend
I don't think this comment makes sense. Only leaf manifests need to be read. Someone reading this code doesn't need to think about the data files that were stored in the root, only that this step is producing ClosableIterable instances that will read a leaf manifest file when they are iterated through.

rdblue · 2026-08-27T23:58:04Z

+ }
+
+ CloseableIterable<TrackedFile> expandedLeafEntries =
+ executorService != null


Did this drop the dataManifests.size() > 1 check that is happening in DataTableScan?

rdblue · 2026-08-27T23:58:47Z

+
+ // read each leaf's data entries lazily; only leaf reads are worth the parallel backend, so the
+ // already-in-hand root data entries are concatenated in directly
+ List<CloseableIterable<TrackedFile>> leafDataEntries = Lists.newArrayList();


Aren't all of these known to be files, and specifically data files? Why call them "entries"?

Since these are lazy, I would call this leafPlanTask. That mirrors the terminology we use in the server-side planning API.

rdblue · 2026-08-27T23:59:14Z

+
+ CloseableIterable<FileScanTask> planFiles() {
+ List<TrackedFile> rootDataFiles = Lists.newArrayList();
+ List<TrackedFile> leafManifests = Lists.newArrayList();


This should be List<ManifestFile>.

rdblue · 2026-08-28T00:01:47Z

+
+ private CloseableIterable<TrackedFile> planLeaf(TrackedFile leaf) {
+ // an upgraded tree can reference a legacy-format leaf
+ if (leaf.formatVersion() != TableMetadata.MIN_FORMAT_VERSION_ADAPTIVE_MANIFEST_TREE) {


It's odd to use != with MIN.
Where should this be enforced? I'd prefer it in the V4ManifestReader, which should be responsible for identifying the versions that it accepts. It could accept 4 and 5 after we release 5 and reject 6 just like TableMetadata would reject 6. I think this should be exposed through ManifestFile and checked there.

rdblue · 2026-08-28T00:04:17Z

+
+ @Override
+ public CloseableIterator<TrackedFile> iterator() {
+ // pass the known leaf size so the reader sizes the read instead of stat-ing the file


This is the responsibility of FileIO: https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java#L127

rdblue · 2026-08-28T00:04:27Z

+ public CloseableIterator<TrackedFile> iterator() {
+ // pass the known leaf size so the reader sizes the read instead of stat-ing the file
+ ByteBuffer manifestDv = leaf.manifestInfo() != null ? leaf.manifestInfo().dv() : null;
+ CloseableIterable<TrackedFile> entries =


files not entries.

rdblue · 2026-08-28T00:08:37Z

+ }
+
+ /** Lazily reads one leaf manifest's data entries; each iteration owns and counts its reader. */
+ private class LeafDataEntries extends CloseableGroup implements CloseableIterable<TrackedFile> {


I don't think that this class is needed. open produces a V4ManifestReader which is already a CloseableIterable<TrackedFile>. The only thing that this is doing differently is incrementing scannedDataManifests() when iterator is called. But we're already passing the scan metrics to the reader, so it can do that itself if metrics are present.
This is also transforming the iterator, but that doesn't need to be done in this iterator and in fact should not be. This is creating an Iterable and then immediately calling iterator() on it. We should compose iterables, rather than embed them like this to keep these steps independent.

rdblue · 2026-08-28T00:10:51Z

+ }
+ }
+
+ private CloseableIterable<TrackedFile> open(InputFile manifest, ByteBuffer manifestDv) {


This doesn't open the file, it creates a reader. This should probably be called reader.

rdblue · 2026-08-28T00:11:19Z

+ .filter(dataFilter)
+ .caseSensitive(caseSensitive)
+ .scanMetrics(scanMetrics)
+ .manifestDv(manifestDv)


I think the manifest DV should be passed into the reader via ManifestFile, which is where it will live. I'm also skeptical that it will be passed as ByteBuffer, so we should work on getting the mumbling implementation in. That way we can use a real bitmap.

rdblue · 2026-08-28T00:12:50Z

+ .build();
+ }
+
+ private TrackedFile requireDataEntry(TrackedFile entry) {


We don't use this pattern of returning a validated object.

rdblue · 2026-08-28T00:15:05Z

+ if (entry.contentType() != FileContent.DATA) {
+ throw new UnsupportedOperationException(
+ "Cannot plan content type in leaf manifest: " + entry.contentType());
+ }


Are you trying to have a more specific error message for manifests found in leaf files?
It doesn't make much sense to do this check twice. I'm also skeptical that this is the responsibility of the planner. Shouldn't this be done by a leaf manifest reader? Verifying that the files in a manifest are valid should be the reader's job.

anoopj · 2026-08-28T23:28:54Z

+
+ private TrackedFile requireDataEntry(TrackedFile entry) {
+ // the tree is at most two levels, so a leaf holds only DATA entries
+ Preconditions.checkArgument(


Check state instead?

anuragmantri · 2026-09-04T22:58:25Z

+ }
+
+ private FileScanTask createTask(TrackedFile trackedFile) {
+ DataFile dataFile = TrackedFileAdapters.asDataFile(trackedFile, specsById);


Just a heads-up from wiring the v4 read path through Spark's DSv2 scan, once a FileScanTask emitted by the v4 planner reaches Spark, its DataFile gets serialized to executors. The tracked-manifest read adapter that presents a v4 row as a DataFile needs to be Serializable for that to work.

Reintroduces the ManifestFile adapter closed in #16867, which stalled because ManifestFile had no way to represent a manifest deletion vector. The V4 scan task planner (#17541) plans over manifests through the ManifestFile API, so this adds ManifestFile.manifestDeletionVector() and has the adapter expose the tracked file's manifest DV.

Add manifest expander
93469d0

anoopj marked this pull request as draft August 6, 2026 18:30

anoopj marked this pull request as ready for review August 6, 2026 18:36

Trigger CI
829dad2

anoopj commented Aug 6, 2026
View reviewed changes

danielcweeks reviewed Aug 6, 2026
View reviewed changes

amogh-jahagirdar reviewed Aug 6, 2026
View reviewed changes

danielcweeks reviewed Aug 6, 2026
View reviewed changes

Comment threadcore/src/main/java/org/apache/iceberg/ScanTaskPlanner.java Outdated

stevenzwu reviewed Aug 7, 2026
View reviewed changes

anoopj changed the title ~~Core: Add V4 manifest expander~~Core: Add V4 scan task plannerAug 7, 2026

Incorporate feedback
529c539

github-actionsBot added the core label Aug 7, 2026

Fix CI
0ce4243

anoopj requested review from amogh-jahagirdar, danielcweeks and stevenzwu August 7, 2026 20:32

singhpk234 self-requested a review August 7, 2026 20:57

stevenzwu reviewed Aug 8, 2026
View reviewed changes

Pass known leaf size when opening V4 leaf manifests
2ad2101

anoopj mentioned this pull request Aug 12, 2026
Core: Add Eager fetch to parquet read path #17284
Merged

rdblue reviewed Aug 14, 2026
View reviewed changes

anoopj added this to V4: metadata tree Aug 19, 2026

PR feedback
b48c3d8

anoopj requested review from rdblue and stevenzwu August 19, 2026 18:56

github-actionsBot added the API label Aug 19, 2026

Revert unintended edit
1394ba4

github-actionsBot removed the API label Aug 19, 2026

Add a UT for MDVs in reader
1ee7d93

gaborkaszab reviewed Aug 24, 2026
View reviewed changes

rdblue reviewed Aug 27, 2026
View reviewed changes

rdblue reviewed Aug 28, 2026
View reviewed changes

anoopj commented Aug 28, 2026
View reviewed changes

anoopj mentioned this pull request Sep 2, 2026
API, Core: Add ManifestFile adapter for v4 tracked files #17932
Merged

anuragmantri reviewed Sep 4, 2026
View reviewed changes

		// delete content is only produced by upgraded trees; the 2-phase path is not built
		// yet.

Uh oh!

Conversation

anoopj commented Aug 6, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anoopj commented Aug 6, 2026

Uh oh!

anoopjAug 6, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdarAug 6, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdarAug 6, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stevenzwuAug 7, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anoopj commented Aug 6, 2026•
edited
Loading

anoopjAug 6, 2026•
edited
Loading

amogh-jahagirdarAug 6, 2026•
edited
Loading

amogh-jahagirdarAug 6, 2026•
edited
Loading

stevenzwuAug 7, 2026•
edited
Loading