Fokko commented on code in PR #1046: URL: https://github.com/apache/iceberg-python/pull/1046#discussion_r1713980714
########## pyiceberg/table/__init__.py: ########## @@ -3379,13 +3379,14 @@ def _copy_with_new_status(entry: ManifestEntry, status: ManifestEntryStatus) -> existing_entries = [] for entry in manifest_file.fetch_manifest_entry(io=self._io, discard_deleted=True): if strict_metrics_evaluator(entry.data_file) == ROWS_MUST_MATCH: + # Based on the metadata, it can be dropped right away deleted_entries.append(_copy_with_new_status(entry, ManifestEntryStatus.DELETED)) self._deleted_data_files.add(entry.data_file) - elif inclusive_metrics_evaluator(entry.data_file) == ROWS_CANNOT_MATCH: - existing_entries.append(_copy_with_new_status(entry, ManifestEntryStatus.EXISTING)) else: - # Based on the metadata, it is unsure to say if the file can be deleted - partial_rewrites_needed = True + # Based on the metadata, we cannot determine if it can be deleted + existing_entries.append(_copy_with_new_status(entry, ManifestEntryStatus.EXISTING)) Review Comment: Yes, these files are unaffected by the delete and should be kept in the manifest as an existing entry. I should have tested more extensively 😱 -- 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