Fix two reload/update correctness issues noted in #95 - #115
Merged
Conversation
When update_from_file(inplace=False, restat=True) rebuilt the search table it recomputed the counts and stats into their _tmp tables, but the final swap covered only the search table -- so the recomputed statistics never reached the live names. The stale counts/stats stayed in place and the freshly built _tmp stats tables were orphaned (exactly the kind of leftover the reload preflight in roed314#95 later flags). Include the counts/stats tables in the swap when they were refreshed, the way reload already builds its swap list. Regression test covers both that the live counts reflect the new data afterward and that no _tmp stats tables are left behind (plus the no-restat path, which builds no _tmp stats at all). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The check rejecting user-chosen index/constraint names that end in a reserved suffix tested for _depN with the regex `_dep[\d]+_$`, whose trailing underscore no generated name ever has -- `_rename_if_exists` appends `_dep<N>` (like the `_old<N>` its sibling guard `_old[\d]+$` matches). So the _depN guard never fired, and a name colliding with the deprecation-renaming scheme slipped through. Drop the stray underscore. The existing restricted-suffix test is parametrized over every reserved suffix so the _depN case is now covered too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous commit added the counts/stats tables to the non-inplace update_from_file swap so the refreshed statistics reach the live names. But the _tmp counts table is built by _clone, which uses a bare CREATE TABLE ... LIKE and copies no indexes, so swapping it in replaced the indexed live counts table with an unindexed one -- cached-count lookups (column_counts and friends) would then fall back to sequential scans after update_from_file(inplace=False, restat=True). Rebuild the standard counts indexes on the _tmp table before the swap, exactly as reload already does (if self.stats.counts in tables: _create_counts_indexes(suffix=suffix)). The regression establishes the counts indexes the way a prior reload would, runs the non-inplace restat update, and asserts they are still present on the live counts table afterward; it fails without the rebuild. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
roed-math
pushed a commit
to roed-math/psycodict
that referenced
this pull request
Jul 22, 2026
Rebased onto main, which absorbed everything this PR used to anticipate; the reconciliation: - Version: 1.0.0 is now set in psycodict.__init__.__version__, the single source of truth roed314#121 established -- pyproject keeps main's dynamic version, SPDX license string and >=3.9 floor, and this PR's classifiers drop the License:: entry (setuptools >= 77 rejects it next to an SPDX string) and the 3.8 entry. - README: the rewrite now carries the Versioning.md pointer (roed314#123) in its Documentation list, which switches to relative links -- on the Sphinx site (roed314#122) those resolve to the copied guide pages, and they still render on GitHub; CHANGELOG and LICENSE stay absolute since neither is on the site. Supported Python is 3.9+; blob/master URLs become blob/main. - CHANGELOG: entries for the late wave -- sum/random honoring saving (roed314#118) and the 3.9 floor (roed314#121) under breaking changes; the docs site (roed314#122), Versioning.md/CONTRIBUTING/SECURITY and __version__ (roed314#121) under added; reload metafile+resort (roed314#114), update_from_file stats publication (roed314#115), copy_dumps delimiter escaping (roed314#116) under fixed; config.ini untracking (roed314#120), this release workflow and CITATION.cff (roed314#124) under infrastructure. - release.yml: the flow notes now say bump __init__.__version__ and merge to main. - Fixes the docs build on main, broken by the roed314#119 x roed314#122 crossing (roed314#119 merged after roed314#122's last CI run): the Configuration docstring's nested defaults list needed blank lines to be valid RST under autodoc, and the README's relative LICENSE link had no target on the site. Suite 857 passed / 36 skipped; ruff clean; sphinx -W clean; python -m build + twine check --strict pass with Version 1.0.0, License-Expression GPL-2.0-or-later, Requires-Python >=3.9 and no License classifier in the wheel metadata. Co-Authored-By: Claude Fable 5 <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.
#95's description flagged two pre-existing oddities and left them for separate work; both are still present on master and neither is touched by any open PR. Each is fixed here in its own commit, with a regression test that fails without the change.
1. Non-inplace
update_from_file(restat=True)never publishes the refreshed statsThe non-inplace path rebuilds the search table and, when
restatis set, recomputes the counts and statistics into their_tmptables — but the final swap wasself._swap_in_tmp([self.search_table]), covering only the search table. So the recomputed statistics stayed in<counts>_tmp/<stats>_tmp, the live counts/stats kept their stale values, and the_tmpstats tables were left orphaned (exactly the leftovers #95's own preflight would later trip over).reloadalready includes the counts/stats tables in its swap list; this makesupdate_from_filedo the same when it refreshed them.Regression test asserts the live counts reflect the new data after the update (
column_countsreads the stored counts, not a live recompute) and that no_tmpstats tables remain; a second test covers the no-restat path.2. Dead
_depNguard in_check_restricted_suffixThe guard rejecting user-chosen index/constraint names ending in a reserved suffix tested
_depNwithr"_dep[\d]+_$"— a trailing underscore that no generated name has._rename_if_existsappends_dep<N>(just like the_old<N>that its working siblingr"_old[\d]+$"matches), so the guard never fired and a name colliding with the deprecation-renaming scheme could slip through. Dropped the stray underscore.The existing restricted-suffix test is parametrized over every reserved suffix, so
_depNis now covered alongside_tmp/_pkey/_oldN.Suite 599 passed / 27 skipped locally (PG18, psycopg 3.3.4); ruff clean. Based directly on master (post-#96), so it merges independently of the rest of the wave.
🤖 Generated with Claude Code