feat(db): promote documents.index_generation_id to a typed generated column#640
Conversation
…column Audit 2026-07-13 deferred item D2 (finding 1 stretch, staged after the A1 lexical RPC rewrite proved out live). - documents.index_generation_id is GENERATED ALWAYS AS (metadata->> 'index_generation_id')::uuid STORED, regex-guarded so malformed values generate NULL instead of failing writes. JSON stays the source of truth for every writer (commit RPC, batch repair, metadata patches) - the column can never drift, no dual-write, no trigger. - is_committed_document_generation gains a (uuid, uuid) overload (same truth table as the (uuid, jsonb) original, which remains). - The five effective reader functions now compare typed uuid columns instead of extracting JSONB per candidate row: corpus_topic_term_stats, match_document_chunks + match_document_chunks_hybrid (codified live bodies), match_document_chunks_text, match_document_lookup_chunks_text. Bodies are byte-exact copies with only that argument changed; _v2 wrappers inherit. uuid equality is normalized where the old text compare was byte-sensitive - unchanged in practice (lowercase UUIDs everywhere). - schema.sql, drift manifest (regenerated via Docker replay), documents Row/Insert/Update types, and schema tests updated in lockstep. Verified: full migration-chain replay from scratch in a supabase/postgres scratch container (ALL MIGRATIONS REPLAYED CLEAN) with spot checks for the stored generated column, malformed-value NULL, and the overload truth table; verify:cheap green (2,268 tests); verify:pr-local green (build + client-bundle scan). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Updates to Preview Branch (claude/documents-generation-column-2026-07-14) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
📝 WalkthroughWalkthroughThe database schema now exposes document index generations as typed UUIDs derived from metadata. Generation checks and retrieval functions use the typed values, with updated generated types, Drift metadata, schema replay definitions, permissions, and tests. ChangesTyped generation gating
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RetrievalFunction
participant DocumentChunk
participant Documents
participant CommitHelper
RetrievalFunction->>DocumentChunk: Read chunk generation UUID
RetrievalFunction->>Documents: Read document generation UUID
RetrievalFunction->>CommitHelper: Compare typed generations
CommitHelper-->>RetrievalFunction: Return committed-generation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@supabase/migrations/20260714110000_promote_documents_index_generation_id.sql`:
- Around line 33-41: Avoid adding index_generation_id as a stored generated
column in the documents migration, since it rewrites the table and requires an
ACCESS EXCLUSIVE lock. Replace it with a nullable column and implement the
required backfill and trigger synchronization path, preserving the existing UUID
validation behavior; otherwise explicitly move this operation to a documented
maintenance-window migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fffd1a47-6053-4dbe-964d-2d877de79685
📒 Files selected for processing (5)
src/lib/supabase/database.types.tssupabase/drift-manifest.jsonsupabase/migrations/20260714110000_promote_documents_index_generation_id.sqlsupabase/schema.sqltests/supabase-schema.test.ts
| alter table public.documents | ||
| add column if not exists index_generation_id uuid generated always as ( | ||
| case | ||
| when metadata->>'index_generation_id' | ||
| ~ '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' | ||
| then (metadata->>'index_generation_id')::uuid | ||
| else null | ||
| end | ||
| ) stored; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File list ==\n'
git ls-files supabase/migrations/20260714110000_promote_documents_index_generation_id.sql
printf '\n== File outline ==\n'
ast-grep outline supabase/migrations/20260714110000_promote_documents_index_generation_id.sql --view expanded || true
printf '\n== Relevant lines ==\n'
cat -n supabase/migrations/20260714110000_promote_documents_index_generation_id.sql | sed -n '1,140p'Repository: BigSimmo/Database
Length of output: 7190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Search for similar generated-column migrations ==\n'
rg -n "generated always as|stored;" supabase/migrations -S || true
printf '\n== Search for documents table definition ==\n'
rg -n "create table public.documents|table public.documents|metadata->>'index_generation_id'|index_generation_id" supabase -S || trueRepository: BigSimmo/Database
Length of output: 50373
🌐 Web query:
PostgreSQL ALTER TABLE ADD COLUMN GENERATED ALWAYS AS STORED table rewrite lock behavior documentation
💡 Result:
In PostgreSQL, adding a column defined as GENERATED ALWAYS AS (...) STORED requires a full table rewrite, which necessitates an ACCESS EXCLUSIVE lock on the table [1][2]. This lock persists for the entire duration of the rewrite process, meaning it blocks all read and write operations on the table for that time [1]. Official PostgreSQL documentation confirms that unlike adding a column with a standard, non-volatile DEFAULT value—which can often be performed quickly without a rewrite—adding a stored generated column triggers a complete rewrite of the table and its indexes [2]. There is currently no native, built-in mechanism to add such a column without this heavy locking behavior [1]. Technical discussions within the PostgreSQL community have acknowledged this limitation, with ongoing efforts and proposals to enable adding stored generated columns without long-lived locks (e.g., by validating an existing check constraint to skip the rewrite), but these are not yet standard features in the current released versions of PostgreSQL [3][4][5]. Consequently, for production tables with a large number of rows, this operation can cause significant downtime or unavailability [6][1]. Typical workarounds involve using virtual generated columns (which do not require a rewrite) or implementing application-level logic/triggers if a stored column is strictly required [1].
Citations:
- 1: https://github.com/mickelsamuel/migrationpilot/blob/main/docs/rules/MP062.md
- 2: https://www.postgresql.org/docs/18/sql-altertable.html
- 3: https://www.postgresql.org/message-id/abkrpUwlGngF4e-d%40phidippus.sen.work
- 4: https://www.postgresql.org/message-id/3af338791e8ab27c4a7efbc4217c97162de76bc8.camel%40cybertec.at
- 5: https://www.postgresql.org/message-id/attachment/196326/v4-0002-Try-to-avoid-a-rewrite-when-adding-a-stored-gener.patch
- 6: https://stackoverflow.com/questions/63791551/will-adding-new-generated-column-lock-table-in-postgresql-12
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Inspect migration content ==\n'
sed -n '1,220p' supabase/migrations/20260714110000_promote_documents_index_generation_id.sql
printf '\n== Inspect documents table definition location(s) ==\n'
rg -n "create table public\.documents|create table .*documents|index_generation_id|metadata" supabase/migrations supabase -SRepository: BigSimmo/Database
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print only the migration and any nearby references to documents schema evolution.
sed -n '1,220p' supabase/migrations/20260714110000_promote_documents_index_generation_id.sql
printf '\n--- nearby document migrations ---\n'
git ls-files supabase/migrations | rg 'documents|index_generation_id|promote_documents'Repository: BigSimmo/Database
Length of output: 12965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Prior migration introducing index_generation_id ==\n'
sed -n '1,220p' supabase/migrations/20260702180000_promote_index_generation_id_columns.sql
printf '\n== Documents table definition in schema snapshot ==\n'
sed -n '61,110p' supabase/schema.sql
printf '\n== Search for other documents ALTERs around index_generation_id ==\n'
rg -n "documents.*index_generation_id|index_generation_id.*documents|add column .*index_generation_id uuid generated always" supabase/migrations/2026070* supabase/schema.sql -SRepository: BigSimmo/Database
Length of output: 11341
Avoid the table rewrite here. ADD COLUMN ... STORED rewrites public.documents and holds an ACCESS EXCLUSIVE lock for the duration, which can block all document reads and writes on a large table. Use a nullable column plus backfill/trigger path, or schedule a maintenance window if downtime is acceptable.
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 34-41: Adding a generated column requires a table rewrite with an ACCESS EXCLUSIVE lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite. Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.
(adding-field-with-default)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/migrations/20260714110000_promote_documents_index_generation_id.sql`
around lines 33 - 41, Avoid adding index_generation_id as a stored generated
column in the documents migration, since it rewrites the table and requires an
ACCESS EXCLUSIVE lock. Replace it with a nullable column and implement the
required backfill and trigger synchronization path, preserving the existing UUID
validation behavior; otherwise explicitly move this operation to a documented
maintenance-window migration.
Source: Linters/SAST tools
1 participant
Summary
Deferred audit item D2 (2026-07-13 audit, finding 1 stretch — staged after the A1 lexical RPC rewrite proved out live at 57ms warm).
The
documentstable was the last place the committed index-generation pointer lived only in JSONB (metadata->>'index_generation_id'); every retrieval read extracted it per candidate row viais_committed_document_generation(uuid, jsonb). The artifact tables were already promoted in July (20260702180000).Design: generated column, not dual-write
documents.index_generation_id uuid GENERATED ALWAYS AS (…) STORED— a regex-guarded cast ofmetadata->>'index_generation_id'. JSON stays the source of truth for every writer (commit RPC, batch repair, arbitraryapply_document_metadata_patchcallers), so the typed column cannot drift — no dual-write bugs, no trigger, zero writer changes. TheADD COLUMNtable rewrite backfills all rows in the same statement (~2k rows, sub-second).is_committed_document_generationgains a(uuid, uuid)overload (same truth table; the(uuid, jsonb)original remains for other callers).corpus_topic_term_stats,match_document_chunks+match_document_chunks_hybrid(the codified-live pg_dump-style bodies, which win on reset — not the superseded lowercase duplicates, which are also updated for file consistency),match_document_chunks_text,match_document_lookup_chunks_text. Bodies were extracted byte-exact from schema.sql by script with per-function replacement-count assertions; only the one argument changed._v2wrappers inherit.Safety sweeps done
index_generation_id(no new ambiguity).document_strict_gate_status) expands an explicit CTE list — no duplicate-column collision on recreation.index_generation_idon documents (all writes go via the RPC arg into metadata), so the GENERATED column can't be write-conflicted.Verification
supabase/postgres:17.6.1.127scratch container:ALL MIGRATIONS REPLAYED CLEAN, plus spot checks — column present withattgenerated='s', valid metadata → typed uuid, malformed metadata → NULL without error, overload truth table(null,null)=t / (X,X)=t / (X,null)=filtered. (Localsupabase db resetis unavailable on this machine — port 54322 sits in a Windows excluded port range — hence the scratch-container equivalent; CI'sdb-reset-verifyre-validates.)verify:cheapgreen (2,268 tests),verify:pr-localgreen (production build + client-bundle scan).Deploy note
Merge auto-applies the migration to live via the Supabase GitHub integration. The table rewrite is sub-second at this corpus size; function redefinitions are transactional. After live apply, the standard post-migration checks (
check:drift, advisors) can be run on request.🤖 Generated with Claude Code
Summary by CodeRabbit