From 08f1843b67fb0ae6341aced6f422df04f035129c Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 19 May 2020 11:27:02 +1000 Subject: [PATCH] automatically omit notes or cards table when possible --- rslib/src/findreplace.rs | 2 +- rslib/src/notetype/schemachange.rs | 4 +- rslib/src/search/cards.rs | 74 ++++++++++--- rslib/src/search/notes.rs | 27 +---- rslib/src/search/sqlwriter.rs | 172 ++++++++++++++++++++++++++--- 5 files changed, 222 insertions(+), 57 deletions(-) diff --git a/rslib/src/findreplace.rs b/rslib/src/findreplace.rs index 52b8bde65..32368cc19 100644 --- a/rslib/src/findreplace.rs +++ b/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); diff --git a/rslib/src/notetype/schemachange.rs b/rslib/src/notetype/schemachange.rs index 096693579..4f87c280c 100644 --- a/rslib/src/notetype/schemachange.rs +++ b/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(); diff --git a/rslib/src/search/cards.rs b/rslib/src/search/cards.rs index aac1ca81c..e4afa8ea2 100644 --- a/rslib/src/search/cards.rs +++ b/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 { - pub fn search_cards(&mut self, search: &str, mode: SortMode) -> Result> { - let top_node = Node::Group(parse(search)?); - let (sql, args) = node_to_sql(self, &top_node, self.normalize_note_text())?; +impl SortMode { + fn required_table(&self) -> RequiredTable { + match self { + 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!( - "select c.id from cards c, notes n where c.nid=n.id and {}", - sql - ); +impl SortKind { + fn required_table(self) -> RequiredTable { + 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> { + 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 => { - 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::FromConfig => unreachable!(), 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. diff --git a/rslib/src/search/notes.rs b/rslib/src/search/notes.rs index 86d408e10..2f486176c 100644 --- a/rslib/src/search/notes.rs +++ b/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> { - 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> { - self.search_notes_inner(search, |sql| { - format!("select n.id from notes n where {}", sql) - }) - } - - fn search_notes_inner(&mut self, search: &str, build_sql: F) -> Result> - 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 sql = build_sql(sql); + let writer = SqlWriter::new(self); + let (sql, args) = writer.build_notes_query(&top_node)?; let mut stmt = self.storage.db.prepare(&sql)?; let ids: Vec<_> = stmt diff --git a/rslib/src/search/sqlwriter.rs b/rslib/src/search/sqlwriter.rs index 348627b6a..c287bc6d7 100644 --- a/rslib/src/search/sqlwriter.rs +++ b/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, normalize_note_text: bool, -} - -pub(super) fn node_to_sql( - req: &mut Collection, - node: &Node, - normalize_note_text: bool, -) -> Result<(String, Vec)> { - let mut sctx = SqlWriter::new(req, normalize_note_text); - sctx.write_node_to_sql(&node)?; - Ok((sctx.sql, sctx.args)) + table: RequiredTable, } 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)> { + 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)> { + 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) { 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 + ); + } }