qzyu999 commented on code in PR #3131:
URL: https://github.com/apache/iceberg-python/pull/3131#discussion_r3090633495
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -724,6 +814,14 @@ def delete(self) -> _DeleteFiles:
snapshot_properties=self._snapshot_properties,
)
+ def replace(self) -> _RewriteFiles:
+ return _RewriteFiles(
Review Comment:
Hi @geruh, you bring up a good point, and it's something I noticed seemed
off along the way. The reason why we have this discrepancy is because we're
mirroring what's found in the Java code itself.
- Looking here
(https://iceberg.apache.org/javadoc/latest/org/apache/iceberg/RewriteFiles.html),
you can see that the "API for replacing files in a table." is called
`RewriteFiles`.
- Looking here
(https://iceberg.apache.org/javadoc/1.4.2/org/apache/iceberg/DataOperations.html#REPLACE),
you can see that the `REPLACE` operation is implemented by `RewriteFiles`.
- On a slightly unrelated note here
(https://iceberg.apache.org/javadoc/1.4.2/org/apache/iceberg/DataOperations.html#OVERWRITE),
the `OVERWRITE` operation itself can be implemented by `ReplacePartitions`.
I named the Python API `replace()` to accurately reflect the
`Operation.REPLACE` snapshot string it generates, while keeping the internal
class named `_RewriteFiles` to match the Java builder logic.
That said, if you feel strongly about matching the Java API's user-facing
method (`rewrite()`) rather than the snapshot operation (`replace()`), I'm
happy to rename the public method to `rewrite()` for consistency. Let me know
what you prefer!
--
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]
