fix: fixed CSM-016
This commit is contained in:
@@ -3,7 +3,8 @@
|
||||
"allow": [
|
||||
"Bash(mix compile *)",
|
||||
"Bash(mix test *)",
|
||||
"Bash(mix dialyzer *)"
|
||||
"Bash(mix dialyzer *)",
|
||||
"Bash(mix ecto.migrate)"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
22
CODESMELL.md
22
CODESMELL.md
@@ -259,20 +259,14 @@
|
||||
|
||||
---
|
||||
|
||||
### CSM-015 — Missing DB Indexes on Foreign Keys
|
||||
- **Files:** `priv/repo/migrations/20260423120000_create_persistence_contract.exs`
|
||||
- **What:** The initial migration uses `references(...)` which creates FK constraints but does NOT create dedicated indexes for most FKs. Missing indexes on:
|
||||
- `media.project_id` (queried in every sidebar/dashboard load)
|
||||
- `post_media.post_id`, `post_media.media_id`
|
||||
- `chat_messages.conversation_id`
|
||||
- `embedding_keys.post_id`, `embedding_keys.project_id`
|
||||
- `dismissed_duplicate_pairs.project_id`
|
||||
- `import_definitions.project_id`
|
||||
- `db_notifications.entity_type`, `db_notifications.entity_id`
|
||||
- `posts.status`, `posts.published_at`, `posts.language` — frequently filtered columns
|
||||
- **Note:** SQLite is more forgiving than PostgreSQL for missing FK indexes, but with growing data the query plans will degrade.
|
||||
- **Fix:** Add a new migration with `create index` for every foreign key and frequently filtered column.
|
||||
- **Test:** Verify with `sqlite3` `EXPLAIN QUERY PLAN` that key lookups use indexes.
|
||||
### ~~CSM-015 — Missing DB Indexes on Foreign Keys~~ ✅ FIXED
|
||||
- **Fixed:** 2026-05-09
|
||||
- **What was done:**
|
||||
- Added migration `20260509145208_add_missing_indexes.exs` with indexes for all missing foreign keys and frequently filtered columns:
|
||||
- FK indexes: `media.project_id`, `post_media.post_id`, `post_media.media_id`, `chat_messages.conversation_id`, `embedding_keys.post_id`, `embedding_keys.project_id`, `dismissed_duplicate_pairs.project_id`, `import_definitions.project_id`, `publish_jobs.project_id`.
|
||||
- Filter indexes: `posts.status`, `posts.published_at`, `posts.language`.
|
||||
- Composite index: `db_notifications(entity_type, entity_id)`.
|
||||
- Added 12 tests in `test/bds/csm015_missing_indexes_test.exs` verifying via `EXPLAIN QUERY PLAN` that all indexed columns use index lookups.
|
||||
|
||||
---
|
||||
|
||||
|
||||
24
priv/repo/migrations/20260509145208_add_missing_indexes.exs
Normal file
24
priv/repo/migrations/20260509145208_add_missing_indexes.exs
Normal file
@@ -0,0 +1,24 @@
|
||||
defmodule BDS.Repo.Migrations.AddMissingIndexes do
|
||||
use Ecto.Migration
|
||||
|
||||
def change do
|
||||
# Foreign key indexes
|
||||
create index(:media, [:project_id])
|
||||
create index(:post_media, [:post_id])
|
||||
create index(:post_media, [:media_id])
|
||||
create index(:chat_messages, [:conversation_id])
|
||||
create index(:embedding_keys, [:post_id])
|
||||
create index(:embedding_keys, [:project_id])
|
||||
create index(:dismissed_duplicate_pairs, [:project_id])
|
||||
create index(:import_definitions, [:project_id])
|
||||
create index(:publish_jobs, [:project_id])
|
||||
|
||||
# Frequently filtered columns
|
||||
create index(:posts, [:status])
|
||||
create index(:posts, [:published_at])
|
||||
create index(:posts, [:language])
|
||||
|
||||
# db_notifications lookup columns
|
||||
create index(:db_notifications, [:entity_type, :entity_id])
|
||||
end
|
||||
end
|
||||
75
test/bds/csm015_missing_indexes_test.exs
Normal file
75
test/bds/csm015_missing_indexes_test.exs
Normal file
@@ -0,0 +1,75 @@
|
||||
defmodule BDS.CSM015MissingIndexesTest do
|
||||
use ExUnit.Case, async: false
|
||||
|
||||
alias BDS.Repo
|
||||
|
||||
setup do
|
||||
:ok = Ecto.Adapters.SQL.Sandbox.checkout(BDS.Repo)
|
||||
:ok
|
||||
end
|
||||
|
||||
defp uses_index?(sql) do
|
||||
{:ok, result} = Ecto.Adapters.SQL.query(Repo, "EXPLAIN QUERY PLAN #{sql}", [])
|
||||
|
||||
Enum.any?(result.rows, fn row ->
|
||||
detail = List.last(row)
|
||||
is_binary(detail) and String.contains?(detail, "INDEX")
|
||||
end)
|
||||
end
|
||||
|
||||
describe "foreign key indexes" do
|
||||
test "media.project_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM media WHERE project_id = 'test'")
|
||||
end
|
||||
|
||||
test "post_media.post_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM post_media WHERE post_id = 'test'")
|
||||
end
|
||||
|
||||
test "post_media.media_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM post_media WHERE media_id = 'test'")
|
||||
end
|
||||
|
||||
test "chat_messages.conversation_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM chat_messages WHERE conversation_id = 'test'")
|
||||
end
|
||||
|
||||
test "embedding_keys.post_id is indexed" do
|
||||
assert uses_index?("SELECT label FROM embedding_keys WHERE post_id = 'test'")
|
||||
end
|
||||
|
||||
test "embedding_keys.project_id is indexed" do
|
||||
assert uses_index?("SELECT label FROM embedding_keys WHERE project_id = 'test'")
|
||||
end
|
||||
|
||||
test "dismissed_duplicate_pairs.project_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM dismissed_duplicate_pairs WHERE project_id = 'test'")
|
||||
end
|
||||
|
||||
test "import_definitions.project_id is indexed" do
|
||||
assert uses_index?("SELECT id FROM import_definitions WHERE project_id = 'test'")
|
||||
end
|
||||
end
|
||||
|
||||
describe "frequently filtered columns" do
|
||||
test "posts.status is indexed" do
|
||||
assert uses_index?("SELECT id FROM posts WHERE status = 'published'")
|
||||
end
|
||||
|
||||
test "posts.published_at is indexed" do
|
||||
assert uses_index?("SELECT id FROM posts WHERE published_at > 0")
|
||||
end
|
||||
|
||||
test "posts.language is indexed" do
|
||||
assert uses_index?("SELECT id FROM posts WHERE language = 'en'")
|
||||
end
|
||||
end
|
||||
|
||||
describe "db_notifications lookup" do
|
||||
test "db_notifications entity_type + entity_id is indexed" do
|
||||
assert uses_index?(
|
||||
"SELECT id FROM db_notifications WHERE entity_type = 'post' AND entity_id = 'test'"
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user