Repository navigation
Conversation
added 2 commits
October 7, 2026 19:32
UpdateStatistics.remove_statistics assigned self._updates instead of appending to it, so any set_statistics call staged earlier in the same chain was silently dropped. For example, table.update_statistics().set_statistics(f).remove_statistics(id).commit() only removed the statistics and never registered f. This is the usage shown in the class docstring and in the API docs. Append with += like set_statistics does, and add a regression test that chains set_statistics and remove_statistics and checks that both updates are staged and applied. Follow-up to apache#3557.
…test Make the precondition explicit: the fixture already has statistics for both snapshots, which is why remove_statistics targets the previous one. Assert on the resulting table metadata only, instead of the private staged updates.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The
remove_statistics()method on the statistics update builder is dropping updates which are queued before it.This PR supersedes #3557
When
remove_statistics()is called, the update queued byset_statistics()gets dropped, so at commit, only the remove is preserved. The side effects of this could be that the query engines plan with stale statistics.This happens as
=is used inremove_statistics()method.=replaces the builder's list of pending updates instead of appending to it.set_statistics()is using+=.The fix is to use
+=inremove_statistics()method, like inset_statistics().Are these changes tested?
Yes,
test_update_statistics_set_then_remove_keeps_both_updates(tests/table/test_init.py) has been added as a new unit test.Consider a table that has snapshots s2 and s1, each with its statistics files.
The test sets a new statistics file for s2, then removes s1's statistics.
After the commit, s2 points to the new file and s1 has no statistics. Both changes are applied.
On the main branch, this test fails, the
set_statistics()for s2 gets dropped, s2 keeps pointing to its old file.With the fix, make test, 4,254 passed, and make lint passing.
Are there any user-facing changes?
No API changes. set-then-remove now applies both updates.
AI disclosure
AI was used to write the code, tests and for local verification.