From fe78ca3c322ada69aecfab0f2fa2bb9796257c58 Mon Sep 17 00:00:00 2001 From: Emmanuel Keller Date: Fri, 8 Sep 2023 00:36:39 +0100 Subject: [PATCH] Feature: Add fallback reason to the explanation (#2647) --- lib/src/dbs/explanation.rs | 23 ++++++++- lib/src/dbs/iterator.rs | 2 +- lib/src/doc/index.rs | 2 +- lib/src/err/mod.rs | 2 +- lib/src/fnc/string.rs | 6 +-- lib/src/fnc/vector.rs | 4 +- lib/src/idx/planner/executor.rs | 2 +- lib/src/idx/planner/mod.rs | 49 +++++++++++------- lib/src/idx/planner/plan.rs | 32 +++++------- lib/src/idx/planner/tree.rs | 36 ++++++------- lib/src/sql/statements/analyze.rs | 2 +- lib/tests/planner.rs | 86 ++++++++++++++++++++++++++++++- 12 files changed, 175 insertions(+), 71 deletions(-) diff --git a/lib/src/dbs/explanation.rs b/lib/src/dbs/explanation.rs index 84d43859..0541f907 100644 --- a/lib/src/dbs/explanation.rs +++ b/lib/src/dbs/explanation.rs @@ -1,3 +1,4 @@ +use crate::ctx::Context; use crate::dbs::Iterable; use crate::sql::{Explain, Object, Value}; use std::collections::HashMap; @@ -6,7 +7,11 @@ use std::collections::HashMap; pub(super) struct Explanation(Vec); impl Explanation { - pub(super) fn new(e: Option<&Explain>, iterables: &Vec) -> (bool, Option) { + pub(super) fn new( + ctx: &Context<'_>, + e: Option<&Explain>, + iterables: &Vec, + ) -> (bool, Option) { match e { None => (true, None), Some(e) => { @@ -14,6 +19,11 @@ impl Explanation { for i in iterables { exp.add_iter(i); } + if let Some(qp) = ctx.get_query_planner() { + for reason in qp.fallbacks() { + exp.add_fallback(reason.to_string()); + } + } (e.0, Some(exp)) } } @@ -27,6 +37,10 @@ impl Explanation { self.0.push(ExplainItem::new_fetch(count)); } + fn add_fallback(&mut self, reason: String) { + self.0.push(ExplainItem::new_fallback(reason)); + } + pub(super) fn output(self, results: &mut Vec) { for e in self.0 { results.push(e.into()); @@ -47,6 +61,13 @@ impl ExplainItem { } } + fn new_fallback(reason: String) -> Self { + Self { + name: "Fallback".into(), + details: vec![("reason", reason.into())], + } + } + fn new_iter(iter: &Iterable) -> Self { match iter { Iterable::Value(v) => Self { diff --git a/lib/src/dbs/iterator.rs b/lib/src/dbs/iterator.rs index 53e59979..b5c4c0ba 100644 --- a/lib/src/dbs/iterator.rs +++ b/lib/src/dbs/iterator.rs @@ -278,7 +278,7 @@ impl Iterator { // Process the query START clause self.setup_start(&cancel_ctx, opt, txn, stm).await?; // Extract the expected behaviour depending on the presence of EXPLAIN with or without FULL - let (do_iterate, mut explanation) = Explanation::new(stm.explain(), &self.entries); + let (do_iterate, mut explanation) = Explanation::new(ctx, stm.explain(), &self.entries); if do_iterate { // Process prepared values diff --git a/lib/src/doc/index.rs b/lib/src/doc/index.rs index 63270d52..a774cfba 100644 --- a/lib/src/doc/index.rs +++ b/lib/src/doc/index.rs @@ -57,7 +57,7 @@ impl<'a> Document<'a> { Index::Search(p) => ic.index_full_text(&mut run, p).await?, Index::MTree(_) => { return Err(Error::FeatureNotYetImplemented { - feature: "MTree indexing", + feature: "MTree indexing".to_string(), }) } }; diff --git a/lib/src/err/mod.rs b/lib/src/err/mod.rs index 9c5346a2..f9a59d00 100644 --- a/lib/src/err/mod.rs +++ b/lib/src/err/mod.rs @@ -606,7 +606,7 @@ pub enum Error { /// The feature has not yet being implemented #[error("Feature not yet implemented: {feature}")] FeatureNotYetImplemented { - feature: &'static str, + feature: String, }, /// Duplicated match references are not allowed diff --git a/lib/src/fnc/string.rs b/lib/src/fnc/string.rs index 62339852..6d65d9a7 100644 --- a/lib/src/fnc/string.rs +++ b/lib/src/fnc/string.rs @@ -140,13 +140,13 @@ pub mod distance { pub fn hamming((_, _): (String, String)) -> Result { Err(Error::FeatureNotYetImplemented { - feature: "string::distance::hamming() function", + feature: "string::distance::hamming() function".to_string(), }) } pub fn levenshtein((_, _): (String, String)) -> Result { Err(Error::FeatureNotYetImplemented { - feature: "string::distance::levenshtein() function", + feature: "string::distance::levenshtein() function".to_string(), }) } } @@ -234,7 +234,7 @@ pub mod similarity { pub fn jaro((_, _): (String, String)) -> Result { Err(Error::FeatureNotYetImplemented { - feature: "string::similarity::jaro() function", + feature: "string::similarity::jaro() function".to_string(), }) } diff --git a/lib/src/fnc/vector.rs b/lib/src/fnc/vector.rs index 2bcec9f6..9c8e9c3e 100644 --- a/lib/src/fnc/vector.rs +++ b/lib/src/fnc/vector.rs @@ -66,7 +66,7 @@ pub mod distance { pub fn mahalanobis((_, _): (Vec, Vec)) -> Result { Err(Error::FeatureNotYetImplemented { - feature: "vector::distance::mahalanobis() function", + feature: "vector::distance::mahalanobis() function".to_string(), }) } @@ -99,7 +99,7 @@ pub mod similarity { pub fn spearman((_, _): (Vec, Vec)) -> Result { Err(Error::FeatureNotYetImplemented { - feature: "vector::similarity::spearman() function", + feature: "vector::similarity::spearman() function".to_string(), }) } } diff --git a/lib/src/idx/planner/executor.rs b/lib/src/idx/planner/executor.rs index 4db95fa3..834736bd 100644 --- a/lib/src/idx/planner/executor.rs +++ b/lib/src/idx/planner/executor.rs @@ -122,7 +122,7 @@ impl QueryExecutor { .. } => self.new_search_index_iterator(ir, io).await, _ => Err(Error::FeatureNotYetImplemented { - feature: "VectorSearch iterator", + feature: "VectorSearch iterator".to_string(), }), } } diff --git a/lib/src/idx/planner/mod.rs b/lib/src/idx/planner/mod.rs index 01553036..f610a819 100644 --- a/lib/src/idx/planner/mod.rs +++ b/lib/src/idx/planner/mod.rs @@ -20,6 +20,7 @@ pub(crate) struct QueryPlanner<'a> { /// There is one executor per table executors: HashMap, requires_distinct: bool, + fallbacks: Vec, } impl<'a> QueryPlanner<'a> { @@ -30,6 +31,7 @@ impl<'a> QueryPlanner<'a> { cond, executors: HashMap::default(), requires_distinct: false, + fallbacks: vec![], } } @@ -40,31 +42,36 @@ impl<'a> QueryPlanner<'a> { t: Table, it: &mut Iterator, ) -> Result<(), Error> { - let res = Tree::build(ctx, self.opt, txn, &t, self.cond).await?; - if let Some((node, im)) = res { - let mut exe = QueryExecutor::new(self.opt, txn, &t, im).await?; - let ok = match PlanBuilder::build(node, self.with)? { - Plan::SingleIndex(exp, io) => { - let ir = exe.add_iterator(exp); - it.ingest(Iterable::Index(t.clone(), ir, io)); - true - } - Plan::MultiIndex(v) => { - for (exp, io) in v { + match Tree::build(ctx, self.opt, txn, &t, self.cond).await? { + Some((node, im)) => { + let mut exe = QueryExecutor::new(self.opt, txn, &t, im).await?; + match PlanBuilder::build(node, self.with)? { + Plan::SingleIndex(exp, io) => { let ir = exe.add_iterator(exp); it.ingest(Iterable::Index(t.clone(), ir, io)); - self.requires_distinct = true; + self.executors.insert(t.0.clone(), exe); + } + Plan::MultiIndex(v) => { + for (exp, io) in v { + let ir = exe.add_iterator(exp); + it.ingest(Iterable::Index(t.clone(), ir, io)); + self.requires_distinct = true; + } + self.executors.insert(t.0.clone(), exe); + } + Plan::TableIterator(fallback) => { + if let Some(fallback) = fallback { + self.fallbacks.push(fallback); + } + self.executors.insert(t.0.clone(), exe); + it.ingest(Iterable::Table(t)); } - true } - Plan::TableIterator => false, - }; - self.executors.insert(t.0.clone(), exe); - if ok { - return Ok(()); + } + None => { + it.ingest(Iterable::Table(t)); } } - it.ingest(Iterable::Table(t)); Ok(()) } @@ -79,4 +86,8 @@ impl<'a> QueryPlanner<'a> { pub(crate) fn requires_distinct(&self) -> bool { self.requires_distinct } + + pub(crate) fn fallbacks(&self) -> &Vec { + &self.fallbacks + } } diff --git a/lib/src/idx/planner/plan.rs b/lib/src/idx/planner/plan.rs index 4b7d67ae..e1e08539 100644 --- a/lib/src/idx/planner/plan.rs +++ b/lib/src/idx/planner/plan.rs @@ -20,7 +20,7 @@ impl<'a> PlanBuilder<'a> { pub(super) fn build(root: Node, with: &'a Option) -> Result { if let Some(with) = with { if matches!(with, With::NoIndex) { - return Ok(Plan::TableIterator); + return Ok(Plan::TableIterator(Some("WITH NOINDEX".to_string()))); } } let mut b = PlanBuilder { @@ -30,12 +30,12 @@ impl<'a> PlanBuilder<'a> { all_exp_with_index: true, }; // Browse the AST and collect information - if !b.eval_node(root)? { - return Ok(Plan::TableIterator); + if let Err(e) = b.eval_node(root) { + return Ok(Plan::TableIterator(Some(e.to_string()))); } // If we didn't found any index, we're done with no index plan if b.indexes.is_empty() { - return Ok(Plan::TableIterator); + return Ok(Plan::TableIterator(Some("NO INDEX FOUND".to_string()))); } // If every boolean operator are AND then we can use the single index plan if b.all_and { @@ -47,7 +47,7 @@ impl<'a> PlanBuilder<'a> { if b.all_exp_with_index { return Ok(Plan::MultiIndex(b.indexes)); } - Ok(Plan::TableIterator) + Ok(Plan::TableIterator(None)) } // Check if we have an explicit list of index we can use @@ -62,7 +62,7 @@ impl<'a> PlanBuilder<'a> { io } - fn eval_node(&mut self, node: Node) -> Result { + fn eval_node(&mut self, node: Node) -> Result<(), String> { match node { Node::Expression { io, @@ -79,10 +79,12 @@ impl<'a> PlanBuilder<'a> { } else if self.all_exp_with_index && !is_bool { self.all_exp_with_index = false; } - self.eval_expression(*left, *right) + self.eval_node(*left)?; + self.eval_node(*right)?; + Ok(()) } - Node::Unsupported => Ok(false), - _ => Ok(true), + Node::Unsupported(reason) => Err(reason), + _ => Ok(()), } } @@ -99,23 +101,13 @@ impl<'a> PlanBuilder<'a> { } } - fn eval_expression(&mut self, left: Node, right: Node) -> Result { - if !self.eval_node(left)? { - return Ok(false); - } - if !self.eval_node(right)? { - return Ok(false); - } - Ok(true) - } - fn add_index_option(&mut self, e: Expression, i: IndexOption) { self.indexes.push((e, i)); } } pub(super) enum Plan { - TableIterator, + TableIterator(Option), SingleIndex(Expression, IndexOption), MultiIndex(Vec<(Expression, IndexOption)>), } diff --git a/lib/src/idx/planner/tree.rs b/lib/src/idx/planner/tree.rs index b0b27230..7b1cd50f 100644 --- a/lib/src/idx/planner/tree.rs +++ b/lib/src/idx/planner/tree.rs @@ -71,20 +71,20 @@ impl<'a> TreeBuilder<'a> { #[cfg_attr(not(target_arch = "wasm32"), async_recursion)] #[cfg_attr(target_arch = "wasm32", async_recursion(?Send))] async fn eval_value(&mut self, v: &Value) -> Result { - Ok(match v { - Value::Expression(e) => self.eval_expression(e).await?, - Value::Idiom(i) => self.eval_idiom(i).await?, - Value::Strand(_) => Node::Scalar(v.to_owned()), - Value::Number(_) => Node::Scalar(v.to_owned()), - Value::Bool(_) => Node::Scalar(v.to_owned()), - Value::Thing(_) => Node::Scalar(v.to_owned()), - Value::Subquery(s) => self.eval_subquery(s).await?, + match v { + Value::Expression(e) => self.eval_expression(e).await, + Value::Idiom(i) => self.eval_idiom(i).await, + Value::Strand(_) => Ok(Node::Scalar(v.to_owned())), + Value::Number(_) => Ok(Node::Scalar(v.to_owned())), + Value::Bool(_) => Ok(Node::Scalar(v.to_owned())), + Value::Thing(_) => Ok(Node::Scalar(v.to_owned())), + Value::Subquery(s) => self.eval_subquery(s).await, Value::Param(p) => { let v = p.compute(self.ctx, self.opt, self.txn, None).await?; - self.eval_value(&v).await? + self.eval_value(&v).await } - _ => Node::Unsupported, - }) + _ => Ok(Node::Unsupported(format!("Unsupported value: {}", v))), + } } async fn eval_idiom(&mut self, i: &Idiom) -> Result { @@ -99,9 +99,7 @@ impl<'a> TreeBuilder<'a> { match e { Expression::Unary { .. - } => Err(Error::FeatureNotYetImplemented { - feature: "unary expressions in index", - }), + } => Ok(Node::Unsupported("unary expressions not supported".to_string())), Expression::Binary { l, o, @@ -173,10 +171,10 @@ impl<'a> TreeBuilder<'a> { } async fn eval_subquery(&mut self, s: &Subquery) -> Result { - Ok(match s { - Subquery::Value(v) => self.eval_value(v).await?, - _ => Node::Unsupported, - }) + match s { + Subquery::Value(v) => self.eval_value(v).await, + _ => Ok(Node::Unsupported(format!("Unsupported subquery: {}", s))), + } } } @@ -201,7 +199,7 @@ pub(super) enum Node { IndexedField(Idiom, DefineIndexStatement), NonIndexedField, Scalar(Value), - Unsupported, + Unsupported(String), } impl Node { diff --git a/lib/src/sql/statements/analyze.rs b/lib/src/sql/statements/analyze.rs index 8764f60d..9ec66ec1 100644 --- a/lib/src/sql/statements/analyze.rs +++ b/lib/src/sql/statements/analyze.rs @@ -58,7 +58,7 @@ impl AnalyzeStatement { } _ => { return Err(Error::FeatureNotYetImplemented { - feature: "Statistics on unique and non-unique indexes.", + feature: "Statistics on unique and non-unique indexes.".to_string(), }) } }; diff --git a/lib/tests/planner.rs b/lib/tests/planner.rs index 211d129a..8b0b2f77 100644 --- a/lib/tests/planner.rs +++ b/lib/tests/planner.rs @@ -110,10 +110,10 @@ async fn select_where_iterate_two_no_index() -> Result<(), Error> { let mut res = execute_test(&two_multi_index_query("WITH NOINDEX", ""), 9).await?; // OR results check_result(&mut res, "[{ name: 'Jaime' }, { name: 'Tobie' }]")?; - check_result(&mut res, &table_explain(2))?; + check_result(&mut res, &table_explain_no_index(2))?; // AND results check_result(&mut res, "[{name: 'Jaime'}]")?; - check_result(&mut res, &table_explain(1))?; + check_result(&mut res, &table_explain_no_index(1))?; Ok(()) } @@ -185,6 +185,31 @@ fn table_explain(fetch_count: usize) -> String { ) } +fn table_explain_no_index(fetch_count: usize) -> String { + format!( + "[ + {{ + detail: {{ + table: 'person' + }}, + operation: 'Iterate Table' + }}, + {{ + detail: {{ + reason: 'WITH NOINDEX' + }}, + operation: 'Fallback' + }}, + {{ + detail: {{ + count: {fetch_count} + }}, + operation: 'Fetch' + }} + ]" + ) +} + const THREE_TABLE_EXPLAIN: &str = "[ { detail: { @@ -332,3 +357,60 @@ const TWO_MULTI_INDEX_EXPLAIN: &str = "[ operation: 'Fetch' } ]"; + +#[tokio::test] +async fn select_with_no_index_unary_operator() -> Result<(), Error> { + let dbs = new_ds().await?; + let ses = Session::owner().with_ns("test").with_db("test"); + let mut res = dbs + .execute("SELECT * FROM table WITH NOINDEX WHERE !param.subparam EXPLAIN", &ses, None) + .await?; + assert_eq!(res.len(), 1); + let tmp = res.remove(0).result?; + let val = Value::parse( + r#"[ + { + detail: { + table: 'table' + }, + operation: 'Iterate Table' + }, + { + detail: { + reason: 'WITH NOINDEX' + }, + operation: 'Fallback' + } + ]"#, + ); + assert_eq!(format!("{:#}", tmp), format!("{:#}", val)); + Ok(()) +} + +#[tokio::test] +async fn select_unsupported_unary_operator() -> Result<(), Error> { + let dbs = new_ds().await?; + let ses = Session::owner().with_ns("test").with_db("test"); + let mut res = + dbs.execute("SELECT * FROM table WHERE !param.subparam EXPLAIN", &ses, None).await?; + assert_eq!(res.len(), 1); + let tmp = res.remove(0).result?; + let val = Value::parse( + r#"[ + { + detail: { + table: 'table' + }, + operation: 'Iterate Table' + }, + { + detail: { + reason: 'unary expressions not supported' + }, + operation: 'Fallback' + } + ]"#, + ); + assert_eq!(format!("{:#}", tmp), format!("{:#}", val)); + Ok(()) +}