Skip to content

fix: VACUUM correctly removes dead index entries#267

Merged
tjgreen42 merged 8 commits intomainfrom
fix/vacuum-dead-tuple-rebuild
Mar 7, 2026
Merged

fix: VACUUM correctly removes dead index entries#267
tjgreen42 merged 8 commits intomainfrom
fix/vacuum-dead-tuple-rebuild

Conversation

@tjgreen42
Copy link
Copy Markdown
Collaborator

@tjgreen42 tjgreen42 commented Mar 5, 2026

Summary

  • Fixes correctness bug where plain VACUUM did not remove dead index
    entries, leading to false positives when Postgres reuses CTIDs
  • Implements four-phase approach in tp_bulkdelete:
    1. Spill memtable to segments
    2. Identify segments containing dead CTIDs (O(segments) memory)
    3. Rebuild affected segments from heap via TpBuildContext
    4. Update metapage statistics
  • Adds comment documenting disjoint_sources=false invariant in
    merge path

Limitations

Testing

  • New vacuum_rebuild regression test covering:
    • CTID reuse after DELETE + VACUUM + INSERT
    • All-docs-deleted segment removal
    • Multi-level segments with deletions
    • No-op VACUUM path
  • All 50 regression tests pass

@tjgreen42 tjgreen42 force-pushed the fix/vacuum-dead-tuple-rebuild branch from 56e05e8 to a4b7e1c Compare March 5, 2026 17:17
@tjgreen42 tjgreen42 marked this pull request as ready for review March 5, 2026 17:17
tjgreen42 and others added 6 commits March 5, 2026 13:17
Reproduces the correctness issue where plain VACUUM does not remove
dead index entries, leading to false positives when CTIDs are reused
by new inserts.
Replaces the old no-op bulkdelete (which discarded callback results)
with a correctness fix that:
1. Spills memtable to segments
2. Identifies segments containing dead CTIDs (O(segments) memory)
3. Rebuilds affected segments from heap via TpBuildContext
4. Updates metapage statistics

This prevents false positives when Postgres reuses CTIDs after VACUUM
frees heap line pointers.
All four test scenarios pass:
- CTID reuse: alpha_matches=0 (fix confirmed)
- All-deleted segment removal works
- Multi-level vacuum preserves remaining docs
- No-op vacuum path works

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The original test used WHERE <@> < 0 which triggers a seqscan that
re-evaluates the operator on the heap tuple, masking the bug. The
fix uses ORDER BY <@> LIMIT which triggers an index scan where
xs_recheck=false causes Postgres to trust the stale index entries.

Verified: test produces alpha_false_positives=1 without the fix,
and alpha_false_positives=0 with the fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DatumGetTextPP may detoast, allocating memory. By calling it inside
per_doc_ctx, the detoasted datum is freed on each MemoryContextReset
instead of accumulating in the outer context.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the complex alpha_false_positives subquery pattern with
a direct SELECT that expects 0 rows. The index scan returns nothing
for deleted terms when the fix is working correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tjgreen42 tjgreen42 force-pushed the fix/vacuum-dead-tuple-rebuild branch from e3c7148 to 481b7d6 Compare March 5, 2026 21:17
@tjgreen42 tjgreen42 merged commit c3d6522 into main Mar 7, 2026
15 checks passed
@tjgreen42 tjgreen42 deleted the fix/vacuum-dead-tuple-rebuild branch March 7, 2026 01:11
@tjgreen42 tjgreen42 mentioned this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant