Skip to content

perf(findings): resolve tag filter names to tag ids so the planner sees how common a tag is - #16634

Merged
Maffooch merged 2 commits into
devfrom
perf/tag-filters-resolve-tag-ids
Oct 30, 2026
Merged

Maffooch merged 2 commits into
devfrom
perf/tag-filters-resolve-tag-ids

Conversation

@Maffooch

@Maffooch Maffooch commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Description

Tag-filtered GET /api/v2/findings/ requests stay slow on large instances after #16509. Performance reports show about 30 queries per request but 5 to 10 s of SQL, for example tags=<tag>&test__engagement__product=<id>&active=true&limit=100&offset=400.

#16509 replaced the DISTINCT join with an EXISTS on the tags through table, but that subquery still joins the tag table and matches on its name:

EXISTS (SELECT 1 FROM dojo_finding_tags U0
        JOIN dojo_tagulous_finding_tags U2 ON U0.tagulous_finding_tags_id = U2.id
        WHERE U0.finding_id = dojo_finding.id AND U2.name = 'some-tag')

The planner cannot tell which tag id that name resolves to, so it assumes the average number of rows per tag (rows ÷ distinct tags). A tag carried by most findings, such as a scanner or source tag, is the worst case. On a 400k-finding dataset where one tag is on every finding, a COUNT over the 160k findings matching active + severity + that tag:

  • today: the subquery is estimated at 573 rows (actual 400,001), so the planner builds a HashAggregate of every tagged row, which spills to disk, then does a nested loop of 400,001 primary-key probes on dojo_finding. 20,636 ms.
  • with the tag id as a literal (U0.tagulous_finding_tags_id IN (1)): the planner sees the id's real frequency from the column statistics and picks a parallel hash join. 2,487 ms.

The finding list runs that filter at least twice per page (the COUNT, then the page or id query), so the request pays it twice.

This change resolves the matching tags to their ids first, with one indexed lookup on the tag table, and the subquery tests the through table against those ids:

EXISTS (SELECT 1 FROM dojo_finding_tags U0
        WHERE U0.finding_id = dojo_finding.id AND U0.tagulous_finding_tags_id IN (1))

It covers every filter built on tag_exists: tag, tags, tags__and, not_tag, not_tags and the test__, test__engagement__ and test__engagement__product__ variants. A lookup that matches more than TAG_ID_LITERAL_LIMIT (1000) tags, such as a one-letter tag= (icontains) value, keeps the join instead of writing thousands of literals into the query. An unknown tag resolves to no ids: the filter matches nothing and its not_ form matches everything, as before.

tags__and (and its test__, test__engagement__ and test__engagement__product__ forms) resolves all of its listed names in one name__in read and builds one EXISTS per tag id. When a listed name is no tag, the AND cannot match, so it returns an empty queryset without building any subquery.

Cost: one extra query per tag filter (the tag lookup, an index scan on the tag name), whatever the number of AND-ed tags.

Measurements

Whole requests through the API, 400k findings over 1,000 products, the target product holding 21k active findings with the tag (the tag is on all 400k findings), 5-run medians on a loaded development machine. Each row compares the old and new tag_exists on the same data and code. Response bodies (rows, order, counts) are byte-identical before and after for every row.

request user SQL before SQL after wall before wall after
tags=<tag>&test__engagement__product=<id>&active=true&limit=100&offset=400 superuser 6,518 ms 904 ms 9,263 ms 1,765 ms
same global Owner 3,946 ms 982 ms 6,994 ms 2,385 ms
same product Owner 2,542 ms 552 ms 4,764 ms 1,817 ms
same, offset=1200 global Owner 2,478 ms 730 ms 4,579 ms 1,805 ms
tags__and=<two less common tags>&severity=Critical&limit=100 global Reader 729 ms 765 ms 2,364 ms 2,181 ms

The last row filters on tags that are each on a small share of findings, which the old join already planned well. It stays within run-to-run noise. In this measurement each AND-ed tag still had its own lookup (two more queries); they are now one. One product-Owner request at offset=1200 measured 972 ms before and 1,644 ms after in this run. Its COUNT, repeated six times in each form with EXPLAIN ANALYZE, had a median of about 500 ms before and about 310 ms after, so that request-level number is noise from the loaded machine.

Test results

  • New in unittests/test_finding_tag_filters.py: the subquery names the tag id and never joins the tag table, for exact, in and icontains; an unknown tag matches nothing and its negation everything, directly and through the FilterSet (tags__and, test__engagement__tags__and, not_tags, each also compared to the old JOIN form); a lookup over the literal limit keeps the join and returns the same rows; tags__and with three tags reads the tag table once. The id, literal-limit and single-read tests fail without this change.
  • The existing tests in that file, which pin every tag filter's results and counts to the old JOIN form and walk every page across orderings and prefetches, pass unchanged: manage.py test unittests.test_finding_tag_filters, 25 tests, OK.
  • ruff check (0.16.10, ruff.toml): clean.

Documentation

No user-visible change: filter names, parameters and results are unchanged.

Checklist

  • Make sure to rebase your PR against the very latest dev.
  • Submit all PRs, features and bug fixes alike, against the dev branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is Ruff compliant (see ruff.toml).
  • Your code is python 3.13 compliant.
  • Add applicable tests to the unit tests.

tag_exists() matched the tag through a join on the tag table's name, so the
planner could not tell which tag a query was about and assumed the average
number of rows per tag. For a tag carried by most findings that estimate was
off by three orders of magnitude (573 expected, 400,001 actual), and the
finding list's COUNT and id queries took a nested-loop semi-join that probed
every tagged row: 20.6 s for one COUNT over 400k tagged findings, 2.5 s once
the subquery names the tag id.

The matching tags are now read first (one indexed lookup on the tag table)
and the EXISTS subquery tests the through table against those ids. A lookup
that matches more than TAG_ID_LITERAL_LIMIT tags (a short icontains value)
keeps the join. Results, counts and paging are unchanged: the existing
equivalence tests against the old JOIN form pass as they are.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch Maffooch added this to the 3.5.0 milestone Oct 30, 2026
@Maffooch
Maffooch requested a review from blakeaowens as a code owner October 30, 2026 02:07
@Maffooch

Copy link
Copy Markdown
Contributor Author

Adversarial review: full tier, head 32e35b6

Verdict: changes requested (R1 to R3). The rewrite is semantically identical to the join on every input I tried, and it fixes the misestimate on skewed tags. What remains is query count on tags__and, plus test gaps on the two branches the existing suite does not reach.

Findings

  • R1 suggestion this-pr dojo/filters.py:181-192 (TagNameANDFilter.filter): each AND-ed tag now costs its own tag lookup. tags__and, test__tags__and, test__engagement__tags__and and test__engagement__product__tags__and therefore add one query per listed tag, bounded only by URL length. Resolve every name in one name__in read (a name-to-id dict), then build one Exists(... __in=[id]) per tag. When any AND-ed name is unknown the whole result is empty, so return queryset.none() without building the subqueries.
  • R2 nit this-pr unittests/test_finding_tag_filters.py (test_a_lookup_matching_many_tags_keeps_the_name_join): the over-limit fallback is the one branch whose SQL differs from the main path, and the test pins only its SQL shape. Also assert its result set (the finding tagged many-tags-* is returned, and its negation excludes it).
  • R3 nit this-pr unittests/test_finding_tag_filters.py: the empty-id-list path is tested only by calling tag_exists(..., "in", [...]) directly. Add a FilterSet-level case for an unknown tag through tags__and (expect no rows) and not_tags (expect every row). The first goes through TagNameANDFilter with exact, which the new tests do not exercise.

What I verified

  • Same results as the join. I compared old (join on name) against new (resolve ids, then IN (ids)) on a 400k-finding dataset by counting critical findings, in SQL. Seven cases gave identical counts: an exact tag (370 = 370); an upper-case variant of it (0 = 0); a leading-space variant (0 = 0); an unknown tag (0 = 0); an in list of two known tags and one unknown (741 = 741); an icontains hitting 11 tag ids (4,075 = 4,075); and an icontains hitting one popular tag (80,000 = 80,000). By construction EXISTS(through JOIN tag WHERE name P) equals tag_id IN (SELECT id FROM tag WHERE name P), with the same lookup on the same column. Case and whitespace handling is therefore unchanged: exact stays case-sensitive and nothing strips values. The tagulous TagModelQuerySet (django-tagulous 2.2.3) does not override filter, so name=<v> on the tag model is a plain exact lookup. Tagulous's case-insensitive rewrite lives only in the tagged model's queryset, which neither form uses.
  • EXPLAIN on skewed data, with the COUNT filtered by active and severity (machine heavily loaded, so treat timings as order of magnitude):
    • Tag on every finding (400,001 rows): the old form estimates the subquery at 573 rows, builds a HashAggregate of 400,001 rows, then runs a nested loop of PK probes, for 65 s. The new form takes a parallel bitmap/nested-loop path from the finding side in 6 to 20 s.
    • Tag on 160,000 rows: old 19 s; new is a parallel hash join at about 10 s.
    • Tag on 1,854 rows: old 0.8 to 1.4 s, new 0.6 to 0.9 s, so no regression for rare tags.
    • NOT EXISTS on a rare tag: Hash Anti Join in both forms, 14 s vs 11 s.
  • The extra query is bounded for tag, tags and not_tags: one indexed read of the tag table per filter, capped at TAG_ID_LITERAL_LIMIT + 1 rows by the slice. tags__and is R1.
  • Empty id list. An unknown tag compiles __in=[], so Django raises EmptyResultSet during compilation instead of running a query that returns 0 rows. Through qs.filter(Exists) and qs.filter(~Exists) that becomes none and all, which your test pins. I searched for code that compiles a filtered finding queryset to raw SQL (sql_with_params, get_compiler().as_sql()). The only hits in this repo are utils_cascade_delete.py, which runs on delete querysets rather than on user tag filters, and the reimporter, which already guards EmptyResultSet. Nothing reachable from these filters breaks.
  • The new tests fail on dev, by reading: on dev the first fails its assertNotIn on the tag table, and the third fails on the missing TAG_ID_LITERAL_LIMIT attribute. Not run locally.
  • CI: read, not re-run. 48 of 48 checks pass on 32e35b6, including Rest Framework Unit Tests, Performance Tests and ruff.

…ng for an unknown one

tags__and (and its test__, test__engagement__ and test__engagement__product__
forms) looked each listed tag up on its own, one query per tag. The names
are now resolved in one name__in read, and one EXISTS per tag id is built
from it. A listed name that is no tag cannot be present, so the filter
returns an empty queryset without building the subqueries.

Tests: the over-limit fallback is checked on the rows it returns, not only
its SQL, and an unknown tag is checked through the FilterSet for tags__and,
test__engagement__tags__and and not_tags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Maffooch

Copy link
Copy Markdown
Contributor Author

Review findings: resolution

  • R1 fixed in 1dee83c: TagNameANDFilter resolves every listed name in one name__in read into a name-to-id dict and builds one EXISTS per id; when any listed name is no tag it returns queryset.none() without building subqueries. test_tags_and_reads_the_tag_table_once asserts one tag-table read for three AND-ed tags (3 before this commit) and the same ids as the old JOIN form.
  • R2 fixed in 1dee83c: the over-limit test now creates a finding tagged many-tags-0 and one tagged with an unrelated tag, and asserts the join-form condition returns exactly the first and its negation excludes it and keeps the second.
  • R3 fixed in 1dee83c: test_unknown_tags_through_the_filterset runs tags__and=no-such-tag, tags__and=alpha,no-such-tag and test__engagement__tags__and=shared,no-such-tag (no rows), not_tags=no-such-tag (every row) and not_tags=no-such-tag,beta, each also compared to the old JOIN form.

@Maffooch
Maffooch added this pull request to the merge queue Oct 30, 2026
Merged via the queue into dev with commit 5e8983d Oct 30, 2026
49 checks passed
@Maffooch
Maffooch deleted the perf/tag-filters-resolve-tag-ids branch October 30, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants