count() returns a stale total for default-settings tables#61
Merged
Conversation
meta_tables.total was only ever written inside if self.stats.saving: blocks, and saving defaults to False — so a table created and filled through psycodict reported count() == 0 from any process, forever, and rewrite()/update_from_file() refused to run (their label validation compares count_distinct() against the stale count()). The write paths (insert_many, upsert, delete, copy_from) now maintain the total through _update_total, which applies the delta in the database itself (GREATEST(total + %s, 0) ... RETURNING) so concurrent writers compose instead of losing each other's rows; _set_total handles the recount cases — reload_final_swap recounts the swapped-in table just before it re-reads meta_tables, and refresh_stats(total=True) persists what it recomputes. Suffixed recounts (a staged _tmp table during reload) touch neither the in-memory total nor meta_tables, and the same gate is added to _record_count's own meta update, which had the same flaw. saving now gates only what its docstring says: recording statistics in the counts table. The precedent for writing meta_tables outside the saving guard is _break_stats, which already does exactly that on the same code paths. For LMFDB (saving = True) observable behaviour is unchanged. New regression tests cover the fresh-connection repro, the staged-suffix isolation, and the two-writer race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
meta_tables.totalwas only ever written insideif self.stats.saving:blocks, andsavingdefaults to False — so a table created and filled through psycodict reportedcount() == 0from any process, forever, andrewrite()/update_from_file()refused to run (their label validation comparescount_distinct()against the stalecount()).The write paths (
insert_many,upsert,delete,copy_from) now maintain the total through_update_total, which applies the delta in the database itself (GREATEST(total + %s, 0) ... RETURNING) so concurrent writers compose instead of losing each other's rows;_set_totalhandles the recount cases —reload_final_swaprecounts the swapped-in table just before it re-reads meta_tables, andrefresh_stats(total=True)persists what it recomputes. Suffixed recounts (a staged_tmptable during reload) touch neither the in-memory total nor meta_tables, and the same gate is added to_record_count's own meta update, which had the same flaw.savingnow gates only what its docstring says: recording statistics in the counts table. The precedent for writing meta_tables outside thesavingguard is_break_stats, which already does exactly that on the same code paths.For LMFDB (
saving = True) observable behaviour is unchanged. New regression tests cover the fresh-connection repro, the staged-suffix isolation, and the two-writer race.Split out of #60 (one PR per issue). This branch carries only this issue's fix and its tests; the rest of the known-bug inventory stays
xfailed, so the full suite is green here and the strict markers guarantee no cross-issue leakage.