Conversation
Post-1.0.0 release bump. Renames the dev SQL files and upgrade script from 1.0.0-dev to 1.1.0-dev, updates version strings in the control file, mod.c, CLAUDE.md, Makefile, test scripts, and expected outputs. Also drops the timescaledb-docker-ha step from RELEASING.md; that part of the release procedure lives in internal docs.
There was a problem hiding this comment.
LGTM — purely mechanical version bump from 1.0.0-dev to 1.1.0-dev.
Extended reasoning...
Overview
This PR touches 72 files, but every change is a mechanical substitution of the version string 1.0.0-dev → 1.1.0-dev. The affected files fall into four categories: (1) core metadata files (pg_textsearch.control, src/mod.c, CLAUDE.md, Makefile), (2) SQL install/upgrade scripts (renamed and version strings updated), (3) 60+ regression test expected-output files (only the INFO: pg_textsearch v... line changes), and (4) three shell test scripts (only the CREATE EXTENSION ... VERSION '...' literal changes). Additionally, RELEASING.md removes the timescaledb-docker-ha step (moved to internal docs) and updates the version-chain diagram.
Security risks
None. There are no logic changes, no new code paths, no authentication or permission changes, and no new dependencies.
Level of scrutiny
Minimal. Version bumps following a release are the most routine maintenance task in this codebase. The pattern is explicitly documented in RELEASING.md, and every modified line is either a version-string literal or a filename. There is no ambiguity about intent or correctness.
Other factors
The one bug reported (anti-pattern in segment_wal_recovery.sh) is explicitly pre-existing and orthogonal to this PR — the PR only changes line 147 of that file, while the anti-pattern is at lines 258–292. Test output regeneration is consistent with the version string change. No outstanding reviewer comments exist.
|
|
||
| "${PGBINDIR}/createdb" -h "${DATA_DIR}" -p "${TEST_PORT}" \ | ||
| "${TEST_DB}" | ||
| run_sql_quiet "CREATE EXTENSION pg_textsearch VERSION '1.0.0-dev';" | ||
| run_sql_quiet "CREATE EXTENSION pg_textsearch VERSION '1.1.0-dev';" |
There was a problem hiding this comment.
🟣 The crash recovery test in segment_wal_recovery.sh uses the CLAUDE.md-prohibited anti-pattern WHERE content <@> 'batch1'::bm25query < 0 at lines 258, 269, and 292 as a convenience row-count filter. This is a pre-existing issue unrelated to this PR's version-string change at line 147.
Extended reasoning...
What the bug is and how it manifests
The three verification queries in the crash-recovery test (lines 258, 269, 292) use the pattern WHERE content <@> query < 0 to count rows after a crash and recovery. CLAUDE.md §'Anti-pattern: WHERE clauses comparing BM25 scores' (around line 154) explicitly prohibits exactly this usage, stating: 'Do not use WHERE clauses that compare against BM25 scores as a generic way to filter for matching documents.'
The specific code path that triggers it
- Line 258:
SELECT count(*) FROM recovery_test WHERE content <@> 'batch1'::bm25query < 0; - Line 269: Same query used for post-crash verification
- Line 292:
SELECT count(*) FROM recovery_test WHERE content <@> 'batch2'::bm25query < 0;
Why existing code doesn't prevent it
CLAUDE.md documents the prohibition but provides no automated enforcement. The exception in CLAUDE.md ('standalone scoring as an explicit test target') does not apply here because these are convenience row-count filters, not deliberate comparisons of standalone vs. index scoring behavior.
What the impact would be
A WHERE content <@> query < 0 clause triggers standalone scoring (sequential scan), not a BM25 index scan. Index scans require ORDER BY. Since standalone scoring reads directly from the heap and recomputes BM25 scores without touching the index, these verification queries would succeed even if the BM25 index segments were entirely corrupt after crash recovery — as long as the heap pages survived. This defeats the purpose of the crash-recovery test.
How to fix it
Replace each anti-pattern query with the CLAUDE.md-recommended subquery form:
SELECT count(*) FROM (
SELECT 1 FROM recovery_test
ORDER BY content <@> 'batch1'::bm25query
) sub;This forces an index scan, ensuring the test actually exercises BM25 index recovery.
Step-by-step proof
- The test inserts ~3000 'batch1' rows and triggers an auto-spill to create BM25 segments on disk.
- The server is SIGKILLed to simulate a crash.
- PostgreSQL recovers via WAL replay.
- The query
SELECT count(*) ... WHERE content <@> 'batch1'::bm25query < 0runs standalone scoring: for each heap row, it re-parses the document text, looks up IDF values from the index catalog, and computes a BM25 score — without ever traversing the on-disk segment pages. - If the segment pages were corrupted or missing (WAL not generated), this query still returns ~3000 because the heap survived; only an
ORDER BY <@>query would have revealed the broken index. - The correct pattern
SELECT count(*) FROM (SELECT 1 FROM recovery_test ORDER BY content <@> 'batch1'::bm25query) subuses an index scan and would fail or return wrong results if segments are corrupt.
Pre-existing nature
This bug exists in the file before PR #332. The PR's only change to this file is updating the version string from 1.0.0-dev to 1.1.0-dev at line 147, which is completely orthogonal to the anti-pattern usage at lines 258–292. The violation is worth noting for reviewers and future authors, but does not block this version-bump PR.
Summary
Post-1.0.0 release bump — we forgot to do this after the 1.0.0 release.
pg_textsearch--1.0.0-dev.sql→pg_textsearch--1.1.0-dev.sqlpg_textsearch--1.0.0--1.0.0-dev.sql→pg_textsearch--1.0.0--1.1.0-dev.sqlpg_textsearch.control,src/mod.c,CLAUDE.md,Makefile, test scripts, and expected outputstimescaledb-docker-hastep fromRELEASING.md; that part of the release procedure now lives in internal docsTesting
make testpassesmake format-checkclean