Skip to content

fix(server): repair connections.entity_ids schema drift (query_sql)#1157

Merged
buremba merged 1 commit into
mainfrom
feat/fix-query-sql-entity-ids
May 29, 2026
Merged

fix(server): repair connections.entity_ids schema drift (query_sql)#1157
buremba merged 1 commit into
mainfrom
feat/fix-query-sql-entity-ids

Conversation

@buremba
Copy link
Copy Markdown
Member

@buremba buremba commented May 29, 2026

Problem

The admin query_sql tool crashes on the connections table in prod: SQL_ERROR: column "entity_ids" does not exist, even when entity_ids isn't selected — it builds the projection from the SAFE_COLUMN_DEFS allowlist, which (correctly) lists entity_ids.

Root cause

connections.entity_ids bigint[] and its GIN index are defined in the squashed baseline (db/migrations/00000000000000_baseline.sql:716,3522). The prod serving DB (owletto) was provisioned from an older pre-squash baseline plus a migration series no longer present in the repo, so connections.entity_ids (added only in the squash) was never applied — while sibling tables (events/watchers/feeds) did get it. Classic squashed-baseline drift; the allowlist is correct, the prod table is behind.

Fix

Idempotent delta migration adding the column + GIN index IF NOT EXISTS: a no-op on fresh installs (baseline already has it) and a repair on drifted DBs. No change to table-schema.ts.

Validation — red→green on real Postgres matching prod's drifted shape

  • Created a scratch DB in the prod PG engine, recreated the drifted connections shape (no entity_ids), ran the exact allowlist projection query_sql builds → reproduced column "entity_ids" does not exist (RED).
  • Applied the migration up-section → SELECT slug, status FROM connections ORDER BY updated_at DESC returned rows (GREEN). Idempotency confirmed (re-run = skip). Scratch DB dropped.
  • tsc --noEmit (packages/server): clean.

Follow-ups (not in this PR)

  • The drift test (packages/server/src/utils/__tests__/table-schema.test.ts) only checks DB→allowlist, not allowlist→DB, and runs against the current baseline — so it could not have caught this. Add the reverse-direction assertion.
  • connections.entity_ids is provisioned but not yet wired into the product (connection↔entity association). Tracked as a separate feature.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Chores
    • Applied a database maintenance update to ensure schema consistency and improve data retrieval performance.

Review Change Stack

connections.entity_ids + its GIN index exist in the squashed baseline but
drifted DBs provisioned from the old pre-squash baseline (e.g. prod owletto)
never got them, so query_sql's allowlist projection fails with
'column "entity_ids" does not exist'. Add an idempotent delta migration
(no-op on fresh installs, repair on drifted DBs).
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

Adds an idempotent database migration that repairs schema drift by creating the entity_ids bigint array column on the connections table and a GIN index on it, with conditional checks to ensure safety for both fresh installs and already-drifted databases.

Changes

Schema Repair – Connections Entity IDs

Layer / File(s) Summary
Add entity_ids column and GIN index to connections
db/migrations/20260529120000_connections_entity_ids.sql
Migration conditionally creates connections.entity_ids as a bigint array column and idx_connections_entity_ids GIN index on up if missing, and conditionally drops both on down if present.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A schema grows with graceful ease,
Entity IDs dance through the trees,
GIN indices help us find our way,
Schema drift repaired this day! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is comprehensive, covering the problem, root cause, fix, validation, and follow-ups, but lacks explicit test plan checkboxes as specified in the repository template. Add a test plan section with checked boxes for validation steps performed (e.g., migration tested on drifted DB, tsc passed) to match the repository template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: an idempotent migration to fix schema drift by adding the missing connections.entity_ids column.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/fix-query-sql-entity-ids

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@db/migrations/20260529120000_connections_entity_ids.sql`:
- Line 16: Replace the blocking index creation with a concurrent one: change the
statement that creates idx_connections_entity_ids on public.connections to use
CREATE INDEX CONCURRENTLY IF NOT EXISTS ... USING gin (entity_ids) so the GIN
build won’t lock the table; also ensure this migration is executed outside a
transactional migration runner (CONCURRENTLY cannot run inside a transaction
block) or mark the migration as non-transactional in your migration tool.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 16297562-bb86-46e2-ad90-e4b03ffbd9de

📥 Commits

Reviewing files that changed from the base of the PR and between 1d7d69f and 52ffde4.

📒 Files selected for processing (1)
  • db/migrations/20260529120000_connections_entity_ids.sql

-- definition exactly so the allowlist stays consistent with the real schema.
ALTER TABLE public.connections ADD COLUMN IF NOT EXISTS entity_ids bigint[];

CREATE INDEX IF NOT EXISTS idx_connections_entity_ids ON public.connections USING gin (entity_ids);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check PostgreSQL version to confirm CONCURRENTLY IF NOT EXISTS support (requires 9.5+)

rg -n "FROM postgres:" --type dockerfile

Repository: lobu-ai/lobu

Length of output: 91


🌐 Web query:

PostgreSQL CREATE INDEX CONCURRENTLY IF NOT EXISTS minimum version

💡 Result:

Minimum PostgreSQL version: PostgreSQL 9.5. Reasoning (from sources): - PostgreSQL 9.5 release notes state: “Add IF NOT EXISTS clause to … CREATE INDEX …” [1]. - The CREATE INDEX grammar documents both optional CONCURRENTLY and IF NOT EXISTS clauses: “CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] name ] …” [2][3]. - A CREATE INDEX IF NOT EXISTS community reference also notes “Postgres 9.5 or newer” and that it “Also works for CREATE INDEX CONCURRENTLY IF NOT EXISTS.” [4]. So the combined form “CREATE INDEX CONCURRENTLY IF NOT EXISTS …” is available starting in PostgreSQL 9.5. [1][2][4]

Citations:


Add CONCURRENTLY to avoid production locks during the GIN index build.

The migration’s CREATE INDEX IF NOT EXISTS ... USING gin (entity_ids); will take a table lock while the index is built on a non-empty public.connections table. CREATE INDEX CONCURRENTLY IF NOT EXISTS is supported by PostgreSQL 9.5+.

🔧 Proposed fix
-CREATE INDEX IF NOT EXISTS idx_connections_entity_ids ON public.connections USING gin (entity_ids);
+CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_connections_entity_ids ON public.connections USING gin (entity_ids);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CREATE INDEX IF NOT EXISTS idx_connections_entity_ids ON public.connections USING gin (entity_ids);
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_connections_entity_ids ON public.connections USING gin (entity_ids);
🤖 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 `@db/migrations/20260529120000_connections_entity_ids.sql` at line 16, Replace
the blocking index creation with a concurrent one: change the statement that
creates idx_connections_entity_ids on public.connections to use CREATE INDEX
CONCURRENTLY IF NOT EXISTS ... USING gin (entity_ids) so the GIN build won’t
lock the table; also ensure this migration is executed outside a transactional
migration runner (CONCURRENTLY cannot run inside a transaction block) or mark
the migration as non-transactional in your migration tool.

@buremba buremba enabled auto-merge (squash) May 29, 2026 16:14
@buremba buremba merged commit 4081c0a into main May 29, 2026
24 checks passed
@buremba buremba deleted the feat/fix-query-sql-entity-ids branch May 29, 2026 16:19
buremba added a commit that referenced this pull request May 29, 2026
…1158)

* feat(server): associate connections with entities (union with feeds)

Add entity_ids to manage_connections create/connect/update (cross-org
validated via assertEntityIdsInOrg, tri-state clear on update); list
entity_names + entity_id filter become a UNION of the connection's own
entity_ids and its feeds' entity_ids; parse entity_ids to number[] so the
API returns a typed array. Bumps owletto pointer for the matching UI.

Depends on the connections.entity_ids drift migration (#1157) for prod DBs.

* chore: bump owletto pointer to merged #249 (09e361d)
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.

2 participants