chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
What each notification says is found for its page alone
Merged Approved
This wraps the notifications query in a subquery that does the filtering, sorting and LIMIT. The "also named" and line-comment-count subqueries now run in an outer select over only the page's rows. I checked the rewrite against the full statement and found no problem.
- The outer columns still come out in the order
row.get(0..14)reads them.shown,ownerandnamestand in for the oldCASE,r.didandr.name. - The line-comment subquery uses
n.subject,n.authorandn.uri. All three come throughn.*. - The
LEFT JOIN repo r, the filters, the cursor andLIMIT ?4stay in the inner query. The outerORDER BYrepeats the same order, so page order is unchanged.
I couldn't see CI results, so I haven't confirmed that the notification tests and the plans gate pass. The PR says it relies on them, and the plans gate is the one most likely to be affected by a changed plan.
All checks have passed
8 passed, 2 skipped
Merged as 9cd3032
What changes
The notifications query now picks its page first: every reason's rows, filtered, sorted, cut to the limit. Only then does it work out the two things said of each notification on that page: whether the account is also named in it, and how many line comments came with a review. The notifications returned are the same.
Why
On prod today the notifications statement ran 173 times, with a mean of 277ms and a max of 4.4s, the slowest of the index's statements. It gathers every row the account was ever notified of, sorts them, and keeps a page. SQLite works out a statement's result columns before it sorts (unless it was built with sorter references, which it isn't by default). So those two correlated subqueries ran for every row ever notified, and for the owner of a busy repository that is thousands of rows for a page of 50. Moved outside the limit, they run once per row returned. The inner query keeps its own ORDER BY, so SQLite cannot flatten it back into the outer one.
How it was checked
The notification tests in the index and serve suites cover each reason, line comments, mentions alongside other reasons, kinds and cursors. They run in CI, along with the plans gate. After deploy, the check is the statement's mean and max in
stats, against 277ms and 4.4s.Shortcomings
It still gathers and sorts every row the account was ever notified of. Cutting that down means taking each reason's own limit and cursor, with the filters moved into each reason. That is a larger rewrite, worth doing only if this one leaves it slow.
🤖 Generated with Claude Code