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