michaelchendd opened a new issue, #2037: URL: https://github.com/apache/iceberg-go/issues/2037
### Proposed Change When manifest merging is enabled, a merge bin may contain only `DELETED` entries from snapshots older than the snapshot currently being committed. Today, `manifestMergeManager.createManifest` creates a `ManifestWriter` before inspecting the entries: https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L362-L407 Historical `DELETED` entries are intentionally not copied into the merged manifest. If every entry in the bin is such a deletion, no entries are written. Closing the already-created writer then returns: ```text empty manifest file has been written ``` This causes the commit to fail even though producing no replacement manifest appears to be a valid result for that bin. This error has occurred intermittently during production commits. It has not yet been confirmed that this exact path caused those occurrences, but the edge case exists independently in the current implementation. Proposed change: - Skip historical `DELETED` entries before creating the output writer. - Create the writer lazily when the first retained entry is encountered. - If no entries are retained, return no output manifest and no error. - Update `mergeGroup` to append the result only when a non-nil manifest was created. - Keep `ManifestWriter`'s existing empty-manifest validation unchanged. The merge caller, rather than the general-purpose writer, should handle this case. Correctness requirements: - A merge bin containing only historical `DELETED` entries must produce no manifest and no error. - `DELETED` entries belonging to the current snapshot must still be retained. - `ADDED` entries belonging to the current snapshot must still be written as added. - Older added entries and existing entries must still be written as existing. - Manifest read errors must continue to propagate and must not be treated as an empty merge. - Writers and underlying output files must still be closed on success and failure. - A nil manifest must never be added to the resulting manifest list. - `ManifestWriter.Close` must continue returning `ErrEmptyManifest` when callers directly attempt to write an empty manifest. Validation: Add regression tests covering: - a merge bin containing only historical deletions; - current-snapshot deletions being retained; - a read error occurring before an output writer is created; - existing non-empty merge behavior remaining unchanged. For the historical-deletion-only case, the test should also verify that no output writer was opened. Backward compatibility: - No public API changes are required. - Non-empty manifest merges retain their existing behavior. - Manifest merging disabled through table properties is unaffected. - The only behavior change is that an all-filtered merge bin produces no output instead of failing the commit. Related code: - Manifest creation and entry filtering: https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L362-L407 - Merge result handling: https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L418-L434 - Empty manifest validation: https://github.com/apache/iceberg-go/blob/main/manifest.go#L1429-L1454 A candidate implementation and regression tests are available here: https://github.com/DataDog/iceberg-go/commit/61d48d345ed698d2860c76ffdde56ad51bf8d1a6 Spec reference: Deleted manifest entries are informational and are not used when planning scans: https://iceberg.apache.org/spec/#manifest-entry-fields No specification change is required. This change only allows the existing historical-delete filtering behavior to produce no replacement manifest. ### Willingness to contribute - [ ] I can contribute this improvement/feature independently - [x] I would be willing to contribute this improvement/feature with guidance from the Iceberg community - [ ] I cannot contribute this improvement/feature at this time ### Specifications - [x] Table - [ ] View - [ ] REST - [ ] Puffin - [ ] Encryption - [ ] Other -- 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]
