Fokko commented on code in PR #569: URL: https://github.com/apache/iceberg-python/pull/569#discussion_r1615912422
########## pyiceberg/table/__init__.py: ########## @@ -443,6 +471,74 @@ def overwrite( for data_file in data_files: update_snapshot.append_data_file(data_file) + def delete(self, delete_filter: Union[str, BooleanExpression], snapshot_properties: Dict[str, str] = EMPTY_DICT) -> None: + if ( + self.table_metadata.properties.get(TableProperties.DELETE_MODE, TableProperties.DELETE_MODE_COPY_ON_WRITE) + == TableProperties.DELETE_MODE_MERGE_ON_READ + ): + warnings.warn("Merge on read is not yet supported, falling back to copy-on-write") + + if isinstance(delete_filter, str): + delete_filter = _parse_row_filter(delete_filter) + + with self.update_snapshot(snapshot_properties=snapshot_properties).delete() as delete_snapshot: + delete_snapshot.delete_by_predicate(delete_filter) + + # Check if there are any files that require an actual rewrite of a data file + if delete_snapshot.rewrites_needed is True: + bound_delete_filter = bind(self._table.schema(), delete_filter, case_sensitive=True) + preserve_row_filter = expression_to_pyarrow(Not(bound_delete_filter)) + commit_uuid = uuid.uuid4() Review Comment: Good call. I think it is best to keep them per snapshot. Since it is more granular, it is easier to trace back to which snapshot it belongs. Otherwise, we also need to pass in the `counter` everywhere to ensure that we generate unique file paths. -- 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