syun64 commented on code in PR #569:
URL: https://github.com/apache/iceberg-python/pull/569#discussion_r1598823358


##########
pyiceberg/table/__init__.py:
##########
@@ -2931,14 +3161,52 @@ def _deleted_entries(self) -> List[ManifestEntry]:
         return []
 
 
-class OverwriteFiles(_MergingSnapshotProducer):
+class OverwriteFiles(_MergingSnapshotProducer["OverwriteFiles"]):

Review Comment:
   Is there a reason why we are keeping this class over using DeleteFiles and 
FastAppend?
   
   It looks like we have two different meaning / implementation of `overwrite` 
in Table/Transaction as opposed to `UpdateSnapshot`, where the former is 
producing a snapshot with the operation `delete` + `append` (and maybe another 
`append`) and the latter is producing a snapshot with the operation 
`overwrite`. I'm wondering if this would be confusing for the users



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

Reply via email to