automatically omit notes or cards table when possible

rslib/src/findreplace.rs

@@ -117,7 +117,7 @@ mod test {
         note2.fields[0] = "three aaa".into();
         col.add_note(&mut note2, DeckID(1))?;
 
-        let nids = col.search_notes_only("")?;
+        let nids = col.search_notes("")?;
         let cnt = col.find_and_replace(nids.clone(), "(?i)AAA", "BBB", None)?;
         assert_eq!(cnt, 2);
 

rslib/src/notetype/schemachange.rs

@@ -61,7 +61,7 @@ impl Collection {
         if !ords_changed(&ords, previous_field_count) {
             if nt.config.sort_field_idx != previous_sort_idx {
                 // only need to update sort field
-                let nids = self.search_notes_only(&format!("mid:{}", nt.id))?;
+                let nids = self.search_notes(&format!("mid:{}", nt.id))?;
                 for nid in nids {
                     let mut note = self.storage.get_note(nid)?.unwrap();
                     note.prepare_for_update(nt, normalize_text)?;
@@ -75,7 +75,7 @@ impl Collection {
 
         self.storage.set_schema_modified()?;
 
-        let nids = self.search_notes_only(&format!("mid:{}", nt.id))?;
+        let nids = self.search_notes(&format!("mid:{}", nt.id))?;
         let usn = self.usn()?;
         for nid in nids {
             let mut note = self.storage.get_note(nid)?.unwrap();

rslib/src/search/cards.rs

@@ -1,7 +1,10 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::{parser::Node, sqlwriter::node_to_sql};
+use super::{
+    parser::Node,
+    sqlwriter::{RequiredTable, SqlWriter},
+};
 use crate::card::CardID;
 use crate::card::CardType;
 use crate::collection::Collection;
@@ -9,6 +12,7 @@ use crate::config::SortKind;
 use crate::err::Result;
 use crate::search::parser::parse;
 
+#[derive(Debug, PartialEq, Clone)]
 pub enum SortMode {
     NoOrder,
     FromConfig,
@@ -16,24 +20,54 @@ pub enum SortMode {
     Custom(String),
 }
 
-impl Collection {
+impl SortMode {
-    pub fn search_cards(&mut self, search: &str, mode: SortMode) -> Result<Vec<CardID>> {
+    fn required_table(&self) -> RequiredTable {
-        let top_node = Node::Group(parse(search)?);
+        match self {
-        let (sql, args) = node_to_sql(self, &top_node, self.normalize_note_text())?;
+            SortMode::NoOrder => RequiredTable::Cards,
+            SortMode::FromConfig => unreachable!(),
+            SortMode::Builtin { kind, .. } => kind.required_table(),
+            SortMode::Custom(ref text) => {
+                if text.contains("n.") {
+                    RequiredTable::CardsAndNotes
+                } else {
+                    RequiredTable::Cards
+                }
+            }
+        }
+    }
+}
 
-        let mut sql = format!(
+impl SortKind {
-            "select c.id from cards c, notes n where c.nid=n.id and {}",
+    fn required_table(self) -> RequiredTable {
-            sql
+        match self {
-        );
+            SortKind::NoteCreation
+            | SortKind::NoteMod
+            | SortKind::NoteField
+            | SortKind::NoteType
+            | SortKind::NoteTags
+            | SortKind::CardTemplate => RequiredTable::CardsAndNotes,
+            SortKind::CardMod
+            | SortKind::CardReps
+            | SortKind::CardDue
+            | SortKind::CardEase
+            | SortKind::CardLapses
+            | SortKind::CardInterval
+            | SortKind::CardDeck => RequiredTable::Cards,
+        }
+    }
+}
+
+impl Collection {
+    pub fn search_cards(&mut self, search: &str, mut mode: SortMode) -> Result<Vec<CardID>> {
+        let top_node = Node::Group(parse(search)?);
+        self.resolve_config_sort(&mut mode);
+        let writer = SqlWriter::new(self);
+
+        let (mut sql, args) = writer.build_cards_query(&top_node, mode.required_table())?;
 
         match mode {
             SortMode::NoOrder => (),
-            SortMode::FromConfig => {
+            SortMode::FromConfig => unreachable!(),
-                let kind = self.get_browser_sort_kind();
-                prepare_sort(self, kind)?;
-                sql.push_str(" order by ");
-                write_order(&mut sql, kind, self.get_browser_sort_reverse())?;
-            }
             SortMode::Builtin { kind, reverse } => {
                 prepare_sort(self, kind)?;
                 sql.push_str(" order by ");
@@ -52,6 +86,16 @@ impl Collection {
 
         Ok(ids)
     }
+
+    /// If the sort mode is based on a config setting, look it up.
+    fn resolve_config_sort(&self, mode: &mut SortMode) {
+        if mode == &SortMode::FromConfig {
+            *mode = SortMode::Builtin {
+                kind: self.get_browser_sort_kind(),
+                reverse: self.get_browser_sort_reverse(),
+            }
+        }
+    }
 }
 
 /// Add the order clause to the sql.

rslib/src/search/notes.rs

@@ -1,38 +1,17 @@
 // Copyright: Ankitects Pty Ltd and contributors
 // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
-use super::{parser::Node, sqlwriter::node_to_sql};
+use super::{parser::Node, sqlwriter::SqlWriter};
 use crate::collection::Collection;
 use crate::err::Result;
 use crate::notes::NoteID;
 use crate::search::parser::parse;
 
 impl Collection {
-    /// This supports card queries as well, but is slower.
     pub fn search_notes(&mut self, search: &str) -> Result<Vec<NoteID>> {
-        self.search_notes_inner(search, |sql| {
-            format!(
-                "select distinct n.id from cards c, notes n where c.nid=n.id and {}",
-                sql
-            )
-        })
-    }
-
-    /// This only supports note-related search terms.
-    pub fn search_notes_only(&mut self, search: &str) -> Result<Vec<NoteID>> {
-        self.search_notes_inner(search, |sql| {
-            format!("select n.id from notes n where {}", sql)
-        })
-    }
-
-    fn search_notes_inner<F>(&mut self, search: &str, build_sql: F) -> Result<Vec<NoteID>>
-    where
-        F: FnOnce(String) -> String,
-    {
         let top_node = Node::Group(parse(search)?);
-        let (sql, args) = node_to_sql(self, &top_node, self.normalize_note_text())?;
+        let writer = SqlWriter::new(self);
-
+        let (sql, args) = writer.build_notes_query(&top_node)?;
-        let sql = build_sql(sql);
 
         let mut stmt = self.storage.db.prepare(&sql)?;
         let ids: Vec<_> = stmt

rslib/src/search/sqlwriter.rs

@@ -16,25 +16,17 @@ use lazy_static::lazy_static;
 use regex::{Captures, Regex};
 use std::{borrow::Cow, fmt::Write};
 
-struct SqlWriter<'a> {
+pub(crate) struct SqlWriter<'a> {
     col: &'a mut Collection,
     sql: String,
     args: Vec<String>,
     normalize_note_text: bool,
-}
+    table: RequiredTable,
-
-pub(super) fn node_to_sql(
-    req: &mut Collection,
-    node: &Node,
-    normalize_note_text: bool,
-) -> Result<(String, Vec<String>)> {
-    let mut sctx = SqlWriter::new(req, normalize_note_text);
-    sctx.write_node_to_sql(&node)?;
-    Ok((sctx.sql, sctx.args))
 }
 
 impl SqlWriter<'_> {
-    fn new(col: &mut Collection, normalize_note_text: bool) -> SqlWriter<'_> {
+    pub(crate) fn new(col: &mut Collection) -> SqlWriter<'_> {
+        let normalize_note_text = col.normalize_note_text();
         let sql = String::new();
         let args = vec![];
         SqlWriter {
@@ -42,6 +34,52 @@ impl SqlWriter<'_> {
             sql,
             args,
             normalize_note_text,
+            table: RequiredTable::CardsOrNotes,
+        }
+    }
+
+    pub(super) fn build_cards_query(
+        mut self,
+        node: &Node,
+        table: RequiredTable,
+    ) -> Result<(String, Vec<String>)> {
+        self.table = table.combine(node.required_table());
+        self.write_cards_table_sql();
+        self.write_node_to_sql(&node)?;
+        Ok((self.sql, self.args))
+    }
+
+    pub(super) fn build_notes_query(mut self, node: &Node) -> Result<(String, Vec<String>)> {
+        self.table = RequiredTable::Notes.combine(node.required_table());
+        self.write_notes_table_sql();
+        self.write_node_to_sql(&node)?;
+        Ok((self.sql, self.args))
+    }
+
+    fn write_cards_table_sql(&mut self) {
+        let sql = match self.table {
+            RequiredTable::Cards => "select c.id from cards c where ",
+            _ => "select c.id from cards c, notes n where c.nid=n.id and ",
+        };
+        self.sql.push_str(sql);
+    }
+
+    fn write_notes_table_sql(&mut self) {
+        let sql = match self.table {
+            RequiredTable::Notes => "select n.id from notes n where ",
+            _ => "select distinct n.id from cards c, notes n where c.nid=n.id and ",
+        };
+        self.sql.push_str(sql);
+    }
+
+    /// As an optimization we can omit the cards or notes tables from
+    /// certain queries. For code that specifies a note id, we need to
+    /// choose the appropriate column name.
+    fn note_id_column(&self) -> &'static str {
+        match self.table {
+            RequiredTable::Notes | RequiredTable::CardsAndNotes => "n.id",
+            RequiredTable::Cards => "c.nid",
+            RequiredTable::CardsOrNotes => unreachable!(),
         }
     }
 
@@ -111,7 +149,7 @@ impl SqlWriter<'_> {
                 write!(self.sql, "(c.flags & 7) == {}", flag).unwrap();
             }
             SearchNode::NoteIDs(nids) => {
-                write!(self.sql, "n.id in ({})", nids).unwrap();
+                write!(self.sql, "{} in ({})", self.note_id_column(), nids).unwrap();
             }
             SearchNode::CardIDs(cids) => {
                 write!(self.sql, "c.id in ({})", cids).unwrap();
@@ -439,8 +477,78 @@ fn text_to_re(glob: &str) -> String {
     text2.into()
 }
 
+#[derive(Debug, PartialEq, Clone, Copy)]
+pub enum RequiredTable {
+    Notes,
+    Cards,
+    CardsAndNotes,
+    CardsOrNotes,
+}
+
+impl RequiredTable {
+    fn combine(self, other: RequiredTable) -> RequiredTable {
+        match (self, other) {
+            (RequiredTable::CardsAndNotes, _) => RequiredTable::CardsAndNotes,
+            (_, RequiredTable::CardsAndNotes) => RequiredTable::CardsAndNotes,
+            (RequiredTable::CardsOrNotes, b) => b,
+            (a, RequiredTable::CardsOrNotes) => a,
+            (a, b) => {
+                if a == b {
+                    a
+                } else {
+                    RequiredTable::CardsAndNotes
+                }
+            }
+        }
+    }
+}
+
+impl Node<'_> {
+    fn required_table(&self) -> RequiredTable {
+        match self {
+            Node::And => RequiredTable::CardsOrNotes,
+            Node::Or => RequiredTable::CardsOrNotes,
+            Node::Not(node) => node.required_table(),
+            Node::Group(nodes) => nodes.iter().fold(RequiredTable::CardsOrNotes, |cur, node| {
+                cur.combine(node.required_table())
+            }),
+            Node::Search(node) => node.required_table(),
+        }
+    }
+}
+
+impl SearchNode<'_> {
+    fn required_table(&self) -> RequiredTable {
+        match self {
+            SearchNode::AddedInDays(_) => RequiredTable::Cards,
+            SearchNode::Deck(_) => RequiredTable::Cards,
+            SearchNode::Rated { .. } => RequiredTable::Cards,
+            SearchNode::State(_) => RequiredTable::Cards,
+            SearchNode::Flag(_) => RequiredTable::Cards,
+            SearchNode::CardIDs(_) => RequiredTable::Cards,
+            SearchNode::Property { .. } => RequiredTable::Cards,
+
+            SearchNode::UnqualifiedText(_) => RequiredTable::Notes,
+            SearchNode::SingleField { .. } => RequiredTable::Notes,
+            SearchNode::Tag(_) => RequiredTable::Notes,
+            SearchNode::Duplicates { .. } => RequiredTable::Notes,
+            SearchNode::Regex(_) => RequiredTable::Notes,
+            SearchNode::NoCombining(_) => RequiredTable::Notes,
+            SearchNode::WordBoundary(_) => RequiredTable::Notes,
+            SearchNode::NoteTypeID(_) => RequiredTable::Notes,
+            SearchNode::NoteType(_) => RequiredTable::Notes,
+
+            SearchNode::NoteIDs(_) => RequiredTable::CardsOrNotes,
+            SearchNode::WholeCollection => RequiredTable::CardsOrNotes,
+
+            SearchNode::CardTemplate(_) => RequiredTable::CardsAndNotes,
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
+    use super::*;
     use crate::{
         collection::{open_collection, Collection},
         i18n::I18n,
@@ -451,12 +559,14 @@ mod test {
     use tempfile::tempdir;
 
     use super::super::parser::parse;
-    use super::*;
 
     // shortcut
     fn s(req: &mut Collection, search: &str) -> (String, Vec<String>) {
         let node = Node::Group(parse(search).unwrap());
-        node_to_sql(req, &node, true).unwrap()
+        let mut writer = SqlWriter::new(req);
+        writer.table = RequiredTable::Notes.combine(node.required_table());
+        writer.write_node_to_sql(&node).unwrap();
+        (writer.sql, writer.args)
     }
 
     #[test]
@@ -657,4 +767,36 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    fn required_table() {
+        assert_eq!(
+            Node::Group(parse("").unwrap()).required_table(),
+            RequiredTable::CardsOrNotes
+        );
+        assert_eq!(
+            Node::Group(parse("test").unwrap()).required_table(),
+            RequiredTable::Notes
+        );
+        assert_eq!(
+            Node::Group(parse("cid:1").unwrap()).required_table(),
+            RequiredTable::Cards
+        );
+        assert_eq!(
+            Node::Group(parse("cid:1 test").unwrap()).required_table(),
+            RequiredTable::CardsAndNotes
+        );
+        assert_eq!(
+            Node::Group(parse("nid:1").unwrap()).required_table(),
+            RequiredTable::CardsOrNotes
+        );
+        assert_eq!(
+            Node::Group(parse("cid:1 nid:1").unwrap()).required_table(),
+            RequiredTable::Cards
+        );
+        assert_eq!(
+            Node::Group(parse("test nid:1").unwrap()).required_table(),
+            RequiredTable::Notes
+        );
+    }
 }