Fokko commented on code in PR #8980:
URL: https://github.com/apache/iceberg/pull/8980#discussion_r1435150752
##########
core/src/main/java/org/apache/iceberg/MicroBatches.java:
##########
@@ -92,7 +92,7 @@ private static List<Pair<ManifestFile, Integer>>
indexManifests(
for (ManifestFile manifest : manifestFiles) {
manifestIndexes.add(Pair.of(manifest, currentFileIndex));
- currentFileIndex += manifest.addedFilesCount() +
manifest.existingFilesCount();
+ currentFileIndex += manifest.addedFilesCount();
Review Comment:
I think it is relatively easy to reason about this part because it is used
only in one path.
The existing entries will be entries of data files that are already part of
a manifest. For example, when you rewrite the metadata; for example because you
use a lot of fast-appends. With a fast append, a new manifest will be written
and added to the manifest-list. At some point, you want to combine these
manifests into a bigger one to reduce the number of calls to the object store
and make the planning part faster.
We want to skip when only the metadata has been rewritten, so I think this
change is correct.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
