-
Notifications
You must be signed in to change notification settings - Fork 413
Clear updates/requirements after commit #1961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
915b85f
b4da27d
4d24d0b
f39a2f7
54d980d
17fe4c2
ec74185
207a753
8549e0f
a01708c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -292,8 +292,6 @@ def _apply(self, updates: Tuple[TableUpdate, ...], requirements: Tuple[TableRequ | |||||||
|
|
||||||||
| if self._autocommit: | ||||||||
| self.commit_transaction() | ||||||||
| self._updates = () | ||||||||
| self._requirements = () | ||||||||
|
|
||||||||
| return self | ||||||||
|
|
||||||||
|
|
@@ -937,13 +935,15 @@ def commit_transaction(self) -> Table: | |||||||
| updates=self._updates, | ||||||||
| requirements=self._requirements, | ||||||||
| ) | ||||||||
| return self._table | ||||||||
| else: | ||||||||
| return self._table | ||||||||
|
|
||||||||
| self._updates = () | ||||||||
| self._requirements = () | ||||||||
|
|
||||||||
| return self._table | ||||||||
|
|
||||||||
|
|
||||||||
| class CreateTableTransaction(Transaction): | ||||||||
| """A transaction that involves the creation of a a new table.""" | ||||||||
| """A transaction that involves the creation of a new table.""" | ||||||||
|
|
||||||||
| def _initial_changes(self, table_metadata: TableMetadata) -> None: | ||||||||
| """Set the initial changes that can reconstruct the initial table metadata when creating the CreateTableTransaction.""" | ||||||||
|
|
@@ -988,11 +988,15 @@ def commit_transaction(self) -> Table: | |||||||
| Returns: | ||||||||
| The table with the updates applied. | ||||||||
| """ | ||||||||
| self._requirements = (AssertCreate(),) | ||||||||
| self._table._do_commit( # pylint: disable=W0212 | ||||||||
| updates=self._updates, | ||||||||
| requirements=self._requirements, | ||||||||
| ) | ||||||||
| if len(self._updates) > 0: | ||||||||
| self._table._do_commit( # pylint: disable=W0212 | ||||||||
| updates=self._updates, | ||||||||
| requirements=(AssertCreate(),), | ||||||||
| ) | ||||||||
|
|
||||||||
| self._updates = () | ||||||||
| self._requirements = () | ||||||||
|
Comment on lines
+991
to
+998
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we use I want to make sure there's a way for future subclass of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/apache/iceberg-python/pull/1961/files#r2252535367 is a reply to the comment above :p
Comment on lines
+997
to
+998
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could refactor it like:
Suggested change
But it doesn't actually commit, but just clear the
Comment on lines
+991
to
+998
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah we cant use Since requirements are checked before updates, this wont work. 👍 im fine with leaving it as is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. The |
||||||||
|
|
||||||||
| return self._table | ||||||||
|
|
||||||||
|
|
||||||||
|
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.