Skip to content

perf(findings): index UPPER(title) for case-insensitive title filters - #16387

Merged
Maffooch merged 1 commit into
devfrom
perf/finding-upper-title-index
Oct 16, 2026
Merged

Maffooch merged 1 commit into
devfrom
perf/finding-upper-title-index

Conversation

@Maffooch

Copy link
Copy Markdown
Contributor

Description

Every case-insensitive filter on a finding's title compiles to UPPER("dojo_finding"."title"::text): iexact, istartswith, icontains, and an IN list of upper-cased values. The two existing title indexes are on the raw column (btree(title) and the gin_trgm_ops index that global search uses for fuzzy matching), and Postgres can only use an index whose expression matches the query's. So every one of those filters reads all the titles.

This adds one btree index on UPPER(title) with text_pattern_ops, declared on Finding.Meta.indexes and built by migration 0300 with AddIndexConcurrently (non-atomic, no lock on dojo_finding, the same pattern as 0299). text_pattern_ops lets the same index serve the LIKE 'PREFIX%' that istartswith renders, as well as equality. The migration then runs ANALYZE dojo_finding. Without statistics for the new expression, the planner overestimated an IN list by orders of magnitude and kept walking another index, and autovacuum may not re-analyze a large, settled findings table for a long time.

Measured on 2,000,000 findings (Postgres 16, EXPLAIN ANALYZE and the real count and page queries):

Filter Before After
title__iexact 351 ms 0.4 ms
IN list of 50 upper-cased titles, count 333 ms 1.2 ms
title__istartswith 347 ms 1.4 ms
title__icontains unchanged (still reads every title) unchanged

Cost:

  • The index is 31 MB on 2,000,000 findings, against a 504 MB table that already carries 46 indexes (2.4 GB).
  • The migration took 14.7 s there, Django startup included.
  • Inserting 100,000 findings took the same time with and without it (199 to 214 s with, 216 to 237 s without). Insert time is dominated by the existing indexes and the audit trigger, so the new index did not show up.

icontains and iendswith would need a trigram index on UPPER(title) instead. That one is left out on purpose: in the same test it took a rare substring from 354 ms to 46 ms, but made a common one (24,000 matches) slower, 321 ms to 460 ms.

Testing

  • makemigrations dojo --check --dry-run: no changes detected.
  • migrate dojo 0300, back to 0299 (drops the index), and forward again on the 2,000,000-finding database. After the round trip, with no manual ANALYZE, the IN list count and page queries both use idx_finding_upper_title (1.2 ms each).
  • Postgres stores the expression as upper((title)::text), the same form the ORM's queries use.
  • ruff check over the repo with the pinned ruff==0.16.10.

🤖 Generated with Claude Code

Every case-insensitive title filter compiles to UPPER(title::text), which
neither the plain title index nor the trigram index can serve, so each one
read every title. On 2,000,000 findings: iexact 351 ms -> 0.4 ms, an IN list
of 50 titles 333 ms -> 1.2 ms, istartswith 347 ms -> 1.4 ms. Built
concurrently, then ANALYZE so the planner has statistics for the expression.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch Maffooch added this to the 3.4.200 milestone Oct 16, 2026
@Maffooch
Maffooch requested a review from blakeaowens as a code owner October 16, 2026 22:14
@github-actions github-actions Bot added the New Migration Adding a new migration file. Take care when merging. label Oct 16, 2026
@Maffooch

Copy link
Copy Markdown
Contributor Author

Adversarial review: perf(findings): index UPPER(title) for case-insensitive title filters

Verdict: approve-with-nits
Reviewed at: ffdb97a Profile: code-change
Repo rules followed: .claude/skills/defectdojo-dev/SKILL.md (reviewer mode; the repo has no review-pr skill), plus the generic review-pr procedure

Attempts to break it

  1. Model and migration drift. Ran makemigrations dojo --check --dry-run on the branch: "No changes detected in app 'dojo'". sqlmigrate dojo 0300 emits CREATE INDEX CONCURRENTLY "idx_finding_upper_title" ON "dojo_finding" ((UPPER("title")) text_pattern_ops) followed by ANALYZE dojo_finding. No break.
  2. Expression mismatch (UPPER("title"::text) in the ORM vs UPPER("title") in the index). Postgres stores the index as upper((title)::text) text_pattern_ops. I ran EXPLAIN ANALYZE on ORM-generated SQL against 2,000,004 findings. title__iexact gives an Index Scan on idx_finding_upper_title (0.5 ms). Upper("title") annotated with __in over 50 values, which renders without ::text, gives an Index Scan with = ANY(...) (6.3 ms full rows, 0.13 ms id-only). A selective title__istartswith uses the index with range conditions (0.08 to 1.7 ms). A value containing _ and % is escaped correctly, and the index range plus a LIKE recheck returns the right rows. The DB collation is en_US.utf8, so text_pattern_ops is what makes the LIKE prefix usable. No break.
  3. Non-selective prefix. istartswith("perf cve") matches every row. The planner takes the pkey scan (3.3 s), which is the right choice, and the same as before. icontains still scans (721 ms), as the description says. No break.
  4. Reversibility on a large table. migrate dojo 0299 --plan touches only 0300. Down took 12 s and dropped the index; up took 20 s, and the index came back indisvalid=t, 31 MB. Left applied at 0300. No break.
  5. Fresh install and backend portability. DD_DATABASE_ENGINE defaults to postgresql in settings.dist.py, and 0299 already uses AddIndexConcurrently. Both "Rest Framework Unit Tests" jobs build a fresh test DB through this migration, and they pass. On an empty table, ANALYZE costs nothing. When the DB role does not own the table, ANALYZE warns and skips; it does not error. No break.
  6. Graph conflicts. OSS dev stops at 0299, so nothing conflicts today. Open PR feat(system_settings): add a read-only mode #16386 also adds a 0300_* with the same parent, so whichever PR merges second has to renumber. That is a follow-up, not a defect here.
  7. Index row size. title is CharField(max_length=511), so even 4-byte characters stay under the btree row limit. The plain btree(title) index already carries the same constraint. No break.

Blocking

None. CI: 48 of 48 checks pass, including the Migration Graph Check and both Rest Framework suites.

Nice to have

  • dojo/db_migrations/0300_finding_upper_title_index.py:21 gives the IN-list figure as 345 ms -> 6.9 ms. The PR body gives 333 ms -> 1.2 ms. Both numbers hold up (full-row fetch vs count/id-only, matching my 6.3 ms and 0.13 ms), but the comment and the description should say which query shape each one is.
  • If a CREATE INDEX CONCURRENTLY is interrupted, it leaves an INVALID index, and a re-run fails with "already exists". This is standard AddIndexConcurrently behavior and 0299 shares it, so it is not specific to this PR.
  • The branch is one commit behind dev (fix(dedupe): stop imports failing when findings are deleted under them #16379). Nothing overlaps.

Verified hands-on

Items 1 to 4 and 7, on a 2M-finding Postgres 16 database with this branch checked out.

Checked only statically

The insert-cost claim (100k inserts, no measurable change). I did not re-run it.

@Maffooch
Maffooch added this pull request to the merge queue Oct 16, 2026
Merged via the queue into dev with commit c9cf205 Oct 16, 2026
48 checks passed
@Maffooch
Maffooch deleted the perf/finding-upper-title-index branch October 16, 2026 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Migration Adding a new migration file. Take care when merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants