-
-
Notifications
You must be signed in to change notification settings - Fork 6
⚡ Bolt: Add database indexes for improved query performance #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Bolt's Journal | ||
|
|
||
| ## 2025-05-14 - [Database Indexing for Performance] | ||
| **Learning:** Found that the core tables (`chats`, `messages`, `calendar_notes`) were missing indexes on frequently queried and sorted columns (foreign keys and `created_at`). In Drizzle ORM, foreign key constraints do not automatically create indexes in PostgreSQL. | ||
| **Action:** Always check for missing indexes on foreign keys and columns used in `WHERE` and `ORDER BY` clauses to ensure query performance scales with data growth. | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| CREATE TABLE "calendar_notes" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "chat_id" uuid, | ||
| "date" timestamp with time zone NOT NULL, | ||
| "content" text NOT NULL, | ||
| "location_tags" jsonb, | ||
| "user_tags" text[], | ||
| "map_feature_id" text, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | ||
| "updated_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE "chat_participants" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "chat_id" uuid NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "role" text DEFAULT 'collaborator' NOT NULL, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | ||
| CONSTRAINT "chat_participants_chat_user_unique" UNIQUE("chat_id","user_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE "locations" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "chat_id" uuid, | ||
| "geojson" jsonb NOT NULL, | ||
| "geometry" geometry(GEOMETRY, 4326), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check if PostGIS and pgvector extensions are created in any migration or setup file
rg -n -i 'CREATE EXTENSION' --type sql
rg -n -i 'CREATE EXTENSION' -g '*.ts' -g '*.js'Repository: QueueLab/QCX Length of output: 38 🏁 Script executed: # Check the migration file structure to verify the claims
fd 'migrations' -t d | head -20Repository: QueueLab/QCX Length of output: 75 🏁 Script executed: # Read the specific migration file mentioned in the review
find . -name "0001_sync_and_add_indexes.sql" -type fRepository: QueueLab/QCX Length of output: 106 🏁 Script executed: # Read the migration file to verify the content at lines 28, 47, 60
cat -n ./drizzle/migrations/0001_sync_and_add_indexes.sql | sed -n '1,70p'Repository: QueueLab/QCX Length of output: 4550 🏁 Script executed: # Check if 0000 migration exists
ls -la drizzle/migrations/ | head -20Repository: QueueLab/QCX Length of output: 377 🏁 Script executed: # Look for any other database setup or initialization files
fd -e sql -e ts -e js | xargs rg -l 'postgis|pgvector' 2>/dev/null | head -20Repository: QueueLab/QCX Length of output: 38 🏁 Script executed: # Read the 0000 migration file to check if extensions are created there
cat -n ./drizzle/migrations/0000_sweet_metal_master.sqlRepository: QueueLab/QCX Length of output: 1764 Add CREATE EXTENSION statements before migration 0001 runs. Migration 0001_sync_and_add_indexes.sql uses PostGIS types ( 🤖 Prompt for AI Agents |
||
| "name" text, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE "system_prompts" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "prompt" text NOT NULL, | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL, | ||
| "updated_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE TABLE "visualizations" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "user_id" uuid NOT NULL, | ||
| "chat_id" uuid, | ||
| "type" text DEFAULT 'map_layer' NOT NULL, | ||
| "data" jsonb NOT NULL, | ||
| "geometry" geometry(GEOMETRY, 4326), | ||
| "created_at" timestamp with time zone DEFAULT now() NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE "chats" ALTER COLUMN "title" SET DATA TYPE text;--> statement-breakpoint | ||
| ALTER TABLE "chats" ALTER COLUMN "title" SET DEFAULT 'Untitled Chat';--> statement-breakpoint | ||
| ALTER TABLE "chats" ALTER COLUMN "visibility" SET DATA TYPE text;--> statement-breakpoint | ||
| ALTER TABLE "chats" ALTER COLUMN "visibility" SET DEFAULT 'private';--> statement-breakpoint | ||
| ALTER TABLE "messages" ALTER COLUMN "role" SET DATA TYPE text;--> statement-breakpoint | ||
| ALTER TABLE "chats" ADD COLUMN "path" text;--> statement-breakpoint | ||
| ALTER TABLE "chats" ADD COLUMN "share_path" text;--> statement-breakpoint | ||
| ALTER TABLE "chats" ADD COLUMN "shareable_link_id" uuid DEFAULT gen_random_uuid();--> statement-breakpoint | ||
| ALTER TABLE "chats" ADD COLUMN "updated_at" timestamp with time zone DEFAULT now() NOT NULL;--> statement-breakpoint | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Line 59 (and the Ensure the application layer (e.g., Drizzle queries) explicitly sets CREATE OR REPLACE FUNCTION update_updated_at()
RETURNS TRIGGER AS $$
BEGIN NEW.updated_at = now(); RETURN NEW; END;
$$ LANGUAGE plpgsql;🤖 Prompt for AI Agents |
||
| ALTER TABLE "messages" ADD COLUMN "embedding" vector(1536);--> statement-breakpoint | ||
| ALTER TABLE "messages" ADD COLUMN "location_id" uuid;--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN "email" text;--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN "role" text DEFAULT 'viewer';--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN "selected_model" text;--> statement-breakpoint | ||
| ALTER TABLE "users" ADD COLUMN "system_prompt" text;--> statement-breakpoint | ||
| ALTER TABLE "calendar_notes" ADD CONSTRAINT "calendar_notes_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "calendar_notes" ADD CONSTRAINT "calendar_notes_chat_id_chats_id_fk" FOREIGN KEY ("chat_id") REFERENCES "public"."chats"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
|
Comment on lines
+1
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The deleted That’s a concrete security regression if the application relied on DB-enforced row isolation (common in Supabase/Postgres setups). With the current migration, access control becomes purely application-side (or wide open if clients have direct DB access). SuggestionRe-add the RLS enablement and policies for
If this removal was intentional, add explicit replacement policies or justify it with an alternative DB security model. Reply with "@CharlieHelps yes please" if you want me to add a commit that ports the prior RLS + policy statements into |
||
| ALTER TABLE "chat_participants" ADD CONSTRAINT "chat_participants_chat_id_chats_id_fk" FOREIGN KEY ("chat_id") REFERENCES "public"."chats"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "chat_participants" ADD CONSTRAINT "chat_participants_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "locations" ADD CONSTRAINT "locations_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "locations" ADD CONSTRAINT "locations_chat_id_chats_id_fk" FOREIGN KEY ("chat_id") REFERENCES "public"."chats"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "system_prompts" ADD CONSTRAINT "system_prompts_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "visualizations" ADD CONSTRAINT "visualizations_user_id_users_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."users"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "visualizations" ADD CONSTRAINT "visualizations_chat_id_chats_id_fk" FOREIGN KEY ("chat_id") REFERENCES "public"."chats"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| CREATE INDEX "calendar_notes_user_id_date_idx" ON "calendar_notes" USING btree ("user_id","date");--> statement-breakpoint | ||
| ALTER TABLE "messages" ADD CONSTRAINT "messages_location_id_locations_id_fk" FOREIGN KEY ("location_id") REFERENCES "public"."locations"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint | ||
| CREATE INDEX "chats_user_id_created_at_idx" ON "chats" USING btree ("user_id","created_at");--> statement-breakpoint | ||
| CREATE INDEX "messages_chat_id_created_at_idx" ON "messages" USING btree ("chat_id","created_at");--> statement-breakpoint | ||
|
Comment on lines
+75
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indexes on large tables can cause long locks and impact availability. If this runs in production on non-trivial data sizes, consider using SuggestionIf production uptime matters, consider building indexes concurrently:
If Drizzle’s migration runner always wraps in a transaction, document that these indexes should be applied during low-traffic periods, or create a separate manual/ops migration path. Reply with "@CharlieHelps yes please" if you’d like me to add a commit that adjusts the SQL to
Comment on lines
+75
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Non-concurrent index creation will lock tables — plan for downtime on populated tables.
🤖 Prompt for AI Agents |
||
| ALTER TABLE "chats" ADD CONSTRAINT "chats_shareable_link_id_unique" UNIQUE("shareable_link_id");--> statement-breakpoint | ||
| ALTER TABLE "users" ADD CONSTRAINT "users_email_unique" UNIQUE("email"); | ||
|
Comment on lines
+1
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The migration creates several tables (e.g., Given the PR description mentions “fixes the migration history consistency,” this is exactly the scenario where you’re likely to encounter drift across environments. Also, the prior deleted migration used SuggestionConsider making this migration idempotent/defensive where feasible:
If you'd like, reply with "@CharlieHelps yes please" and I can add a commit that adjusts the migration to be defensive (IF NOT EXISTS + duplicate_object guards) while keeping the intended final schema.
Comment on lines
+62
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also, if you intend case-insensitive uniqueness, a plain unique constraint on SuggestionDecide and encode the desired email uniqueness semantics:
Reply with "@CharlieHelps yes please" if you want me to add a commit implementing case-insensitive uniqueness via
Comment on lines
+1
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bundling schema sync with index additions in a single migration increases deployment risk. This migration combines new table creation, column additions, FK constraints, and index creation in one file. If any statement fails mid-way (e.g., a column already exists from a manual fix), the 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix markdown lint: add blank line after heading.
Static analysis (markdownlint MD022) flags that the heading on Line 3 needs a blank line below it before the content on Line 4.
Proposed fix
## 2025-05-14 - [Database Indexing for Performance] + **Learning:** Found that the core tables (`chats`, `messages`, `calendar_notes`) were missing indexes on frequently queried and sorted columns (foreign keys and `created_at`). In Drizzle ORM, foreign key constraints do not automatically create indexes in PostgreSQL.🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents