From ee3a1c211fd1c1ddc513f0307fa374dbb551542b Mon Sep 17 00:00:00 2001 From: Emmanuel Keller Date: Thu, 13 Jul 2023 11:12:34 +0100 Subject: [PATCH] Select/Explain should only return the explanation (#2256) --- lib/src/dbs/iterator.rs | 132 ++++++++++++++++------------------ lib/tests/matches.rs | 110 +++++++++++++--------------- lib/tests/select.rs | 156 +++++++++++++++++++++------------------- 3 files changed, 197 insertions(+), 201 deletions(-) diff --git a/lib/src/dbs/iterator.rs b/lib/src/dbs/iterator.rs index 522e1d81..6266cdcf 100644 --- a/lib/src/dbs/iterator.rs +++ b/lib/src/dbs/iterator.rs @@ -89,28 +89,25 @@ impl Iterator { // Process the query START clause self.setup_start(&cancel_ctx, opt, txn, stm).await?; // Process any EXPLAIN clause - let explanation = self.output_explain(&cancel_ctx, opt, txn, stm)?; - // Process prepared values - self.iterate(&cancel_ctx, opt, txn, stm).await?; - // Return any document errors - if let Some(e) = self.error.take() { - return Err(e); - } - // Process any SPLIT clause - self.output_split(ctx, opt, txn, stm).await?; - // Process any GROUP clause - self.output_group(ctx, opt, txn, stm).await?; - // Process any ORDER clause - self.output_order(ctx, opt, txn, stm).await?; - // Process any START clause - self.output_start(ctx, opt, txn, stm).await?; - // Process any LIMIT clause - self.output_limit(ctx, opt, txn, stm).await?; - // Process any FETCH clause - self.output_fetch(ctx, opt, txn, stm).await?; - // Add the EXPLAIN clause to the result - if let Some(e) = explanation { - self.results.push(e); + if !self.output_explain(&cancel_ctx, opt, txn, stm)? { + // Process prepared values + self.iterate(&cancel_ctx, opt, txn, stm).await?; + // Return any document errors + if let Some(e) = self.error.take() { + return Err(e); + } + // Process any SPLIT clause + self.output_split(ctx, opt, txn, stm).await?; + // Process any GROUP clause + self.output_group(ctx, opt, txn, stm).await?; + // Process any ORDER clause + self.output_order(ctx, opt, txn, stm).await?; + // Process any START clause + self.output_start(ctx, opt, txn, stm).await?; + // Process any LIMIT clause + self.output_limit(ctx, opt, txn, stm).await?; + // Process any FETCH clause + self.output_fetch(ctx, opt, txn, stm).await?; } // Output the results Ok(mem::take(&mut self.results).into()) @@ -362,54 +359,49 @@ impl Iterator { _opt: &Options, _txn: &Transaction, stm: &Statement<'_>, - ) -> Result, Error> { - Ok(if stm.explain() { - let mut explains = Vec::with_capacity(self.entries.len()); - for iter in &self.entries { - let (operation, detail) = match iter { - Iterable::Value(v) => ("Iterate Value", vec![("value", v.to_owned())]), - Iterable::Table(t) => { - ("Iterate Table", vec![("table", Value::from(t.0.to_owned()))]) - } - Iterable::Thing(t) => { - ("Iterate Thing", vec![("thing", Value::Thing(t.to_owned()))]) - } - Iterable::Range(r) => { - ("Iterate Range", vec![("table", Value::from(r.tb.to_owned()))]) - } - Iterable::Edges(e) => { - ("Iterate Edges", vec![("from", Value::Thing(e.from.to_owned()))]) - } - Iterable::Mergeable(t, v) => ( - "Iterate Mergeable", - vec![("thing", Value::Thing(t.to_owned())), ("value", v.to_owned())], - ), - Iterable::Relatable(t1, t2, t3) => ( - "Iterate Relatable", - vec![ - ("thing-1", Value::Thing(t1.to_owned())), - ("thing-2", Value::Thing(t2.to_owned())), - ("thing-3", Value::Thing(t3.to_owned())), - ], - ), - Iterable::Index(t, p) => ( - "Iterate Index", - vec![("table", Value::from(t.0.to_owned())), ("plan", p.explain())], - ), - }; - let explain = Object::from(HashMap::from([ - ("operation", Value::from(operation)), - ("detail", Value::Object(Object::from(HashMap::from_iter(detail)))), - ])); - explains.push(Value::Object(explain)); - } - Some(Value::Object(Object::from(HashMap::from([( - "explain", - Value::Array(Array::from(explains)), - )])))) - } else { - None - }) + ) -> Result { + if !stm.explain() { + return Ok(false); + } + for iter in &self.entries { + let (operation, detail) = match iter { + Iterable::Value(v) => ("Iterate Value", vec![("value", v.to_owned())]), + Iterable::Table(t) => { + ("Iterate Table", vec![("table", Value::from(t.0.to_owned()))]) + } + Iterable::Thing(t) => { + ("Iterate Thing", vec![("thing", Value::Thing(t.to_owned()))]) + } + Iterable::Range(r) => { + ("Iterate Range", vec![("table", Value::from(r.tb.to_owned()))]) + } + Iterable::Edges(e) => { + ("Iterate Edges", vec![("from", Value::Thing(e.from.to_owned()))]) + } + Iterable::Mergeable(t, v) => ( + "Iterate Mergeable", + vec![("thing", Value::Thing(t.to_owned())), ("value", v.to_owned())], + ), + Iterable::Relatable(t1, t2, t3) => ( + "Iterate Relatable", + vec![ + ("thing-1", Value::Thing(t1.to_owned())), + ("thing-2", Value::Thing(t2.to_owned())), + ("thing-3", Value::Thing(t3.to_owned())), + ], + ), + Iterable::Index(t, p) => ( + "Iterate Index", + vec![("table", Value::from(t.0.to_owned())), ("plan", p.explain())], + ), + }; + let explain = Object::from(HashMap::from([ + ("operation", Value::from(operation)), + ("detail", Value::Object(Object::from(HashMap::from_iter(detail)))), + ])); + self.results.push(Value::Object(explain)); + } + Ok(true) } #[cfg(target_arch = "wasm32")] diff --git a/lib/tests/matches.rs b/lib/tests/matches.rs index 7578ed5b..d6822260 100644 --- a/lib/tests/matches.rs +++ b/lib/tests/matches.rs @@ -11,12 +11,13 @@ async fn select_where_matches_using_index() -> Result<(), Error> { CREATE blog:1 SET title = 'Hello World!'; DEFINE ANALYZER simple TOKENIZERS blank,class; DEFINE INDEX blog_title ON blog FIELDS title SEARCH ANALYZER simple BM25 HIGHLIGHTS; - SELECT id, search::highlight('', '', 1) AS title FROM blog WHERE title @1@ 'Hello' EXPLAIN; + SELECT id FROM blog WHERE title @1@ 'Hello' EXPLAIN; + SELECT id, search::highlight('', '', 1) AS title FROM blog WHERE title @1@ 'Hello'; "; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 4); + assert_eq!(res.len(), 5); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; @@ -24,13 +25,6 @@ async fn select_where_matches_using_index() -> Result<(), Error> { let tmp = res.remove(0).result?; let val = Value::parse( "[ - { - id: blog:1, - title: 'Hello World!' - }, - { - explain: - [ { detail: { plan: { @@ -42,7 +36,15 @@ async fn select_where_matches_using_index() -> Result<(), Error> { }, operation: 'Iterate Index' } - ] + ]", + ); + assert_eq!(tmp, val); + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + id: blog:1, + title: 'Hello World!' } ]", ); @@ -57,34 +59,36 @@ async fn select_where_matches_without_using_index_iterator() -> Result<(), Error CREATE blog:2 SET title = 'Foo Bar!'; DEFINE ANALYZER simple TOKENIZERS blank,class FILTERS lowercase; DEFINE INDEX blog_title ON blog FIELDS title SEARCH ANALYZER simple BM25(1.2,0.75) HIGHLIGHTS; - SELECT id,search::highlight('', '', 1) AS title FROM blog WHERE (title @0@ 'hello' AND identifier > 0) OR (title @1@ 'world' AND identifier < 99) EXPLAIN; + SELECT id FROM blog WHERE (title @0@ 'hello' AND identifier > 0) OR (title @1@ 'world' AND identifier < 99) EXPLAIN; + SELECT id,search::highlight('', '', 1) AS title FROM blog WHERE (title @0@ 'hello' AND identifier > 0) OR (title @1@ 'world' AND identifier < 99); "; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 5); + assert_eq!(res.len(), 6); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; let _ = res.remove(0).result?; let _ = res.remove(0).result?; let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + detail: { + table: 'blog', + }, + operation: 'Iterate Table' + } + ]", + ); + assert_eq!(tmp, val); + let tmp = res.remove(0).result?; let val = Value::parse( "[ { id: blog:1, title: 'Hello World!' - }, - { - explain: - [ - { - detail: { - table: 'blog', - }, - operation: 'Iterate Table' - } - ] } ]", ); @@ -93,41 +97,32 @@ async fn select_where_matches_without_using_index_iterator() -> Result<(), Error } async fn select_where_matches_using_index_and_arrays(parallel: bool) -> Result<(), Error> { + let p = if parallel { + "PARALLEL" + } else { + "" + }; let sql = format!( r" CREATE blog:1 SET content = ['Hello World!', 'Be Bop', 'Foo Bãr']; DEFINE ANALYZER simple TOKENIZERS blank,class; DEFINE INDEX blog_content ON blog FIELDS content SEARCH ANALYZER simple BM25 HIGHLIGHTS; - SELECT id, search::highlight('', '', 1) AS content FROM blog WHERE content @1@ 'Hello Bãr' {} EXPLAIN; - ", - if parallel { - "PARALLEL" - } else { - "" - } + SELECT id FROM blog WHERE content @1@ 'Hello Bãr' {p} EXPLAIN; + SELECT id, search::highlight('', '', 1) AS content FROM blog WHERE content @1@ 'Hello Bãr' {p}; + " ); let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(&sql, &ses, None).await?; - assert_eq!(res.len(), 4); + assert_eq!(res.len(), 5); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; let _ = res.remove(0).result?; + // let tmp = res.remove(0).result?; let val = Value::parse( "[ - { - id: blog:1, - content: [ - 'Hello World!', - 'Be Bop', - 'Foo Bãr' - ] - }, - { - explain: - [ { detail: { plan: { @@ -139,6 +134,19 @@ async fn select_where_matches_using_index_and_arrays(parallel: bool) -> Result<( }, operation: 'Iterate Index' } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + id: blog:1, + content: [ + 'Hello World!', + 'Be Bop', + 'Foo Bãr' ] } ]", @@ -164,7 +172,7 @@ async fn select_where_matches_using_index_offsets() -> Result<(), Error> { DEFINE ANALYZER simple TOKENIZERS blank,class; DEFINE INDEX blog_title ON blog FIELDS title SEARCH ANALYZER simple BM25(1.2,0.75) HIGHLIGHTS; DEFINE INDEX blog_content ON blog FIELDS content SEARCH ANALYZER simple BM25 HIGHLIGHTS; - SELECT id, search::offsets(0) AS title, search::offsets(1) AS content FROM blog WHERE title @0@ 'title' AND content @1@ 'Hello Bãr' EXPLAIN; + SELECT id, search::offsets(0) AS title, search::offsets(1) AS content FROM blog WHERE title @0@ 'title' AND content @1@ 'Hello Bãr'; "; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); @@ -186,22 +194,6 @@ async fn select_where_matches_using_index_offsets() -> Result<(), Error> { 0: [{s:0, e:5}], 2: [{s:4, e:7}] } - }, - { - explain: - [ - { - detail: { - plan: { - index: 'blog_content', - operator: '@1@', - value: 'Hello Bãr' - }, - table: 'blog', - }, - operation: 'Iterate Index' - } - ] } ]", ); diff --git a/lib/tests/select.rs b/lib/tests/select.rs index 67f24168..d22f080a 100644 --- a/lib/tests/select.rs +++ b/lib/tests/select.rs @@ -266,11 +266,12 @@ async fn select_where_field_is_thing_and_with_index() -> Result<(), Error> { DEFINE INDEX author ON TABLE post COLUMNS author; CREATE post:1 SET author = person:tobie; CREATE post:2 SET author = person:tobie; - SELECT * FROM post WHERE author = person:tobie EXPLAIN;"; + SELECT * FROM post WHERE author = person:tobie EXPLAIN; + SELECT * FROM post WHERE author = person:tobie;"; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 5); + assert_eq!(res.len(), 6); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; @@ -278,6 +279,24 @@ async fn select_where_field_is_thing_and_with_index() -> Result<(), Error> { let _ = res.remove(0).result?; // let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + detail: { + plan: { + index: 'author', + operator: '=', + value: person:tobie + }, + table: 'post', + }, + operation: 'Iterate Index' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; let val = Value::parse( "[ { @@ -287,22 +306,6 @@ async fn select_where_field_is_thing_and_with_index() -> Result<(), Error> { { author: person:tobie, id: post:2 - }, - { - explain: - [ - { - detail: { - plan: { - index: 'author', - operator: '=', - value: person:tobie - }, - table: 'post', - }, - operation: 'Iterate Index' - } - ] } ]", ); @@ -316,37 +319,40 @@ async fn select_where_and_with_index() -> Result<(), Error> { CREATE person:tobie SET name = 'Tobie', genre='m'; CREATE person:jaime SET name = 'Jaime', genre='m'; DEFINE INDEX person_name ON TABLE person COLUMNS name; - SELECT name FROM person WHERE name = 'Tobie' AND genre = 'm' EXPLAIN;"; + SELECT name FROM person WHERE name = 'Tobie' AND genre = 'm' EXPLAIN; + SELECT name FROM person WHERE name = 'Tobie' AND genre = 'm';"; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 4); + assert_eq!(res.len(), 5); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; let _ = res.remove(0).result?; // let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + detail: { + plan: { + index: 'person_name', + operator: '=', + value: 'Tobie' + }, + table: 'person', + }, + operation: 'Iterate Index' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; let val = Value::parse( "[ { name: 'Tobie' - }, - { - explain: - [ - { - detail: { - plan: { - index: 'person_name', - operator: '=', - value: 'Tobie' - }, - table: 'person', - }, - operation: 'Iterate Index' - } - ] } ]", ); @@ -360,37 +366,40 @@ async fn select_where_and_with_unique_index() -> Result<(), Error> { CREATE person:tobie SET name = 'Tobie', genre='m'; CREATE person:jaime SET name = 'Jaime', genre='m'; DEFINE INDEX person_name ON TABLE person COLUMNS name UNIQUE; - SELECT name FROM person WHERE name = 'Jaime' AND genre = 'm' EXPLAIN;"; + SELECT name FROM person WHERE name = 'Jaime' AND genre = 'm' EXPLAIN; + SELECT name FROM person WHERE name = 'Jaime' AND genre = 'm';"; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 4); + assert_eq!(res.len(), 5); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; let _ = res.remove(0).result?; // let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + detail: { + plan: { + index: 'person_name', + operator: '=', + value: 'Jaime' + }, + table: 'person', + }, + operation: 'Iterate Index' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; let val = Value::parse( "[ { name: 'Jaime' - }, - { - explain: - [ - { - detail: { - plan: { - index: 'person_name', - operator: '=', - value: 'Jaime' - }, - table: 'person', - }, - operation: 'Iterate Index' - } - ] } ]", ); @@ -405,11 +414,12 @@ async fn select_where_and_with_fulltext_index() -> Result<(), Error> { CREATE person:jaime SET name = 'Jaime', genre='m'; DEFINE ANALYZER simple TOKENIZERS blank,class FILTERS lowercase; DEFINE INDEX ft_name ON TABLE person COLUMNS name SEARCH ANALYZER simple BM25(1.2,0.75); - SELECT name FROM person WHERE name @@ 'Jaime' AND genre = 'm' EXPLAIN;"; + SELECT name FROM person WHERE name @@ 'Jaime' AND genre = 'm' EXPLAIN; + SELECT name FROM person WHERE name @@ 'Jaime' AND genre = 'm';"; let dbs = Datastore::new("memory").await?; let ses = Session::for_kv().with_ns("test").with_db("test"); let res = &mut dbs.execute(sql, &ses, None).await?; - assert_eq!(res.len(), 5); + assert_eq!(res.len(), 6); // let _ = res.remove(0).result?; let _ = res.remove(0).result?; @@ -417,26 +427,28 @@ async fn select_where_and_with_fulltext_index() -> Result<(), Error> { let _ = res.remove(0).result?; // let tmp = res.remove(0).result?; + let val = Value::parse( + "[ + { + detail: { + plan: { + index: 'ft_name', + operator: '@@', + value: 'Jaime' + }, + table: 'person', + }, + operation: 'Iterate Index' + } + ]", + ); + assert_eq!(tmp, val); + // + let tmp = res.remove(0).result?; let val = Value::parse( "[ { name: 'Jaime' - }, - { - explain: - [ - { - detail: { - plan: { - index: 'ft_name', - operator: '@@', - value: 'Jaime' - }, - table: 'person', - }, - operation: 'Iterate Index' - } - ] } ]", );