You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "geruh (via GitHub)" <gi...@apache.org> on 2026/01/03 00:22:41 UTC
[PR] feat: Add support for rolling back to snapshot [iceberg-python]
geruh opened a new pull request, #2878:
URL: https://github.com/apache/iceberg-python/pull/2878
# Rationale for this change
This PR adds the ability to rollback a table to a ancestoral snapshot. Some of this work was also done in #758, and is a progress pr to be merged after #2871.
Additionally, adding some more tests.
## Are these changes tested?
Yes
## Are there any user-facing changes?
New API
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2659072004
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
+ raise ValueError("Either snapshot_id or ref_name must be provided, not both")
+
+ target_snapshot_id: int
+ if snapshot_id is not None:
+ target_snapshot_id = snapshot_id
+ else:
+ if ref_name not in self._transaction.table_metadata.refs:
+ raise ValueError(f"Cannot find matching snapshot ID for ref: {ref_name}")
+ target_snapshot_id = self._transaction.table_metadata.refs[ref_name].snapshot_id
+
+ if self._transaction.table_metadata.snapshot_by_id(target_snapshot_id) is None:
+ raise ValueError(f"Cannot set current snapshot to unknown snapshot id: {target_snapshot_id}")
+
+ update, requirement = self._transaction._set_ref_snapshot(
+ snapshot_id=target_snapshot_id,
+ ref_name=MAIN_BRANCH,
+ type="branch",
Review Comment:
looks like there are a few places where this should be using the enum instead... I can throw together a PR
Edit: [PR](https://github.com/apache/iceberg-python/pull/2880)!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2659072004
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
+ raise ValueError("Either snapshot_id or ref_name must be provided, not both")
+
+ target_snapshot_id: int
+ if snapshot_id is not None:
+ target_snapshot_id = snapshot_id
+ else:
+ if ref_name not in self._transaction.table_metadata.refs:
+ raise ValueError(f"Cannot find matching snapshot ID for ref: {ref_name}")
+ target_snapshot_id = self._transaction.table_metadata.refs[ref_name].snapshot_id
+
+ if self._transaction.table_metadata.snapshot_by_id(target_snapshot_id) is None:
+ raise ValueError(f"Cannot set current snapshot to unknown snapshot id: {target_snapshot_id}")
+
+ update, requirement = self._transaction._set_ref_snapshot(
+ snapshot_id=target_snapshot_id,
+ ref_name=MAIN_BRANCH,
+ type="branch",
Review Comment:
looks like there are a few places where this should be using the enum instead... I can throw together a PR
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2659070821
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
+ raise ValueError("Either snapshot_id or ref_name must be provided, not both")
+
+ target_snapshot_id: int
+ if snapshot_id is not None:
+ target_snapshot_id = snapshot_id
+ else:
+ if ref_name not in self._transaction.table_metadata.refs:
+ raise ValueError(f"Cannot find matching snapshot ID for ref: {ref_name}")
+ target_snapshot_id = self._transaction.table_metadata.refs[ref_name].snapshot_id
+
+ if self._transaction.table_metadata.snapshot_by_id(target_snapshot_id) is None:
+ raise ValueError(f"Cannot set current snapshot to unknown snapshot id: {target_snapshot_id}")
+
+ update, requirement = self._transaction._set_ref_snapshot(
+ snapshot_id=target_snapshot_id,
+ ref_name=MAIN_BRANCH,
+ type="branch",
Review Comment:
could use the enum instead of the string https://github.com/apache/iceberg-python/blob/main/pyiceberg/table/refs.py#L29
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
Review Comment:
this is a silly nit but why do this? This looks almost java-esque and could be
```suggestion
if snapshot_id is None and ref_name is None:
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2662916310
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
Review Comment:
you could probably do some fanciness with any and all but not gonna harp on it
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "geruh (via GitHub)" <gi...@apache.org>.
geruh commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2659497893
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
Review Comment:
Yeah lol I just cleaned up the original in a pythonish way to say one must be set. However, seems like this is confusing
Original: https://github.com/apache/iceberg-python/pull/758/changes#diff-23e8153e0fd497a9212215bd2067068f3b56fa071770c7ef326db3d3d03cee9bR2092
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#issuecomment-3752607392
Thanks for the PR @geruh and thanks for the review @jayceslesar
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat: Add support for rolling back to snapshot [iceberg-python]
Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on code in PR #2878:
URL: https://github.com/apache/iceberg-python/pull/2878#discussion_r2662887615
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -941,6 +949,76 @@ def remove_branch(self, branch_name: str) -> ManageSnapshots:
"""
return self._remove_ref_snapshot(ref_name=branch_name)
+ def set_current_snapshot(self, snapshot_id: int | None = None, ref_name: str | None = None) -> ManageSnapshots:
+ """Set the current snapshot to a specific snapshot ID or ref.
+
+ Args:
+ snapshot_id: The ID of the snapshot to set as current.
+ ref_name: The snapshot reference (branch or tag) to set as current.
+
+ Returns:
+ This for method chaining.
+
+ Raises:
+ ValueError: If neither or both arguments are provided, or if the snapshot/ref does not exist.
+ """
+ self._commit_if_ref_updates_exist()
+
+ if (snapshot_id is None) == (ref_name is None):
Review Comment:
ah I see hahahaha
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org