stevenzwu commented on code in PR #16936:
URL: https://github.com/apache/iceberg/pull/16936#discussion_r3457444883
##########
api/src/main/java/org/apache/iceberg/ManifestFile.java:
##########
@@ -186,6 +186,26 @@ default boolean hasDeletedFiles() {
/** Returns the total number of rows in all files with status DELETED in the
manifest file. */
Long deletedRowsCount();
+ /**
+ * Returns the number of files with status REPLACED in the manifest file, or
null if not tracked.
+ *
+ * <p>REPLACED files are the prior-state entries of v4 REPLACED/MODIFIED
pairs and are not live.
+ * Returns null for manifest files written by pre-v4 writers.
+ */
+ default Integer replacedFilesCount() {
Review Comment:
Acknowledging the general guidance: public-interface changes should be
minimized and delayed. Justification for keeping these two on the interface in
this PR:
**In-tree consumers** (this PR + close follow-ups):
- `ContentEntryAdapters.manifestInfo()` (this PR) populates the v4 root
manifest's `manifest_info` struct from `manifest.replacedFilesCount()` /
`replacedRowsCount()`.
- `MergingSnapshotProducer.validateAddedDVs` (follow-up phase) filters
concurrent data manifests by `replacedFilesCount > 0` to detect v4 colocated-DV
write conflicts — without it, every concurrent data manifest must be
deep-scanned.
**The cast alternative is unsafe.** `V1/V2/V3/V4
Metadata.ManifestFileWrapper` are delegate wrappers used inside
`ManifestListWriter`. `instanceof GenericManifestFile` returns false for them
and would silently mask the v4 metadata they delegate to a wrapped
`GenericManifestFile`. The interface-default pattern matches v3's existing
extensions (`firstRowId`, `keyMetadata`, `containsNaN`) and survives the
delegation chain.
**A `TrackedFile`-based parallel v4 API was considered.** Would require
rewriting `ManifestGroup` and every engine that consumes `FileScanTask`. The
cost is disproportionate to the savings.
**Projected total v4 surface**: 5–8 default methods across the full v4
stack, in the same shape as v3's row-lineage / encryption / containsNaN
expansion. `writerFormatVersion()` was originally part of this PR but has no
Phase 1 internal consumer, so it's been deferred to the root-manifest
writer/reader PR where it has a direct caller — same minimal-surface reasoning.
---
**Projected new default methods across the full v4 stack:**
_Already landed or in flight (3)_:
- `default Integer replacedFilesCount()` — this PR. Read by
`ContentEntryAdapters.manifestInfo()` and
`MergingSnapshotProducer.validateAddedDVs` (follow-up).
- `default Long replacedRowsCount()` — this PR. Companion to
`replacedFilesCount`.
- `default int writerFormatVersion()` — root-manifest writer/reader PR. Read
by `RootManifestWriter.addEntry()` for per-leaf format dispatch in the root
manifest.
_Likely next wave (+2), gated on `ManifestInfo` schema extension_:
- `default Integer modifiedFilesCount()` — v4 MODIFIED entry count (live
half of REPLACED/MODIFIED pairs).
- `default Long modifiedRowsCount()` — companion.
_Possible if specific v4 spec features land (+2 to +3)_:
- `default ByteBuffer deletedManifestPositions()` — root-level manifest DV
bitmaps (currently out-of-scope optimization).
- `default ByteBuffer replacedManifestPositions()` — same; could be merged
into a single method.
- `default String affiliatedManifestPath()` — data/delete manifest affinity
for single-pass planning (currently out of scope).
_Explicitly NOT on `ManifestFile` (different abstraction layers)_:
- Per-data-file colocated DV → projected via
`TrackedFileAdapters.asDVDeleteFile()` from a manifest row.
- Snapshot-level metadata (summary metrics, root manifest location) →
`Snapshot` API.
- Direct data-file entries in v4 root manifest (small-write optimization) →
synthesized as single-entry `GenericManifestFile`, reuses existing shape — no
new methods.
--
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]
