lists: count in SQL and group threads in one pass
Each page built five full lists just to count them for the sidebar,
and grouping searched the groups linearly per message and fetched each
thread with its own query. Counts now run the same filter clauses as
COUNT(*), grouping is a map keyed by thread, and the threads a section
needs come back in one query. For 3,000 messages the counts drop from
42 ms to 2 ms and the page from ~60 ms to ~21 ms (release build).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
diff --git a/src/lists.rs b/src/lists.rs
index 33bc7c8..3330235 100644
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -2,8 +2,10 @@
//! individual messages, grouped by thread; a group holds only the thread's
//! messages that belong in that section.
+use std::collections::BTreeMap;
+
use crate::Error;
-use crate::model::{Message, State, Thread, User};
+use crate::model::{Message, State, Thread, ThreadId, User};
use crate::store::{Filter, Store};
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -80,14 +82,25 @@ pub fn sections(store: &Store, me: &User, view: &View) -> Result<Vec<Section>, E
Ok(out)
}
-/// How many messages a view lists, for the sidebar.
+/// How many messages a view lists, for the sidebar: the same filters
+/// [`sections`] loads, counted in SQL.
pub fn count(store: &Store, me: &User, view: &View) -> Result<usize, Error> {
- let n = sections(store, me, view)?
- .iter()
- .flat_map(|s| &s.groups)
- .map(|g| g.rows.len())
- .sum();
- Ok(n)
+ let filters = match view {
+ View::ForMe => State::LANES
+ .map(|state| Filter::ForMe {
+ user: &me.login,
+ state,
+ })
+ .to_vec(),
+ View::Lane(state) => vec![Filter::State(*state)],
+ View::Search(query) => match query.trim() {
+ "" => Vec::new(),
+ query => vec![Filter::Search(query)],
+ },
+ };
+ filters
+ .into_iter()
+ .try_fold(0usize, |n, f| Ok(n.saturating_add(store.count(f)?)))
}
/// Takes messages oldest first, as the store returns them.
@@ -97,17 +110,20 @@ fn section(
msgs: Vec<Message>,
order: Order,
) -> Result<Section, Error> {
- let mut groups: Vec<Group> = Vec::new();
+ let mut rows: BTreeMap<ThreadId, Vec<Message>> = BTreeMap::new();
for m in msgs {
- if let Some(g) = groups.iter_mut().find(|g| g.thread.id == m.thread) {
- g.rows.push(m);
- } else if let Some(thread) = store.thread(&m.thread)? {
- groups.push(Group {
- thread,
- rows: vec![m],
- });
- }
+ rows.entry(m.thread.clone()).or_default().push(m);
}
+ let mut threads = store.threads(&rows.keys().cloned().collect())?;
+ let mut groups: Vec<Group> = rows
+ .into_iter()
+ .filter_map(|(id, rows)| {
+ Some(Group {
+ thread: threads.remove(&id)?,
+ rows,
+ })
+ })
+ .collect();
match order {
Order::OldestFirst => groups.sort_by_key(|g| g.rows.first().map(|m| m.at)),
Order::NewestFirst => {
@@ -182,6 +198,27 @@ mod tests {
let store = fixtures::store().unwrap();
let sam = store.user(SAM).unwrap().unwrap();
assert_eq!(count(&store, &sam, &View::ForMe).unwrap(), 7);
+
+ // Counts come from SQL, so pin them to the rows each view lists.
+ for login in [SAM, ALEX] {
+ let me = store.user(login).unwrap().unwrap();
+ let views = [
+ View::ForMe,
+ View::Search("estimate".into()),
+ View::Search(" ".into()),
+ ]
+ .into_iter()
+ .chain(State::LANES.map(View::Lane));
+ for view in views {
+ let rows: usize = sections(&store, &me, &view)
+ .unwrap()
+ .iter()
+ .flat_map(|s| &s.groups)
+ .map(|g| g.rows.len())
+ .sum();
+ assert_eq!(count(&store, &me, &view).unwrap(), rows, "{login} {view:?}");
+ }
+ }
}
#[test]
diff --git a/src/store.rs b/src/store.rs
index 05d98e3..9e32d7d 100644
--- a/src/store.rs
+++ b/src/store.rs
@@ -10,11 +10,13 @@ use std::sync::{Arc, Mutex, MutexGuard, PoisonError};
use camino::Utf8Path;
use jiff::civil::DateTime;
use rusqlite::types::{FromSql, FromSqlError, FromSqlResult, ToSqlOutput, ValueRef};
-use rusqlite::{Connection, OptionalExtension as _, Params, Row, ToSql, Transaction, params};
+use rusqlite::{
+ Connection, OptionalExtension as _, Params, Row, ToSql, Transaction, params, params_from_iter,
+};
use crate::Error;
use crate::model::{
- Account, Comment, Event, Kind, Message, MessageId, State, Thread, User, Values,
+ Account, Comment, Event, Kind, Message, MessageId, State, Thread, ThreadId, User, Values,
};
/// Applied in order; `PRAGMA user_version` counts how many have run.
@@ -62,6 +64,37 @@ pub enum Filter<'a> {
Search(&'a str),
}
+impl Filter<'_> {
+ /// The WHERE clause over [`MESSAGE_JOINS`] and its arguments, shared
+ /// by loading and counting so the two can't disagree.
+ fn sql(self) -> (&'static str, Vec<rusqlite::types::Value>) {
+ use rusqlite::types::Value::Text;
+ match self {
+ Filter::State(state) => (
+ "m.kind = 'received' AND m.state = ?1",
+ vec![Text(state.slug().to_owned())],
+ ),
+ Filter::ForMe { user, state } => (
+ "m.kind = 'received' AND m.state = ?1 AND (
+ EXISTS (SELECT 1 FROM assignees a WHERE a.message = m.id AND a.user = ?2)
+ OR (?1 = 'inbox' AND NOT EXISTS (SELECT 1 FROM assignees a WHERE a.message = m.id))
+ )",
+ vec![Text(state.slug().to_owned()), Text(user.to_owned())],
+ ),
+ Filter::Search(text) => (
+ r"t.subject LIKE ?1 ESCAPE '\' OR m.from_name LIKE ?1 ESCAPE '\'
+ OR u.slug LIKE ?1 ESCAPE '\' OR m.body LIKE ?1 ESCAPE '\'",
+ vec![Text(format!("%{}%", escape_like(text)))],
+ ),
+ }
+ }
+}
+
+/// What a [`Filter`]'s clause can see besides the message: its thread
+/// (`t`) and the user who sent it (`u`).
+const MESSAGE_JOINS: &str =
+ "JOIN threads t ON t.id = m.thread LEFT JOIN users u ON u.login = m.sent_by";
+
/// Where "now" comes from: the fixed clock fixtures are written against,
/// or the system clock for a live server.
#[derive(Debug, Clone, Copy)]
@@ -243,30 +276,40 @@ impl Store {
/// Messages matching the filter, oldest first.
pub fn messages(&self, filter: Filter<'_>) -> Result<Vec<Message>, Error> {
+ let (clause, args) = filter.sql();
+ load_messages(&self.lock().conn, clause, params_from_iter(args))
+ }
+
+ /// How many messages match the filter, without loading them.
+ pub fn count(&self, filter: Filter<'_>) -> Result<usize, Error> {
+ let (clause, args) = filter.sql();
let inner = self.lock();
- let conn = &inner.conn;
- match filter {
- Filter::State(state) => {
- load_messages(conn, "m.kind = 'received' AND m.state = ?1", [state])
- }
- Filter::ForMe { user, state } => load_messages(
- conn,
- "m.kind = 'received' AND m.state = ?1 AND (
- EXISTS (SELECT 1 FROM assignees a WHERE a.message = m.id AND a.user = ?2)
- OR (?1 = 'inbox' AND NOT EXISTS (SELECT 1 FROM assignees a WHERE a.message = m.id))
- )",
- params![state, user],
- ),
- Filter::Search(text) => {
- let pattern = format!("%{}%", escape_like(text));
- load_messages(
- conn,
- r"t.subject LIKE ?1 ESCAPE '\' OR m.from_name LIKE ?1 ESCAPE '\'
- OR u.slug LIKE ?1 ESCAPE '\' OR m.body LIKE ?1 ESCAPE '\'",
- [pattern],
- )
- }
- }
+ let mut stmt = inner.conn.prepare_cached(&format!(
+ "SELECT COUNT(*) FROM messages m {MESSAGE_JOINS} WHERE {clause}"
+ ))?;
+ let n: i64 = stmt.query_row(params_from_iter(args), |r| r.get(0))?;
+ // COUNT(*) is never negative.
+ Ok(usize::try_from(n).unwrap_or_default())
+ }
+
+ /// The threads with these ids, in one query.
+ pub fn threads(&self, ids: &BTreeSet<ThreadId>) -> Result<BTreeMap<ThreadId, Thread>, Error> {
+ let inner = self.lock();
+ let mut stmt = inner.conn.prepare_cached(
+ "SELECT id, account, subject FROM threads
+ WHERE id IN (SELECT value FROM json_each(?1))",
+ )?;
+ let threads = stmt
+ .query_map([json(ids)?], |r| {
+ Ok(Thread {
+ id: r.get(0)?,
+ account: r.get(1)?,
+ subject: r.get(2)?,
+ })
+ })?
+ .map(|t| t.map(|t| (t.id.clone(), t)))
+ .collect::<Result<_, _>>()?;
+ Ok(threads)
}
/// The thread's messages and comments in time order. A message sorts
@@ -978,9 +1021,7 @@ fn load_messages(
m.from_name, m.from_addr, m.state, m.folder, m.sent_by, m.sent_to,
(SELECT json_group_array(user)
FROM (SELECT user FROM assignees WHERE message = m.id ORDER BY user))
- FROM messages m
- JOIN threads t ON t.id = m.thread
- LEFT JOIN users u ON u.login = m.sent_by
+ FROM messages m {MESSAGE_JOINS}
WHERE {filter}
ORDER BY m.at, m.id"
);
@@ -2193,6 +2234,7 @@ mod tests {
assert!(store.history(&fixtures::id(4)).is_err());
exec(&store, "ALTER TABLE threads RENAME TO gone_threads;");
db_err(store.add_comment(SAM, &fixtures::id(1), "hi"));
+ assert!(store.threads(&BTreeSet::new()).is_err());
exec(&store, "ALTER TABLE users RENAME TO gone_users;");
assert!(store.users().is_err());
assert!(store.is_empty().is_err());