From 729fe59578e4be7752b5d77a7c7e1ea07c523275 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Tue, 29 May 2018 14:01:46 -0700 Subject: [PATCH 01/13] [edn] Pre: Rename keyword to namespaced_keyword. The `Keyword` type evolved to become more general: we now use the one type for both :regular and :name/spaced keywords. This changes reflects the new generality. --- edn/src/edn.rustpeg | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/edn/src/edn.rustpeg b/edn/src/edn.rustpeg index da0e9905..c09ac224 100644 --- a/edn/src/edn.rustpeg +++ b/edn/src/edn.rustpeg @@ -203,25 +203,26 @@ pub op -> OpType = ":db/add" { OpType::Add } / ":db/retract" { OpType::Retract } -raw_keyword -> Keyword +raw_namespaced_keyword -> Keyword = keyword_prefix ns:$(symbol_namespace) namespace_separator n:$(symbol_name) { Keyword::namespaced(ns, n) } + / #expected("namespaced keyword") -raw_forward_keyword -> Keyword - = v:raw_keyword {? if v.is_forward() { Ok(v) } else { Err("expected :forward/keyword") } } +raw_forward_namespaced_keyword -> Keyword + = v:raw_namespaced_keyword {? if v.is_forward() { Ok(v) } else { Err("expected namespaced :forward/keyword") } } -raw_backward_keyword -> Keyword - = v:raw_keyword {? if v.is_backward() { Ok(v) } else { Err("expected :backward/_keyword") } } +raw_backward_namespaced_keyword -> Keyword + = v:raw_namespaced_keyword {? if v.is_backward() { Ok(v) } else { Err("expected namespaced :backward/_keyword") } } entid -> Entid = v:( raw_basedinteger / raw_hexinteger / raw_octalinteger / raw_integer ) { Entid::Entid(v) } - / v:raw_keyword { Entid::Ident(v) } + / v:raw_namespaced_keyword { Entid::Ident(v) } forward_entid -> Entid = v:( raw_basedinteger / raw_hexinteger / raw_octalinteger / raw_integer ) { Entid::Entid(v) } - / v:raw_forward_keyword { Entid::Ident(v) } + / v:raw_forward_namespaced_keyword { Entid::Ident(v) } backward_entid -> Entid - = v:raw_backward_keyword { Entid::Ident(v.to_reversed()) } + = v:raw_backward_namespaced_keyword { Entid::Ident(v.to_reversed()) } lookup_ref -> LookupRef = "(" __ "lookup-ref" __ a:(entid) __ v:(value) __ ")" { LookupRef { a: AttributePlace::Entid(a), v } } From 47a0f40cce53ad63941b94b74a4e4eea0b43545f Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 28 May 2018 14:33:25 -0700 Subject: [PATCH 02/13] Pre: Fix warnings. --- src/query_builder.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/query_builder.rs b/src/query_builder.rs index 5abeca60..bafc59e7 100644 --- a/src/query_builder.rs +++ b/src/query_builder.rs @@ -124,11 +124,6 @@ mod test { Store, }; - use mentat_core::{ - DateTime, - Utc, - }; - #[test] fn test_scalar_query() { let mut store = Store::open("").expect("store connection"); From 3cc8b4fd245b775944eb875dcf6e4cf40bdbf2c5 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 31 May 2018 11:33:03 -0700 Subject: [PATCH 03/13] Pre: Prefer [(pred ...)] to [[pred ...]] syntax. This is a style choice. We supported both, perhaps for Datomic compliance, but it's not the standard we use in our code base. In addition, it doesn't read like lisp (which is what EDN is copying), since [] is not function application in most lisps. It's also a convenience: I don't want to parse brackets that have to agree with `rust-peg`. It's not hard but it's also not worth doing. --- query-algebrizer/src/clauses/not.rs | 2 +- query-algebrizer/src/clauses/or.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 4d312023..43c0a901 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -338,7 +338,7 @@ mod testing { [:find ?x ?age :where [?x :foo/age ?age] - [[< ?age 30]] + [(< ?age 30)] (not [?x :foo/knows "John"] [?x :foo/knows "Daphne"])]"#; let cc = alg(&schema, query); diff --git a/query-algebrizer/src/clauses/or.rs b/query-algebrizer/src/clauses/or.rs index 39e8208f..e680a783 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -973,7 +973,7 @@ mod testing { [:find ?x ?age :where [?x :foo/age ?age] - [[< ?age 30]] + [(< ?age 30)] (or [?x :foo/knows "John"] [?x :foo/knows "Daphne"])]"#; let cc = alg(known, query); From f1fc9f184661dcb717fc57fa8c258e9a4f5cf2fd Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 31 May 2018 12:13:28 -0700 Subject: [PATCH 04/13] Part 0: Extract query-parser errors. --- query-parser/src/errors.rs | 49 ++++++++++++++++++++++++++++ query-parser/src/lib.rs | 6 +++- query-parser/src/parse.rs | 65 ++++---------------------------------- 3 files changed, 61 insertions(+), 59 deletions(-) create mode 100644 query-parser/src/errors.rs diff --git a/query-parser/src/errors.rs b/query-parser/src/errors.rs new file mode 100644 index 00000000..990112ba --- /dev/null +++ b/query-parser/src/errors.rs @@ -0,0 +1,49 @@ +// Copyright 2018 Mozilla +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. + +#![allow(dead_code)] + +use edn; + +use mentat_parser_utils::{ + ValueParseError, +}; + +error_chain! { + types { + Error, ErrorKind, ResultExt, Result; + } + + foreign_links { + EdnParseError(edn::parse::ParseError); + } + + errors { + DuplicateVariableError { + description("duplicate variable") + display("duplicates in variable list") + } + + FindParseError(e: ValueParseError) { + description(":find parse error") + display(":find parse error") + } + + UnknownLimitVar(var: edn::PlainSymbol) { + description("limit var not present in :in") + display("limit var {} not present in :in", var) + } + + InvalidLimit(val: edn::Value) { + description("limit value not valid") + display("expected natural number, got {}", val) + } + } +} diff --git a/query-parser/src/lib.rs b/query-parser/src/lib.rs index 1c035276..c6939efc 100644 --- a/query-parser/src/lib.rs +++ b/query-parser/src/lib.rs @@ -24,12 +24,16 @@ extern crate edn; #[macro_use] extern crate mentat_parser_utils; +mod errors; mod parse; -pub use parse::{ +pub use errors::{ Error, ErrorKind, Result, ResultExt, +}; + +pub use parse::{ parse_find_string, }; diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs index 49659632..545ed38b 100644 --- a/query-parser/src/parse.rs +++ b/query-parser/src/parse.rs @@ -36,6 +36,13 @@ use self::combine::combinator::{any, choice, or, try}; use self::mentat_core::ValueType; +use errors::{ + Error, + ErrorKind, + Result, + ResultExt, +}; + use self::mentat_parser_utils::{ KeywordMapParser, ResultParser, @@ -87,64 +94,6 @@ use self::mentat_query::{ WhereFn, }; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - foreign_links { - EdnParseError(edn::parse::ParseError); - } - - errors { - DuplicateVariableError { - description("duplicate variable") - display("duplicates in variable list") - } - - NotAVariableError(value: edn::ValueAndSpan) { - description("not a variable") - display("not a variable: '{}'", value) - } - - FindParseError(e: ValueParseError) { - description(":find parse error") - display(":find parse error") - } - - WhereParseError(e: ValueParseError) { - description(":where parse error") - display(":where parse error") - } - - // Not yet used. - WithParseError { - description(":with parse error") - display(":with parse error") - } - - InvalidInputError(value: edn::Value) { - description("invalid input") - display("invalid input: '{}'", value) - } - - MissingFieldError(value: edn::Keyword) { - description("missing field") - display("missing field: '{}'", value) - } - - UnknownLimitVar(var: edn::PlainSymbol) { - description("limit var not present in :in") - display("limit var {} not present in :in", var) - } - - InvalidLimit(val: edn::Value) { - description("limit value not valid") - display("expected natural number, got {}", val) - } - } -} - pub struct Query<'a>(std::marker::PhantomData<&'a ()>); def_parser!(Query, variable, Variable, { From ad9a1394a3e753213c3eef74bcf7ede98c6e9728 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 28 May 2018 13:01:14 -0700 Subject: [PATCH 05/13] Part 1: Push ValueRc and friends into `edn` crate. This is a pre-requisite for moving the existing `combine`-based parser to use `rust-peg` -- part of the push to use `rust-peg` for all parsing started in https://github.com/mozilla/mentat/pull/681. We need the types for the parsed structure "very early", and the `edn` crate is the earliest such crate. This is an unfortunate destruction of boundaries between parts of the system, but it's the best way we have to achieve this right now. --- core/src/lib.rs | 6 +-- core/src/types.rs | 93 ++------------------------------------ edn/src/lib.rs | 6 +++ edn/src/value_rc.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 93 deletions(-) create mode 100644 edn/src/value_rc.rs diff --git a/core/src/lib.rs b/core/src/lib.rs index bf2446c8..f13341b3 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -38,10 +38,13 @@ pub use chrono::{ }; pub use edn::{ + Cloned, FromMicros, + FromRc, Keyword, ToMicros, Utc, + ValueRc, }; pub use cache::{ @@ -56,15 +59,12 @@ mod sql_types; pub use types::{ Binding, - Cloned, Entid, - FromRc, KnownEntid, StructuredMap, TypedValue, ValueType, ValueTypeTag, - ValueRc, now, }; diff --git a/core/src/types.rs b/core/src/types.rs index ca3b86d4..1ead80b3 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -39,103 +39,16 @@ use ::indexmap::{ use ::edn::{ self, + Cloned, FromMicros, + FromRc, Keyword, Utc, + ValueRc, }; use values; -pub trait FromRc { - fn from_rc(val: Rc) -> Self; - fn from_arc(val: Arc) -> Self; -} - -impl FromRc for Rc where T: Sized + Clone { - fn from_rc(val: Rc) -> Self { - val.clone() - } - - fn from_arc(val: Arc) -> Self { - match ::std::sync::Arc::::try_unwrap(val) { - Ok(v) => Self::new(v), - Err(r) => Self::new(r.cloned()), - } - } -} - -impl FromRc for Arc where T: Sized + Clone { - fn from_rc(val: Rc) -> Self { - match ::std::rc::Rc::::try_unwrap(val) { - Ok(v) => Self::new(v), - Err(r) => Self::new(r.cloned()), - } - } - - fn from_arc(val: Arc) -> Self { - val.clone() - } -} - -impl FromRc for Box where T: Sized + Clone { - fn from_rc(val: Rc) -> Self { - match ::std::rc::Rc::::try_unwrap(val) { - Ok(v) => Self::new(v), - Err(r) => Self::new(r.cloned()), - } - } - - fn from_arc(val: Arc) -> Self { - match ::std::sync::Arc::::try_unwrap(val) { - Ok(v) => Self::new(v), - Err(r) => Self::new(r.cloned()), - } - } -} - -// We do this a lot for errors. -pub trait Cloned { - fn cloned(&self) -> T; - fn to_value_rc(&self) -> ValueRc; -} - -impl Cloned for Rc where T: Sized + Clone { - fn cloned(&self) -> T { - (*self.as_ref()).clone() - } - - fn to_value_rc(&self) -> ValueRc { - ValueRc::from_rc(self.clone()) - } -} - -impl Cloned for Arc where T: Sized + Clone { - fn cloned(&self) -> T { - (*self.as_ref()).clone() - } - - fn to_value_rc(&self) -> ValueRc { - ValueRc::from_arc(self.clone()) - } -} - -impl Cloned for Box where T: Sized + Clone { - fn cloned(&self) -> T { - self.as_ref().clone() - } - - fn to_value_rc(&self) -> ValueRc { - ValueRc::new(self.cloned()) - } -} - -/// -/// This type alias exists to allow us to use different boxing mechanisms for values. -/// This type must implement `FromRc` and `Cloned`, and a `From` implementation must exist for -/// `TypedValue`. -/// -pub type ValueRc = Arc; - /// Represents one entid in the entid space. /// /// Per https://www.sqlite.org/datatype3.html (see also http://stackoverflow.com/a/8499544), SQLite diff --git a/edn/src/lib.rs b/edn/src/lib.rs index f8423a37..0527c05a 100644 --- a/edn/src/lib.rs +++ b/edn/src/lib.rs @@ -30,6 +30,12 @@ pub mod types; pub mod pretty_print; pub mod utils; pub mod matcher; +pub mod value_rc; +pub use value_rc::{ + Cloned, + FromRc, + ValueRc, +}; pub mod parse { include!(concat!(env!("OUT_DIR"), "/edn.rs")); diff --git a/edn/src/value_rc.rs b/edn/src/value_rc.rs new file mode 100644 index 00000000..88a5ba7c --- /dev/null +++ b/edn/src/value_rc.rs @@ -0,0 +1,107 @@ +// Copyright 2018 Mozilla +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. + +use ::std::rc::{ + Rc, +}; + +use ::std::sync::{ + Arc, +}; + +pub trait FromRc { + fn from_rc(val: Rc) -> Self; + fn from_arc(val: Arc) -> Self; +} + +impl FromRc for Rc where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + val.clone() + } + + fn from_arc(val: Arc) -> Self { + match ::std::sync::Arc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } +} + +impl FromRc for Arc where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + match ::std::rc::Rc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } + + fn from_arc(val: Arc) -> Self { + val.clone() + } +} + +impl FromRc for Box where T: Sized + Clone { + fn from_rc(val: Rc) -> Self { + match ::std::rc::Rc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } + + fn from_arc(val: Arc) -> Self { + match ::std::sync::Arc::::try_unwrap(val) { + Ok(v) => Self::new(v), + Err(r) => Self::new(r.cloned()), + } + } +} + +// We do this a lot for errors. +pub trait Cloned { + fn cloned(&self) -> T; + fn to_value_rc(&self) -> ValueRc; +} + +impl Cloned for Rc where T: Sized + Clone { + fn cloned(&self) -> T { + (*self.as_ref()).clone() + } + + fn to_value_rc(&self) -> ValueRc { + ValueRc::from_rc(self.clone()) + } +} + +impl Cloned for Arc where T: Sized + Clone { + fn cloned(&self) -> T { + (*self.as_ref()).clone() + } + + fn to_value_rc(&self) -> ValueRc { + ValueRc::from_arc(self.clone()) + } +} + +impl Cloned for Box where T: Sized + Clone { + fn cloned(&self) -> T { + self.as_ref().clone() + } + + fn to_value_rc(&self) -> ValueRc { + ValueRc::new(self.cloned()) + } +} + +/// +/// This type alias exists to allow us to use different boxing mechanisms for values. +/// This type must implement `FromRc` and `Cloned`, and a `From` implementation must exist for +/// `TypedValue`. +/// +pub type ValueRc = Arc; From 1d8d94f88725730e76d331152b02d09d83cd855b Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 28 May 2018 14:34:42 -0700 Subject: [PATCH 06/13] Part 2: Turn `(type-function ?var)` into `(type ?var type-keyword)`. This is more general (the parser doesn't encode the set of known types), and avoids a dependency on `ValueType`. --- core/src/types.rs | 18 ++++++++ query-algebrizer/src/clauses/predicate.rs | 6 ++- query-algebrizer/tests/type_reqs.rs | 22 +++------- query-parser/Cargo.toml | 4 -- query-parser/src/lib.rs | 3 -- query-parser/src/parse.rs | 51 ++++++++--------------- query-parser/tests/find_tests.rs | 1 - query-translator/tests/translate.rs | 8 ++-- query/src/lib.rs | 3 +- tests/query.rs | 27 ++++-------- 10 files changed, 60 insertions(+), 83 deletions(-) diff --git a/core/src/types.rs b/core/src/types.rs index 1ead80b3..716f943f 100644 --- a/core/src/types.rs +++ b/core/src/types.rs @@ -131,6 +131,24 @@ impl ValueType { }) } + pub fn from_keyword(keyword: &Keyword) -> Option { + if keyword.namespace() != Some("db.type") { + return None; + } + + return match keyword.name() { + "ref" => Some(ValueType::Ref), + "boolean" => Some(ValueType::Boolean), + "instant" => Some(ValueType::Instant), + "long" => Some(ValueType::Long), + "double" => Some(ValueType::Double), + "string" => Some(ValueType::String), + "keyword" => Some(ValueType::Keyword), + "uuid" => Some(ValueType::Uuid), + _ => None, + } + } + pub fn into_typed_value(self) -> TypedValue { TypedValue::typed_ns_keyword("db.type", match self { ValueType::Ref => "ref", diff --git a/query-algebrizer/src/clauses/predicate.rs b/query-algebrizer/src/clauses/predicate.rs index f099b7b8..add9eb8c 100644 --- a/query-algebrizer/src/clauses/predicate.rs +++ b/query-algebrizer/src/clauses/predicate.rs @@ -16,6 +16,7 @@ use mentat_core::{ use mentat_query::{ FnArg, + PlainSymbol, Predicate, TypeAnnotation, }; @@ -66,7 +67,10 @@ impl ConjoiningClauses { /// Apply a type annotation, which is a construct like a predicate that constrains the argument /// to be a specific ValueType. pub(crate) fn apply_type_anno(&mut self, anno: &TypeAnnotation) -> Result<()> { - self.add_type_requirement(anno.variable.clone(), ValueTypeSet::of_one(anno.value_type)); + match ValueType::from_keyword(&anno.value_type) { + Some(value_type) => self.add_type_requirement(anno.variable.clone(), ValueTypeSet::of_one(value_type)), + None => bail!(ErrorKind::InvalidArgumentType(PlainSymbol::plain("type"), ValueTypeSet::any(), 2)), + } Ok(()) } diff --git a/query-algebrizer/tests/type_reqs.rs b/query-algebrizer/tests/type_reqs.rs index 6cec8e27..a8f5179b 100644 --- a/query-algebrizer/tests/type_reqs.rs +++ b/query-algebrizer/tests/type_reqs.rs @@ -43,22 +43,12 @@ fn prepopulated_schema() -> Schema { #[test] fn test_empty_known() { - let type_names = [ - "boolean", - "long", - "double", - "string", - "keyword", - "uuid", - "instant", - "ref", - ]; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - for known_type in type_names.iter() { - for required in type_names.iter() { - let q = format!("[:find ?e :where [?e :test/{} ?v] [({} ?v)]]", - known_type, required); + for known_type in ValueType::all_enums().iter() { + for required in ValueType::all_enums().iter() { + let q = format!("[:find ?e :where [?e :test/{} ?v] [(type ?v {})]]", + known_type.into_keyword().name(), required); println!("Query: {}", q); let cc = alg(known, &q); // It should only be empty if the known type and our requirement differ. @@ -72,7 +62,7 @@ fn test_empty_known() { fn test_multiple() { let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let q = "[:find ?e :where [?e _ ?v] [(long ?v)] [(double ?v)]]"; + let q = "[:find ?e :where [?e _ ?v] [(type ?v :db.type/long)] [(type ?v :db.type/double)]]"; let cc = alg(known, &q); assert!(cc.empty_because.is_some()); } @@ -81,5 +71,5 @@ fn test_multiple() { fn test_unbound() { let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - bails(known, "[:find ?e :where [(string ?e)]]"); + bails(known, "[:find ?e :where [(type ?e :db.type/string)]]"); } diff --git a/query-parser/Cargo.toml b/query-parser/Cargo.toml index b6670281..2a91dbc4 100644 --- a/query-parser/Cargo.toml +++ b/query-parser/Cargo.toml @@ -7,14 +7,10 @@ workspace = ".." combine = "2.3.2" error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } maplit = "0.1" -matches = "0.1" [dependencies.edn] path = "../edn" -[dependencies.mentat_core] - path = "../core" - [dependencies.mentat_parser_utils] path = "../parser-utils" diff --git a/query-parser/src/lib.rs b/query-parser/src/lib.rs index c6939efc..e5deff3a 100644 --- a/query-parser/src/lib.rs +++ b/query-parser/src/lib.rs @@ -16,9 +16,6 @@ extern crate maplit; #[macro_use] extern crate error_chain; -#[macro_use] -extern crate matches; - extern crate edn; #[macro_use] diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs index 545ed38b..0b11d3bf 100644 --- a/query-parser/src/parse.rs +++ b/query-parser/src/parse.rs @@ -12,7 +12,6 @@ extern crate combine; extern crate edn; extern crate mentat_parser_utils; extern crate mentat_query; -extern crate mentat_core; use std; // To refer to std::result::Result. @@ -34,8 +33,6 @@ use self::combine::{ use self::combine::combinator::{any, choice, or, try}; -use self::mentat_core::ValueType; - use errors::{ Error, ErrorKind, @@ -100,6 +97,10 @@ def_parser!(Query, variable, Variable, { satisfy_map(Variable::from_value) }); +def_parser!(Query, keyword, edn::Keyword, { + satisfy_map(|v: &edn::ValueAndSpan| v.inner.as_keyword().cloned()) +}); + def_parser!(Query, source_var, SrcVar, { satisfy_map(SrcVar::from_value) }); @@ -178,6 +179,8 @@ def_matches_plain_symbol!(Where, not, "not"); def_matches_plain_symbol!(Where, not_join, "not-join"); +def_matches_plain_symbol!(Where, type_symbol, "type"); + def_parser!(Where, rule_vars, BTreeSet, { seq() .of_exactly(many1(Query::variable()).and_then(unique_vars)) @@ -339,36 +342,14 @@ def_parser!(Where, pred, WhereClause, { }))) }); -def_parser!(Query, type_anno_type, ValueType, { - satisfy_map(|v: &edn::ValueAndSpan| { - match v.inner { - edn::SpannedValue::PlainSymbol(ref s) => { - let name = s.0.as_str(); - match name { - "ref" => Some(ValueType::Ref), - "boolean" => Some(ValueType::Boolean), - "instant" => Some(ValueType::Instant), - "long" => Some(ValueType::Long), - "double" => Some(ValueType::Double), - "string" => Some(ValueType::String), - "keyword" => Some(ValueType::Keyword), - "uuid" => Some(ValueType::Uuid), - _ => None - } - }, - _ => None, - } - }) -}); - /// A type annotation. def_parser!(Where, type_annotation, WhereClause, { // Accept either a nested list or a nested vector here: - // `[(string ?x)]` or `[[string ?x]]` + // `[(type ?x :db.type/string)]` or `[[type ?x :db.type/long]]` vector() .of_exactly(seq() - .of_exactly((Query::type_anno_type(), Query::variable()) - .map(|(ty, var)| { + .of_exactly((Where::type_symbol(), Query::variable(), Query::keyword()) + .map(|(_, var, ty)| { WhereClause::TypeAnnotation( TypeAnnotation { value_type: ty, @@ -759,7 +740,10 @@ mod test { let input = input.with_spans(); let mut par = Where::pattern(); let result = par.parse(input.atom_stream()); - assert!(matches!(result, Err(_)), "Expected a parse error."); + match result { + Err(_) => (), + _ => assert!(false, "Expected a parse error"), + } } #[test] @@ -1137,15 +1121,16 @@ mod test { #[test] fn test_type_anno() { assert_edn_parses_to!(Where::type_annotation, - "[(string ?x)]", + "[(type ?x :db.type/string)]", WhereClause::TypeAnnotation(TypeAnnotation { - value_type: ValueType::String, + value_type: edn::Keyword::namespaced("db.type", "string"), variable: Variable::from_valid_name("?x"), })); + // We don't check for valid types, or even that the type is namespaced. assert_edn_parses_to!(Where::clause, - "[[long ?foo]]", + "[[type ?foo :db_type_long]]", WhereClause::TypeAnnotation(TypeAnnotation { - value_type: ValueType::Long, + value_type: edn::Keyword::plain("db_type_long"), variable: Variable::from_valid_name("?foo"), })); diff --git a/query-parser/tests/find_tests.rs b/query-parser/tests/find_tests.rs index e274e6ff..95e5bc66 100644 --- a/query-parser/tests/find_tests.rs +++ b/query-parser/tests/find_tests.rs @@ -12,7 +12,6 @@ extern crate maplit; extern crate edn; -extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_parser; diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index 0e17e4a0..3fa6ffe6 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -348,7 +348,7 @@ fn test_unknown_ident() { fn test_type_required_long() { let schema = Schema::default(); - let query = r#"[:find ?x :where [?x _ ?e] [(long ?e)]]"#; + let query = r#"[:find ?x :where [?x _ ?e] [(type ?e :db.type/long)]]"#; let SQLQuery { sql, args } = translate(&schema, query); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` \ @@ -363,7 +363,7 @@ fn test_type_required_long() { fn test_type_required_double() { let schema = Schema::default(); - let query = r#"[:find ?x :where [?x _ ?e] [(double ?e)]]"#; + let query = r#"[:find ?x :where [?x _ ?e] [(type ?e :db.type/double)]]"#; let SQLQuery { sql, args } = translate(&schema, query); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` \ @@ -378,7 +378,7 @@ fn test_type_required_double() { fn test_type_required_boolean() { let schema = Schema::default(); - let query = r#"[:find ?x :where [?x _ ?e] [(boolean ?e)]]"#; + let query = r#"[:find ?x :where [?x _ ?e] [(type ?e :db.type/boolean)]]"#; let SQLQuery { sql, args } = translate(&schema, query); assert_eq!(sql, "SELECT DISTINCT `datoms00`.e AS `?x` \ @@ -392,7 +392,7 @@ fn test_type_required_boolean() { fn test_type_required_string() { let schema = Schema::default(); - let query = r#"[:find ?x :where [?x _ ?e] [(string ?e)]]"#; + let query = r#"[:find ?x :where [?x _ ?e] [(type ?e :db.type/string)]]"#; let SQLQuery { sql, args } = translate(&schema, query); // Note: strings should use `all_datoms` and not `datoms`. diff --git a/query/src/lib.rs b/query/src/lib.rs index 7c692949..139b7330 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -58,7 +58,6 @@ use mentat_core::{ FromRc, TypedValue, ValueRc, - ValueType, }; pub type SrcVarName = String; // Do not include the required syntactic '$'. @@ -967,7 +966,7 @@ pub struct NotJoin { #[derive(Clone, Debug, Eq, PartialEq)] pub struct TypeAnnotation { - pub value_type: ValueType, + pub value_type: Keyword, pub variable: Variable, } diff --git a/tests/query.rs b/tests/query.rs index edca7983..b0422f23 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -602,7 +602,7 @@ fn test_aggregates_type_handling() { // You can't sum instants. let r = store.q_once(r#"[:find (sum ?v) . - :where [_ _ ?v] [(instant ?v)]]"#, + :where [_ _ ?v] [(type ?v :db.type/instant)]]"#, None); match r { Result::Err( @@ -623,7 +623,7 @@ fn test_aggregates_type_handling() { // But you can count them. let r = store.q_once(r#"[:find (count ?v) . - :where [_ _ ?v] [(instant ?v)]]"#, + :where [_ _ ?v] [(type ?v :db.type/instant)]]"#, None) .into_scalar_result() .expect("results") @@ -634,7 +634,7 @@ fn test_aggregates_type_handling() { // And you can min them, which returns an instant. let r = store.q_once(r#"[:find (min ?v) . - :where [_ _ ?v] [(instant ?v)]]"#, + :where [_ _ ?v] [(type ?v :db.type/instant)]]"#, None) .into_scalar_result() .expect("results") @@ -644,7 +644,7 @@ fn test_aggregates_type_handling() { assert_eq!(Binding::Scalar(TypedValue::Instant(earliest)), r); let r = store.q_once(r#"[:find (sum ?v) . - :where [_ _ ?v] [(long ?v)]]"#, + :where [_ _ ?v] [(type ?v :db.type/long)]]"#, None) .into_scalar_result() .expect("results") @@ -655,7 +655,7 @@ fn test_aggregates_type_handling() { assert_eq!(Binding::Scalar(TypedValue::Long(total)), r); let r = store.q_once(r#"[:find (avg ?v) . - :where [_ _ ?v] [(double ?v)]]"#, + :where [_ _ ?v] [(type ?v :db.type/double)]]"#, None) .into_scalar_result() .expect("results") @@ -710,19 +710,8 @@ fn test_type_reqs() { } }; - let type_names = &[ - "boolean", - "long", - "double", - "string", - "keyword", - "uuid", - "instant", - "ref", - ]; - - for name in type_names { - let q = format!("[:find [?v ...] :in ?e :where [?e _ ?v] [({} ?v)]]", name); + for value_type in ValueType::all_enums().iter() { + let q = format!("[:find [?v ...] :in ?e :where [?e _ ?v] [(type ?v {})]]", value_type.into_keyword()); let results = conn.q_once(&mut c, &q, QueryInputs::with_value_sequence(vec![ (Variable::from_valid_name("?e"), TypedValue::Ref(entid)), ])) @@ -746,7 +735,7 @@ fn test_type_reqs() { let longs_query = r#"[:find [?v ...] :order (asc ?v) :in ?e - :where [?e _ ?v] [(long ?v)]]"#; + :where [?e _ ?v] [(type ?v :db.type/long)]]"#; let res = conn.q_once(&mut c, longs_query, QueryInputs::with_value_sequence(vec![ (Variable::from_valid_name("?e"), TypedValue::Ref(entid)), From a4a88923098778fa2322cd94f9648dbcc6de1f43 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 4 Jun 2018 14:56:56 -0700 Subject: [PATCH 07/13] Part 3a: Move file to preserve blame. --- query/src/lib.rs => edn/src/query.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename query/src/lib.rs => edn/src/query.rs (100%) diff --git a/query/src/lib.rs b/edn/src/query.rs similarity index 100% rename from query/src/lib.rs rename to edn/src/query.rs From a8073056f2220d23d93ad1f53af1f606439bed8d Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 28 May 2018 14:47:35 -0700 Subject: [PATCH 08/13] Part 3: Move `query` into `edn`. It's unfortunate to squash two crates together like this, but it's the best option. --- edn/src/lib.rs | 1 + edn/src/query.rs | 126 +++++++++++------------- query-algebrizer/src/clauses/pattern.rs | 15 ++- query/src/lib.rs | 12 +++ 4 files changed, 81 insertions(+), 73 deletions(-) create mode 100644 query/src/lib.rs diff --git a/edn/src/lib.rs b/edn/src/lib.rs index 0527c05a..a2a1dbcc 100644 --- a/edn/src/lib.rs +++ b/edn/src/lib.rs @@ -25,6 +25,7 @@ extern crate serde_derive; pub mod entities; // Intentionally not pub. mod namespaceable_name; +pub mod query; pub mod symbols; pub mod types; pub mod pretty_print; diff --git a/edn/src/query.rs b/edn/src/query.rs index 139b7330..c1c69fd9 100644 --- a/edn/src/query.rs +++ b/edn/src/query.rs @@ -30,18 +30,18 @@ ///! deeply into this it's worth recognizing that this loss of 'sovereignty' is ///! a tradeoff against well-typed function signatures and other such boundaries. -extern crate edn; -extern crate mentat_core; - use std::collections::{ BTreeSet, HashSet, }; +use std; use std::fmt; -use std::rc::Rc; +use std::rc::{ + Rc, +}; -use edn::{ +use ::{ BigInt, DateTime, OrderedFloat, @@ -49,15 +49,14 @@ use edn::{ Utc, }; -pub use edn::{ - Keyword, - PlainSymbol, +use ::value_rc::{ + FromRc, + ValueRc, }; -use mentat_core::{ - FromRc, - TypedValue, - ValueRc, +pub use ::{ + Keyword, + PlainSymbol, }; pub type SrcVarName = String; // Do not include the required syntactic '$'. @@ -87,15 +86,15 @@ impl Variable { } pub trait FromValue { - fn from_value(v: &edn::ValueAndSpan) -> Option; + fn from_value(v: &::ValueAndSpan) -> Option; } /// If the provided EDN value is a PlainSymbol beginning with '?', return /// it wrapped in a Variable. If not, return None. /// TODO: intern strings. #398. impl FromValue for Variable { - fn from_value(v: &edn::ValueAndSpan) -> Option { - if let edn::SpannedValue::PlainSymbol(ref s) = v.inner { + fn from_value(v: &::ValueAndSpan) -> Option { + if let ::SpannedValue::PlainSymbol(ref s) = v.inner { Variable::from_symbol(s) } else { None @@ -138,8 +137,8 @@ impl std::fmt::Display for Variable { pub struct QueryFunction(pub PlainSymbol); impl FromValue for QueryFunction { - fn from_value(v: &edn::ValueAndSpan) -> Option { - if let edn::SpannedValue::PlainSymbol(ref s) = v.inner { + fn from_value(v: &::ValueAndSpan) -> Option { + if let ::SpannedValue::PlainSymbol(ref s) = v.inner { QueryFunction::from_symbol(s) } else { None @@ -177,8 +176,8 @@ pub enum SrcVar { } impl FromValue for SrcVar { - fn from_value(v: &edn::ValueAndSpan) -> Option { - if let edn::SpannedValue::PlainSymbol(ref s) = v.inner { + fn from_value(v: &::ValueAndSpan) -> Option { + if let ::SpannedValue::PlainSymbol(ref s) = v.inner { SrcVar::from_symbol(s) } else { None @@ -211,19 +210,6 @@ pub enum NonIntegerConstant { Uuid(Uuid), } -impl NonIntegerConstant { - pub fn into_typed_value(self) -> TypedValue { - match self { - NonIntegerConstant::BigInteger(_) => unimplemented!(), // TODO: #280. - NonIntegerConstant::Boolean(v) => TypedValue::Boolean(v), - NonIntegerConstant::Float(v) => TypedValue::Double(v), - NonIntegerConstant::Text(v) => v.into(), - NonIntegerConstant::Instant(v) => TypedValue::Instant(v), - NonIntegerConstant::Uuid(v) => TypedValue::Uuid(v), - } - } -} - impl<'a> From<&'a str> for NonIntegerConstant { fn from(val: &'a str) -> NonIntegerConstant { NonIntegerConstant::Text(ValueRc::new(val.to_string())) @@ -249,8 +235,8 @@ pub enum FnArg { } impl FromValue for FnArg { - fn from_value(v: &edn::ValueAndSpan) -> Option { - use edn::SpannedValue::*; + fn from_value(v: &::ValueAndSpan) -> Option { + use ::SpannedValue::*; match v.inner { Integer(x) => Some(FnArg::EntidOrInteger(x)), @@ -362,14 +348,14 @@ impl PatternNonValuePlace { } impl FromValue for PatternNonValuePlace { - fn from_value(v: &edn::ValueAndSpan) -> Option { + fn from_value(v: &::ValueAndSpan) -> Option { match v.inner { - edn::SpannedValue::Integer(x) => if x >= 0 { + ::SpannedValue::Integer(x) => if x >= 0 { Some(PatternNonValuePlace::Entid(x)) } else { None }, - edn::SpannedValue::PlainSymbol(ref x) => if x.0.as_str() == "_" { + ::SpannedValue::PlainSymbol(ref x) => if x.0.as_str() == "_" { Some(PatternNonValuePlace::Placeholder) } else { if let Some(v) = Variable::from_symbol(x) { @@ -378,7 +364,7 @@ impl FromValue for PatternNonValuePlace { None } }, - edn::SpannedValue::Keyword(ref x) => + ::SpannedValue::Keyword(ref x) => Some(x.clone().into()), _ => None, } @@ -416,38 +402,38 @@ impl From for PatternValuePlace { } impl FromValue for PatternValuePlace { - fn from_value(v: &edn::ValueAndSpan) -> Option { + fn from_value(v: &::ValueAndSpan) -> Option { match v.inner { - edn::SpannedValue::Integer(x) => + ::SpannedValue::Integer(x) => Some(PatternValuePlace::EntidOrInteger(x)), - edn::SpannedValue::PlainSymbol(ref x) if x.0.as_str() == "_" => + ::SpannedValue::PlainSymbol(ref x) if x.0.as_str() == "_" => Some(PatternValuePlace::Placeholder), - edn::SpannedValue::PlainSymbol(ref x) => + ::SpannedValue::PlainSymbol(ref x) => Variable::from_symbol(x).map(PatternValuePlace::Variable), - edn::SpannedValue::Keyword(ref x) if x.is_namespaced() => + ::SpannedValue::Keyword(ref x) if x.is_namespaced() => Some(x.clone().into()), - edn::SpannedValue::Boolean(x) => + ::SpannedValue::Boolean(x) => Some(PatternValuePlace::Constant(NonIntegerConstant::Boolean(x))), - edn::SpannedValue::Float(x) => + ::SpannedValue::Float(x) => Some(PatternValuePlace::Constant(NonIntegerConstant::Float(x))), - edn::SpannedValue::BigInteger(ref x) => + ::SpannedValue::BigInteger(ref x) => Some(PatternValuePlace::Constant(NonIntegerConstant::BigInteger(x.clone()))), - edn::SpannedValue::Instant(x) => + ::SpannedValue::Instant(x) => Some(PatternValuePlace::Constant(NonIntegerConstant::Instant(x))), - edn::SpannedValue::Text(ref x) => + ::SpannedValue::Text(ref x) => // TODO: intern strings. #398. Some(PatternValuePlace::Constant(x.clone().into())), - edn::SpannedValue::Uuid(ref u) => + ::SpannedValue::Uuid(ref u) => Some(PatternValuePlace::Constant(NonIntegerConstant::Uuid(u.clone()))), // These don't appear in queries. - edn::SpannedValue::Nil => None, - edn::SpannedValue::NamespacedSymbol(_) => None, - edn::SpannedValue::Keyword(_) => None, // … yet. - edn::SpannedValue::Map(_) => None, - edn::SpannedValue::List(_) => None, - edn::SpannedValue::Set(_) => None, - edn::SpannedValue::Vector(_) => None, + ::SpannedValue::Nil => None, + ::SpannedValue::NamespacedSymbol(_) => None, + ::SpannedValue::Keyword(_) => None, // … yet. + ::SpannedValue::Map(_) => None, + ::SpannedValue::List(_) => None, + ::SpannedValue::Set(_) => None, + ::SpannedValue::Vector(_) => None, } } } @@ -647,8 +633,7 @@ pub enum Limit { /// Examples: /// /// ```rust -/// # extern crate mentat_query; -/// # use mentat_query::{Element, FindSpec, Variable}; +/// # use edn::query::{Element, FindSpec, Variable}; /// /// # fn main() { /// @@ -687,7 +672,7 @@ pub enum FindSpec { /// Returns true if the provided `FindSpec` returns at most one result. impl FindSpec { pub fn is_unit_limited(&self) -> bool { - use FindSpec::*; + use self::FindSpec::*; match self { &FindScalar(..) => true, &FindTuple(..) => true, @@ -697,7 +682,7 @@ impl FindSpec { } pub fn expected_column_count(&self) -> usize { - use FindSpec::*; + use self::FindSpec::*; match self { &FindScalar(..) => 1, &FindColl(..) => 1, @@ -729,7 +714,7 @@ impl FindSpec { } pub fn columns<'s>(&'s self) -> Box + 's> { - use FindSpec::*; + use self::FindSpec::*; match self { &FindScalar(ref e) => Box::new(std::iter::once(e)), &FindColl(ref e) => Box::new(std::iter::once(e)), @@ -792,16 +777,16 @@ impl Binding { /// placeholder or unique. /// /// ``` - /// extern crate mentat_query; + /// use edn::query::{Binding,Variable,VariableOrPlaceholder}; /// use std::rc::Rc; /// - /// let v = mentat_query::Variable::from_valid_name("?foo"); - /// let vv = mentat_query::VariableOrPlaceholder::Variable(v); - /// let p = mentat_query::VariableOrPlaceholder::Placeholder; + /// let v = Variable::from_valid_name("?foo"); + /// let vv = VariableOrPlaceholder::Variable(v); + /// let p = VariableOrPlaceholder::Placeholder; /// - /// let e = mentat_query::Binding::BindTuple(vec![p.clone()]); - /// let b = mentat_query::Binding::BindTuple(vec![p.clone(), vv.clone()]); - /// let d = mentat_query::Binding::BindTuple(vec![vv.clone(), p, vv]); + /// let e = Binding::BindTuple(vec![p.clone()]); + /// let b = Binding::BindTuple(vec![p.clone(), vv.clone()]); + /// let d = Binding::BindTuple(vec![vv.clone(), p, vv]); /// assert!(b.is_valid()); // One var, one placeholder: OK. /// assert!(!e.is_valid()); // Empty: not OK. /// assert!(!d.is_valid()); // Duplicate var: not OK. @@ -1052,7 +1037,7 @@ pub trait ContainsVariables { impl ContainsVariables for WhereClause { fn accumulate_mentioned_variables(&self, acc: &mut BTreeSet) { - use WhereClause::*; + use self::WhereClause::*; match self { &OrJoin(ref o) => o.accumulate_mentioned_variables(acc), &Pred(ref p) => p.accumulate_mentioned_variables(acc), @@ -1067,7 +1052,7 @@ impl ContainsVariables for WhereClause { impl ContainsVariables for OrWhereClause { fn accumulate_mentioned_variables(&self, acc: &mut BTreeSet) { - use OrWhereClause::*; + use self::OrWhereClause::*; match self { &And(ref clauses) => for clause in clauses { clause.accumulate_mentioned_variables(acc) }, &Clause(ref clause) => clause.accumulate_mentioned_variables(acc), @@ -1124,7 +1109,6 @@ impl ContainsVariables for Predicate { } } - impl ContainsVariables for TypeAnnotation { fn accumulate_mentioned_variables(&self, acc: &mut BTreeSet) { acc_ref(acc, &self.variable); diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index e338c392..dd4561a4 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -18,6 +18,7 @@ use mentat_core::{ }; use mentat_query::{ + NonIntegerConstant, Pattern, PatternValuePlace, PatternNonValuePlace, @@ -42,6 +43,17 @@ use types::{ use Known; +pub fn into_typed_value(nic: NonIntegerConstant) -> TypedValue { + match nic { + NonIntegerConstant::BigInteger(_) => unimplemented!(), // TODO: #280. + NonIntegerConstant::Boolean(v) => TypedValue::Boolean(v), + NonIntegerConstant::Float(v) => TypedValue::Double(v), + NonIntegerConstant::Text(v) => v.into(), + NonIntegerConstant::Instant(v) => TypedValue::Instant(v), + NonIntegerConstant::Uuid(v) => TypedValue::Uuid(v), + } +} + /// Application of patterns. impl ConjoiningClauses { @@ -518,7 +530,7 @@ impl ConjoiningClauses { } }, PatternValuePlace::Constant(nic) => { - Place(EvolvedValuePlace::Value(nic.into_typed_value())) + Place(EvolvedValuePlace::Value(into_typed_value(nic))) }, } } @@ -655,7 +667,6 @@ mod testing { use mentat_query::{ Keyword, - NonIntegerConstant, Variable, }; diff --git a/query/src/lib.rs b/query/src/lib.rs new file mode 100644 index 00000000..ab117a70 --- /dev/null +++ b/query/src/lib.rs @@ -0,0 +1,12 @@ +// Copyright 2016 Mozilla +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed +// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +// CONDITIONS OF ANY KIND, either express or implied. See the License for the +// specific language governing permissions and limitations under the License. + +extern crate edn; +pub use edn::query::*; From 09f1d633b5debf15954501cbb32b5192818882fc Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 28 May 2018 15:27:19 -0700 Subject: [PATCH 09/13] Part 4: Parse queries with `rust-peg`. There's an unfortunate conflation here between implementing the query parser in `rust-peg` and moving some validation that now happens at parse time to happen later. The result is that we introduce `ParsedFindQuery` as a less-processed `FindQuery`, and that we only use string errors (which is all `rust-peg` supports) instead of the structured errors in query-parser's errors module. The next commit will address this, on the road to removing the `query-parser` module entirely. --- edn/src/edn.rustpeg | 233 +++++++ edn/src/query.rs | 134 ++++ query-parser/src/lib.rs | 9 +- query-parser/src/parse.rs | 1235 ------------------------------------- 4 files changed, 372 insertions(+), 1239 deletions(-) delete mode 100644 query-parser/src/parse.rs diff --git a/edn/src/edn.rustpeg b/edn/src/edn.rustpeg index c09ac224..5b77672d 100644 --- a/edn/src/edn.rustpeg +++ b/edn/src/edn.rustpeg @@ -1,3 +1,4 @@ +/* -*- comment-start: "//"; -*- */ /* vim: set filetype=rust.rustpeg */ // Copyright 2016 Mozilla @@ -24,6 +25,8 @@ use ordered_float::OrderedFloat; use uuid::Uuid; use entities::*; +use query; +use query::FromValue; use symbols::*; use types::{SpannedValue, Span, ValueAndSpan}; @@ -155,12 +158,14 @@ pub symbol -> SpannedValue = ns:( sns:$(symbol_namespace) namespace_separator { sns })? n:$(plain_symbol_name) { SpannedValue::from_symbol(ns, n) } + / #expected("symbol") pub keyword -> SpannedValue = keyword_prefix ns:( sns:$(symbol_namespace) namespace_separator { sns })? n:$(symbol_name) { SpannedValue::from_keyword(ns, n) } + / #expected("keyword") pub list -> SpannedValue = "(" __ v:(value)* __ ")" { SpannedValue::List(LinkedList::from_iter(v)) } @@ -188,6 +193,7 @@ pub value -> ValueAndSpan = span: Span::new(start, end) } } + / #expected("value") atom -> ValueAndSpan = v:value {? if v.is_atom() { Ok(v) } else { Err("expected atom") } } @@ -199,10 +205,29 @@ comment = #quiet<";" [^\r\n]* [\r\n]?> __ = (whitespace / comment)* +// Transaction entity parser starts here. + pub op -> OpType = ":db/add" { OpType::Add } / ":db/retract" { OpType::Retract } +raw_keyword -> Keyword = + keyword_prefix + ns:( sns:$(symbol_namespace) namespace_separator { sns })? + n:$(symbol_name) { + match ns { + Some(ns) => Keyword::namespaced(ns, n), + None => Keyword::plain(n), + } + } + / #expected("keyword") + +raw_forward_keyword -> Keyword + = v:raw_keyword {? if v.is_forward() { Ok(v) } else { Err("expected :forward or :forward/keyword") } } + +raw_backward_keyword -> Keyword + = v:raw_keyword {? if v.is_backward() { Ok(v) } else { Err("expected :_backword or :backward/_keyword") } } + raw_namespaced_keyword -> Keyword = keyword_prefix ns:$(symbol_namespace) namespace_separator n:$(symbol_name) { Keyword::namespaced(ns, n) } / #expected("namespaced keyword") @@ -216,16 +241,20 @@ raw_backward_namespaced_keyword -> Keyword entid -> Entid = v:( raw_basedinteger / raw_hexinteger / raw_octalinteger / raw_integer ) { Entid::Entid(v) } / v:raw_namespaced_keyword { Entid::Ident(v) } + / #expected("entid") forward_entid -> Entid = v:( raw_basedinteger / raw_hexinteger / raw_octalinteger / raw_integer ) { Entid::Entid(v) } / v:raw_forward_namespaced_keyword { Entid::Ident(v) } + / #expected("forward entid") backward_entid -> Entid = v:raw_backward_namespaced_keyword { Entid::Ident(v.to_reversed()) } + / #expected("backward entid") lookup_ref -> LookupRef = "(" __ "lookup-ref" __ a:(entid) __ v:(value) __ ")" { LookupRef { a: AttributePlace::Entid(a), v } } + / #expected("lookup-ref") tx_function -> TxFunction = "(" __ n:$(symbol_name) __ ")" { TxFunction { op: PlainSymbol::plain(n) } } @@ -253,6 +282,210 @@ pub entity -> Entity = __ "[" __ op:(op) __ e:(entity_place) __ a:(forward_entid) __ v:(value_place) __ "]" __ { Entity::AddOrRetract { op, e: e, a: AttributePlace::Entid(a), v: v } } / __ "[" __ op:(op) __ e:(value_place) __ a:(backward_entid) __ v:(entity_place) __ "]" __ { Entity::AddOrRetract { op, e: v, a: AttributePlace::Entid(a), v: e } } / __ map:map_notation __ { Entity::MapNotation(map) } + / #expected("entity") pub entities -> Vec> = __ "[" __ es:(entity*) __ "]" __ { es } + +// Query parser starts here. +// +// We expect every rule except the `raw_*` rules to eat whitespace +// (with `__`) at its start and finish. That means that every string +// pattern (say "[") should be bracketed on either side with either a +// whitespace-eating rule or an explicit whitespace eating `__`. + +query_function -> query::QueryFunction + = __ n:$(symbol_name) __ {? query::QueryFunction::from_symbol(&PlainSymbol::plain(n)).ok_or("expected query function") } + +fn_arg -> query::FnArg + = v:value {? query::FnArg::from_value(&v).ok_or("expected query function argument") } + / __ "[" args:fn_arg+ "]" __ { query::FnArg::Vector(args) } + +find_elem -> query::Element + = __ v:variable __ { query::Element::Variable(v) } + / __ "(" __ "the" v:variable ")" __ { query::Element::Corresponding(v) } + / __ "(" __ "pull" var:variable "[" patterns:pull_attribute+ "]" __ ")" __ { query::Element::Pull(query::Pull { var, patterns }) } + / __ "(" func:query_function args:fn_arg* ")" __ { query::Element::Aggregate(query::Aggregate { func, args }) } + +find_spec -> query::FindSpec + = f:find_elem "." __ { query::FindSpec::FindScalar(f) } + / fs:find_elem+ { query::FindSpec::FindRel(fs) } + / __ "[" f:find_elem __ "..." __ "]" __ { query::FindSpec::FindColl(f) } + / __ "[" fs:find_elem+ "]" __ { query::FindSpec::FindTuple(fs) } + +pull_attribute -> query::PullAttributeSpec + = __ "*" __ { query::PullAttributeSpec::Wildcard } + / __ k:raw_forward_namespaced_keyword __ alias:(":as" __ alias:raw_forward_keyword __ { alias })? { + let attribute = query::PullConcreteAttribute::Ident(::std::rc::Rc::new(k)); + let alias = alias.map(|alias| ::std::rc::Rc::new(alias)); + query::PullAttributeSpec::Attribute( + query::NamedPullAttribute { + attribute, + alias: alias, + }) + } + +limit -> query::Limit + = __ v:variable __ { query::Limit::Variable(v) } + / __ n:(raw_octalinteger / raw_hexinteger / raw_basedinteger / raw_integer) __ {? + if n > 0 { + Ok(query::Limit::Fixed(n as u64)) + } else { + Err("expected positive integer") + } + } + +order -> query::Order + = __ "(" __ "asc" v:variable ")" __ { query::Order(query::Direction::Ascending, v) } + / __ "(" __ "desc" v:variable ")" __ { query::Order(query::Direction::Descending, v) } + / v:variable { query::Order(query::Direction::Ascending, v) } + + +pattern_value_place -> query::PatternValuePlace + = v:value {? query::PatternValuePlace::from_value(&v).ok_or("expected pattern_value_place") } + +pattern_non_value_place -> query::PatternNonValuePlace + = v:value {? query::PatternNonValuePlace::from_value(&v).ok_or("expected pattern_non_value_place") } + +pattern -> query::WhereClause + = __ "[" + src:src_var? + e:pattern_non_value_place + a:pattern_non_value_place + v:pattern_value_place? + tx:pattern_non_value_place? + "]" __ + {? + let v = v.unwrap_or(query::PatternValuePlace::Placeholder); + let tx = tx.unwrap_or(query::PatternNonValuePlace::Placeholder); + + // Pattern::new takes care of reversal of reversed + // attributes: [?x :foo/_bar ?y] turns into + // [?y :foo/bar ?x]. + // + // This is a bit messy: the inner conversion to a Pattern can + // fail if the input is something like + // + // ```edn + // [?x :foo/_reversed 23.4] + // ``` + // + // because + // + // ```edn + // [23.4 :foo/reversed ?x] + // ``` + // + // is nonsense. That leaves us with a nested optional, which we unwrap here. + query::Pattern::new(src, e, a, v, tx) + .map(query::WhereClause::Pattern) + .ok_or("expected pattern") + } + +// TODO: this shouldn't be checked at parse time. +rule_vars -> BTreeSet + = vs:variable+ {? + let given = vs.len(); + let set: BTreeSet = vs.into_iter().collect(); + if given != set.len() { + Err("expected unique variables") + } else { + Ok(set) + } + } + +or_pattern_clause -> query::OrWhereClause + = clause:where_clause { query::OrWhereClause::Clause(clause) } + +or_and_clause -> query::OrWhereClause + = __ "(" __ "and" clauses:where_clause+ ")" __ { query::OrWhereClause::And(clauses) } + +or_where_clause -> query::OrWhereClause + = or_pattern_clause + / or_and_clause + +or_clause -> query::WhereClause + = __ "(" __ "or" clauses:or_where_clause+ ")" __ { + query::WhereClause::OrJoin(query::OrJoin::new(query::UnifyVars::Implicit, clauses)) + } + +or_join_clause -> query::WhereClause + = __ "(" __ "or-join" __ "[" vars:rule_vars "]" clauses:or_where_clause+ ")" __ { + query::WhereClause::OrJoin(query::OrJoin::new(query::UnifyVars::Explicit(vars), clauses)) + } + +not_clause -> query::WhereClause + = __ "(" __ "not" clauses:where_clause+ ")" __ { + query::WhereClause::NotJoin(query::NotJoin::new(query::UnifyVars::Implicit, clauses)) + } + +not_join_clause -> query::WhereClause + = __ "(" __ "not-join" __ "[" vars:rule_vars "]" clauses:where_clause+ ")" __ { + query::WhereClause::NotJoin(query::NotJoin::new(query::UnifyVars::Explicit(vars), clauses)) + } + +type_annotation -> query::WhereClause + = __ "[" __ "(" __ "type" var:variable __ ty:raw_keyword __ ")" __ "]" __ { + query::WhereClause::TypeAnnotation( + query::TypeAnnotation { + value_type: ty, + variable: var, + }) + } + +pred -> query::WhereClause + = __ "[" __ "(" func:query_function args:fn_arg* ")" __ "]" __ { + query::WhereClause::Pred( + query::Predicate { + operator: func.0, + args: args, + }) + } + +pub where_fn -> query::WhereClause + = __ "[" __ "(" func:query_function args:fn_arg* ")" __ binding:binding "]" __ { + query::WhereClause::WhereFn( + query::WhereFn { + operator: func.0, + args: args, + binding, + }) + } + +where_clause -> query::WhereClause + // Right now we only support patterns and predicates. See #239 for more. + = pattern + / or_join_clause + / or_clause + / not_join_clause + / not_clause + / type_annotation + / pred + / where_fn + +query_part -> query::QueryPart + = __ ":find" fs:find_spec { query::QueryPart::FindSpec(fs) } + / __ ":in" in_vars:variable+ { query::QueryPart::InVars(in_vars) } + / __ ":limit" l:limit { query::QueryPart::Limit(l) } + / __ ":order" os:order+ { query::QueryPart::Order(os) } + / __ ":where" ws:where_clause+ { query::QueryPart::WhereClauses(ws) } + / __ ":with" with_vars:variable+ { query::QueryPart::WithVars(with_vars) } + +pub query -> query::ParsedFindQuery + = __ "[" qps:query_part+ "]" __ {? query::ParsedFindQuery::from_parts(qps) } + +variable -> query::Variable + = v:value {? query::Variable::from_value(&v).ok_or("expected variable") } + +src_var -> query::SrcVar + = v:value {? query::SrcVar::from_value(&v).ok_or("expected src_var") } + +variable_or_placeholder -> query::VariableOrPlaceholder + = v:variable { query::VariableOrPlaceholder::Variable(v) } + / __ "_" __ { query::VariableOrPlaceholder::Placeholder } + +binding -> query::Binding + = __ "[" __ "[" vs:variable_or_placeholder+ "]" __ "]" __ { query::Binding::BindRel(vs) } + / __ "[" v:variable "..." __ "]" __ { query::Binding::BindColl(v) } + / __ "[" vs:variable_or_placeholder+ "]" __ { query::Binding::BindTuple(vs) } + / v:variable { query::Binding::BindScalar(v) } diff --git a/edn/src/query.rs b/edn/src/query.rs index c1c69fd9..418dea2e 100644 --- a/edn/src/query.rs +++ b/edn/src/query.rs @@ -949,6 +949,15 @@ pub struct NotJoin { pub clauses: Vec, } +impl NotJoin { + pub fn new(unify_vars: UnifyVars, clauses: Vec) -> NotJoin { + NotJoin { + unify_vars: unify_vars, + clauses: clauses, + } + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub struct TypeAnnotation { pub value_type: Keyword, @@ -981,6 +990,131 @@ pub struct FindQuery { // TODO: in_rules; } +#[allow(dead_code)] +#[derive(Debug, Eq, PartialEq)] +pub struct ParsedFindQuery { + pub find_spec: FindSpec, + pub default_source: SrcVar, + pub with: Vec, + pub in_vars: Vec, + pub in_sources: BTreeSet, + pub limit: Limit, + pub where_clauses: Vec, + pub order: Option>, + // TODO: in_rules; +} + +pub(crate) enum QueryPart { + FindSpec(FindSpec), + WithVars(Vec), + InVars(Vec), + Limit(Limit), + WhereClauses(Vec), + Order(Vec), +} + +impl ParsedFindQuery { + pub(crate) fn from_parts(parts: Vec) -> std::result::Result { + let mut find_spec: Option = None; + let mut with: Option> = None; + let mut in_vars: Option> = None; + let mut limit: Option = None; + let mut where_clauses: Option> = None; + let mut order: Option> = None; + + for part in parts.into_iter() { + match part { + QueryPart::FindSpec(x) => { + if find_spec.is_some() { + return Err("find query has repeated :find"); + } + find_spec = Some(x) + }, + QueryPart::WithVars(x) => { + if with.is_some() { + return Err("find query has repeated :with"); + } + with = Some(x) + }, + QueryPart::InVars(x) => { + if in_vars.is_some() { + return Err("find query has repeated :in"); + } + in_vars = Some(x) + }, + QueryPart::Limit(x) => { + if limit.is_some() { + return Err("find query has repeated :limit"); + } + limit = Some(x) + }, + QueryPart::WhereClauses(x) => { + if where_clauses.is_some() { + return Err("find query has repeated :where"); + } + where_clauses = Some(x) + }, + QueryPart::Order(x) => { + if order.is_some() { + return Err("find query has repeated :order"); + } + order = Some(x) + }, + } + } + + Ok(ParsedFindQuery { + find_spec: find_spec.ok_or("expected :find")?, + default_source: SrcVar::DefaultSrc, + with: with.unwrap_or(vec![]), + in_vars: in_vars.unwrap_or(vec![]), + in_sources: BTreeSet::default(), + limit: limit.unwrap_or(Limit::None), + where_clauses: where_clauses.ok_or("expected :where")?, + order, + }) + } + + + pub fn into_find_query(self: ParsedFindQuery) -> Result { + let in_vars = { + let len = self.in_vars.len(); + let set: BTreeSet = self.in_vars.into_iter().collect(); + if len != set.len() { + return Err("find query has repeated :in variable".into()); + } + set + }; + + let with = { + let len = self.with.len(); + let set: BTreeSet = self.with.into_iter().collect(); + if len != set.len() { + return Err("find query has repeated :with variable".into()); + } + set + }; + + // Make sure that if we have `:limit ?x`, `?x` appears in `:in`. + if let Limit::Variable(ref v) = self.limit { + if !in_vars.contains(v) { + return Err("limit var not present in :in"); + } + } + + Ok(FindQuery { + find_spec: self.find_spec, + default_source: self.default_source, + with, + in_vars, + in_sources: self.in_sources, + limit: self.limit, + where_clauses: self.where_clauses, + order: self.order, + }) + } +} + impl FindQuery { pub fn simple(spec: FindSpec, where_clauses: Vec) -> FindQuery { FindQuery { diff --git a/query-parser/src/lib.rs b/query-parser/src/lib.rs index e5deff3a..2896dc50 100644 --- a/query-parser/src/lib.rs +++ b/query-parser/src/lib.rs @@ -22,7 +22,6 @@ extern crate edn; extern crate mentat_parser_utils; mod errors; -mod parse; pub use errors::{ Error, @@ -31,6 +30,8 @@ pub use errors::{ ResultExt, }; -pub use parse::{ - parse_find_string, -}; +pub fn parse_find_string(string: &str) -> Result { + edn::parse::query(string) + .map_err(|e| e.into()) + .and_then(|parsed| parsed.into_find_query().map_err(|e| e.into())) +} diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs deleted file mode 100644 index 0b11d3bf..00000000 --- a/query-parser/src/parse.rs +++ /dev/null @@ -1,1235 +0,0 @@ -// Copyright 2016 Mozilla -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// Unless required by applicable law or agreed to in writing, software distributed -// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR -// CONDITIONS OF ANY KIND, either express or implied. See the License for the -// specific language governing permissions and limitations under the License. - -extern crate combine; -extern crate edn; -extern crate mentat_parser_utils; -extern crate mentat_query; - -use std; // To refer to std::result::Result. - -use std::collections::BTreeSet; - -use self::combine::{ - eof, - look_ahead, - many, - many1, - optional, - parser, - satisfy, - satisfy_map, - Parser, - ParseResult, - Stream, -}; - -use self::combine::combinator::{any, choice, or, try}; - -use errors::{ - Error, - ErrorKind, - Result, - ResultExt, -}; - -use self::mentat_parser_utils::{ - KeywordMapParser, - ResultParser, - ValueParseError, -}; - -use self::mentat_parser_utils::value_and_span::Stream as ValueStream; -use self::mentat_parser_utils::value_and_span::{ - Item, - OfExactlyParsing, - forward_any_keyword, - forward_namespaced_keyword, - keyword_map, - list, - map, - seq, - vector, -}; - -use self::mentat_query::{ - Aggregate, - Binding, - Direction, - Element, - FindQuery, - FindSpec, - FnArg, - FromValue, - Limit, - Order, - OrJoin, - OrWhereClause, - NamedPullAttribute, - NotJoin, - Pattern, - PatternNonValuePlace, - PatternValuePlace, - Predicate, - Pull, - PullAttributeSpec, - PullConcreteAttribute, - QueryFunction, - SrcVar, - TypeAnnotation, - UnifyVars, - Variable, - VariableOrPlaceholder, - WhereClause, - WhereFn, -}; - -pub struct Query<'a>(std::marker::PhantomData<&'a ()>); - -def_parser!(Query, variable, Variable, { - satisfy_map(Variable::from_value) -}); - -def_parser!(Query, keyword, edn::Keyword, { - satisfy_map(|v: &edn::ValueAndSpan| v.inner.as_keyword().cloned()) -}); - -def_parser!(Query, source_var, SrcVar, { - satisfy_map(SrcVar::from_value) -}); - -// TODO: interning. -def_parser!(Query, query_function, QueryFunction, { - satisfy_map(QueryFunction::from_value) -}); - -def_parser!(Query, fn_arg, FnArg, { - satisfy_map(FnArg::from_value).or(vector().of_exactly(many::, _>(Query::fn_arg())).map(FnArg::Vector)) -}); - -def_parser!(Query, arguments, Vec, { - (many::, _>(Query::fn_arg())) -}); - -def_parser!(Query, direction, Direction, { - satisfy_map(|v: &edn::ValueAndSpan| { - match v.inner { - edn::SpannedValue::PlainSymbol(ref s) => { - let name = s.0.as_str(); - match name { - "asc" => Some(Direction::Ascending), - "desc" => Some(Direction::Descending), - _ => None, - } - }, - _ => None, - } - }) -}); - -def_parser!(Query, order, Order, { - seq().of_exactly((Query::direction(), Query::variable())) - .map(|(d, v)| Order(d, v)) - .or(Query::variable().map(|v| Order(Direction::Ascending, v))) -}); - -def_matches_plain_symbol!(Query, the, "the"); -def_matches_plain_symbol!(Query, pull, "pull"); -def_matches_plain_symbol!(Query, wildcard, "*"); -def_matches_keyword!(Query, alias_as, "as"); - -pub struct Where<'a>(std::marker::PhantomData<&'a ()>); - -def_parser!(Where, pattern_value_place, PatternValuePlace, { - satisfy_map(PatternValuePlace::from_value) -}); - -def_parser!(Query, natural_number, u64, { - any().and_then(|v: &edn::ValueAndSpan| { - match v.inner { - edn::SpannedValue::Integer(x) if (x > 0) => { - Ok(x as u64) - }, - ref spanned => { - let e = Box::new(Error::from_kind(ErrorKind::InvalidLimit(spanned.clone().into()))); - Err(combine::primitives::Error::Other(e)) - }, - } - }) -}); - -def_parser!(Where, pattern_non_value_place, PatternNonValuePlace, { - satisfy_map(PatternNonValuePlace::from_value) -}); - -def_matches_plain_symbol!(Where, and, "and"); - -def_matches_plain_symbol!(Where, or, "or"); - -def_matches_plain_symbol!(Where, or_join, "or-join"); - -def_matches_plain_symbol!(Where, not, "not"); - -def_matches_plain_symbol!(Where, not_join, "not-join"); - -def_matches_plain_symbol!(Where, type_symbol, "type"); - -def_parser!(Where, rule_vars, BTreeSet, { - seq() - .of_exactly(many1(Query::variable()).and_then(unique_vars)) -}); - -def_parser!(Where, or_pattern_clause, OrWhereClause, { - Where::clause().map(|clause| OrWhereClause::Clause(clause)) -}); - -def_parser!(Where, or_and_clause, OrWhereClause, { - seq() - .of_exactly(Where::and() - .with(many1(Where::clause())) - .map(OrWhereClause::And)) -}); - -def_parser!(Where, or_where_clause, OrWhereClause, { - choice([Where::or_pattern_clause(), Where::or_and_clause()]) -}); - -def_parser!(Where, or_clause, WhereClause, { - seq() - .of_exactly(Where::or() - .with(many1(Where::or_where_clause())) - .map(|clauses| { - WhereClause::OrJoin(OrJoin::new(UnifyVars::Implicit, clauses)) - })) -}); - -def_parser!(Where, or_join_clause, WhereClause, { - seq() - .of_exactly(Where::or_join() - .with(Where::rule_vars()) - .and(many1(Where::or_where_clause())) - .map(|(vars, clauses)| { - WhereClause::OrJoin(OrJoin::new(UnifyVars::Explicit(vars), clauses)) - })) -}); - -def_parser!(Where, not_clause, WhereClause, { - seq() - .of_exactly(Where::not() - .with(many1(Where::clause())) - .map(|clauses| { - WhereClause::NotJoin( - NotJoin { - unify_vars: UnifyVars::Implicit, - clauses: clauses, - }) - })) -}); - -def_parser!(Where, not_join_clause, WhereClause, { - seq() - .of_exactly(Where::not_join() - .with(Where::rule_vars()) - .and(many1(Where::clause())) - .map(|(vars, clauses)| { - WhereClause::NotJoin( - NotJoin { - unify_vars: UnifyVars::Explicit(vars), - clauses: clauses, - }) - })) -}); - -def_parser!(Query, func, (QueryFunction, Vec), { - (Query::query_function(), Query::arguments()) -}); - -def_parser!(Query, aggregate, Aggregate, { - Query::func() - .map(|(func, args)| Aggregate { - func, args, - }) -}); - -def_parser!(Query, pull_concrete_attribute_ident, PullConcreteAttribute, { - forward_namespaced_keyword() - .map(|k| PullConcreteAttribute::Ident(::std::rc::Rc::new(k.clone()))) -}); - -def_parser!(Query, pull_concrete_attribute, PullAttributeSpec, { - (Query::pull_concrete_attribute_ident(), - optional(try(Query::alias_as() - .with(forward_any_keyword() - .map(|alias| ::std::rc::Rc::new(alias.clone())))))) - .map(|(attribute, alias)| - PullAttributeSpec::Attribute( - NamedPullAttribute { - attribute, - alias: alias, - })) -}); - -def_parser!(Query, pull_wildcard_attribute, PullAttributeSpec, { - Query::wildcard().map(|_| PullAttributeSpec::Wildcard) -}); - -def_parser!(Query, pull_attribute, PullAttributeSpec, { - choice([ - try(Query::pull_concrete_attribute()), - try(Query::pull_wildcard_attribute()), - // TODO: reversed keywords, entids (with aliases, presumably…). - ]) -}); - -// A wildcard can appear only once. -// If a wildcard appears, only map expressions can be present. -fn validate_attributes<'a, I>(attrs: I) -> std::result::Result<(), &'static str> - where I: IntoIterator { - let mut wildcard_seen = false; - let mut non_map_or_wildcard_seen = false; - for attr in attrs { - match attr { - &PullAttributeSpec::Wildcard => { - if wildcard_seen { - return Err("duplicate wildcard pull attribute"); - } - wildcard_seen = true; - if non_map_or_wildcard_seen { - return Err("wildcard with specified attributes"); - } - }, - // &PullAttributeSpec::LimitedAttribute(_, _) => { - &PullAttributeSpec::Attribute(_) => { - non_map_or_wildcard_seen = true; - if wildcard_seen { - return Err("wildcard with specified attributes"); - } - }, - // TODO: map form. - } - } - Ok(()) -} - -def_parser!(Query, pull_attributes, Vec, { - vector().of_exactly(many1(Query::pull_attribute())) - .and_then(|attrs: Vec| - validate_attributes(&attrs) - .and(Ok(attrs)) - .map_err(|e| combine::primitives::Error::Unexpected(e.into()))) -}); - -/// A vector containing just a parenthesized filter expression. -def_parser!(Where, pred, WhereClause, { - // Accept either a nested list or a nested vector here: - // `[(foo ?x ?y)]` or `[[foo ?x ?y]]` - vector() - .of_exactly(seq() - .of_exactly(Query::func() - .map(|(f, args)| { - WhereClause::Pred( - Predicate { - operator: f.0, - args: args, - }) - }))) -}); - -/// A type annotation. -def_parser!(Where, type_annotation, WhereClause, { - // Accept either a nested list or a nested vector here: - // `[(type ?x :db.type/string)]` or `[[type ?x :db.type/long]]` - vector() - .of_exactly(seq() - .of_exactly((Where::type_symbol(), Query::variable(), Query::keyword()) - .map(|(_, var, ty)| { - WhereClause::TypeAnnotation( - TypeAnnotation { - value_type: ty, - variable: var, - }) - }))) -}); - -/// A vector containing a parenthesized function expression and a binding. -def_parser!(Where, where_fn, WhereClause, { - // Accept either a nested list or a nested vector here: - // `[(foo ?x ?y) binding]` or `[[foo ?x ?y] binding]` - vector() - .of_exactly( - (seq().of_exactly( - Query::func()), - Bind::binding()) - .map(|((f, args), binding)| { - WhereClause::WhereFn( - WhereFn { - operator: f.0, - args: args, - binding: binding, - }) - })) -}); - -def_parser!(Where, pattern, WhereClause, { - vector() - .of_exactly( - // While *technically* Datomic allows you to have a query like: - // [:find … :where [[?x]]] - // We don't -- we require at least e, a. - (optional(Query::source_var()), // src - Where::pattern_non_value_place(), // e - Where::pattern_non_value_place(), // a - optional(Where::pattern_value_place()), // v - optional(Where::pattern_non_value_place())) // tx - .and_then(|(src, e, a, v, tx)| { - let v = v.unwrap_or(PatternValuePlace::Placeholder); - let tx = tx.unwrap_or(PatternNonValuePlace::Placeholder); - - // Pattern::new takes care of reversal of reversed - // attributes: [?x :foo/_bar ?y] turns into - // [?y :foo/bar ?x]. - // - // This is a bit messy: the inner conversion to a Pattern can - // fail if the input is something like - // - // ```edn - // [?x :foo/_reversed 23.4] - // ``` - // - // because - // - // ```edn - // [23.4 :foo/reversed ?x] - // ``` - // - // is nonsense. That leaves us with a nested optional, which we unwrap here. - Pattern::new(src, e, a, v, tx) - .map(WhereClause::Pattern) - .ok_or(combine::primitives::Error::Expected("pattern".into())) - })) -}); - -def_parser!(Where, clause, WhereClause, { - choice([try(Where::pattern()), - // It's either - // (or-join [vars] clauses…) - // or - // (or clauses…) - // We don't yet handle source vars. - try(Where::or_join_clause()), - try(Where::or_clause()), - try(Where::not_join_clause()), - try(Where::not_clause()), - - try(Where::type_annotation()), - try(Where::pred()), - try(Where::where_fn()), - ]) -}); - -def_parser!(Where, clauses, Vec, { - // Right now we only support patterns and predicates. See #239 for more. - (many1::, _>(Where::clause())) -}); - -pub struct Find<'a>(std::marker::PhantomData<&'a ()>); - -def_matches_plain_symbol!(Find, period, "."); - -def_matches_plain_symbol!(Find, ellipsis, "..."); - -def_matches_plain_symbol!(Find, placeholder, "_"); - -def_parser!(Find, variable_element, Element, { - Query::variable().map(Element::Variable) -}); - -def_parser!(Find, corresponding_element, Element, { - Query::the().with(Query::variable()) - .map(Element::Corresponding) -}); - -def_parser!(Find, aggregate_element, Element, { - Query::aggregate().map(Element::Aggregate) -}); - -def_parser!(Find, pull_element, Element, { - Query::pull().with(Query::variable().and(Query::pull_attributes())) - .map(|(var, attrs)| Element::Pull(Pull { var: var, patterns: attrs })) -}); - -enum ElementType { - Corresponding, - Pull, - Aggregate, -} - -def_parser!(Find, seq_elem, Element, { - let element_parser_for_type = |ty: ElementType| { - match ty { - ElementType::Corresponding => Find::corresponding_element(), - ElementType::Pull => Find::pull_element(), - ElementType::Aggregate => Find::aggregate_element(), - } - }; - - // This slightly tortured phrasing ensures that we don't consume - // when the first item in the list -- the function name -- doesn't - // match, but once we decide what the list is, we go ahead and - // commit to that branch. - seq().of_exactly( - // This comes first because otherwise (the ?x) will match as an aggregate. - look_ahead(Query::the()).map(|_| ElementType::Corresponding) - - // Similarly, we have to parse pull before general functions. - .or(look_ahead(Query::pull()).map(|_| ElementType::Pull)) - .or(look_ahead(Query::func()).map(|_| ElementType::Aggregate)) - .then(element_parser_for_type)) -}); - -def_parser!(Find, elem, Element, { - try(Find::variable_element()) - .or(Find::seq_elem()) -}); - -def_parser!(Find, find_scalar, FindSpec, { - Find::elem().skip(Find::period()) - .map(FindSpec::FindScalar) -}); - -def_parser!(Find, find_coll, FindSpec, { - vector().of_exactly(Find::elem().skip(Find::ellipsis())) - .map(FindSpec::FindColl) -}); - -def_parser!(Find, elements, Vec, { - many1::, _>(Find::elem()) -}); - -def_parser!(Find, find_rel, FindSpec, { - Find::elements().map(FindSpec::FindRel) -}); - -def_parser!(Find, find_tuple, FindSpec, { - vector().of_exactly(Find::elements()) - .map(FindSpec::FindTuple) -}); - -/// Parse a stream of values into one of four find specs. -/// -/// `:find` must be an array of plain var symbols (?foo), pull expressions, and aggregates. For now -/// we only support variables and the annotations necessary to declare which flavor of :find we -/// want: -/// -/// -/// `?x ?y ?z ` = FindRel -/// `[?x ...] ` = FindColl -/// `?x . ` = FindScalar -/// `[?x ?y ?z]` = FindTuple -def_parser!(Find, spec, FindSpec, { - // Any one of the four specs might apply, so we combine them with `choice`. Our parsers consume - // input, so we need to wrap them in `try` so that they operate independently. - choice::<[&mut Parser; 4], _> - ([&mut try(Find::find_scalar()), - &mut try(Find::find_coll()), - &mut try(Find::find_tuple()), - &mut try(Find::find_rel())]) -}); - -fn unique_vars(vars: Vec) -> std::result::Result, combine::primitives::Error> { - let given = vars.len(); - let set: BTreeSet = vars.into_iter().collect(); - if given != set.len() { - // TODO: find out what the variable is! - let e = Box::new(Error::from_kind(ErrorKind::DuplicateVariableError)); - Err(combine::primitives::Error::Other(e)) - } else { - Ok(set) - } -} - -def_parser!(Find, vars, BTreeSet, { - many(Query::variable()).and_then(unique_vars) -}); - -/// This is awkward, but will do for now. We use `keyword_map()` to optionally accept vector find -/// queries, then we use `FindQueryPart` to collect parts that have heterogeneous types; and then we -/// construct a `FindQuery` from them. -def_parser!(Find, query, FindQuery, { - let find_map = keyword_map_of!( - ("find", Find::spec()), - ("in", Find::vars()), - ("limit", Query::variable().map(Limit::Variable).or(Query::natural_number().map(Limit::Fixed))), - ("order", many1(Query::order())), - ("where", Where::clauses()), - ("with", Find::vars()) // Note: no trailing comma allowed! - ); - - (or(keyword_map(), vector())) - .of_exactly(find_map) - .and_then(|(find_spec, in_vars, limit, order_clauses, where_clauses, with_vars) | -> std::result::Result> { - let limit = limit.unwrap_or(Limit::None); - - // Make sure that if we have `:limit ?x`, `?x` appears in `:in`. - let in_vars = in_vars.unwrap_or(BTreeSet::default()); - if let Limit::Variable(ref v) = limit { - if !in_vars.contains(v) { - let e = Box::new(Error::from_kind(ErrorKind::UnknownLimitVar(v.name()))); - return Err(combine::primitives::Error::Other(e)); - } - } - - Ok(FindQuery { - default_source: SrcVar::DefaultSrc, - find_spec: find_spec.ok_or(combine::primitives::Error::Unexpected("expected :find".into()))?, - in_sources: BTreeSet::default(), // TODO - in_vars: in_vars, - limit: limit, - order: order_clauses, - where_clauses: where_clauses.ok_or(combine::primitives::Error::Unexpected("expected :where".into()))?, - with: with_vars.unwrap_or(BTreeSet::default()), - }) - }) -}); - -pub struct Bind<'a>(std::marker::PhantomData<&'a ()>); - -def_parser!(Bind, bind_scalar, Binding, { - Query::variable() - .skip(eof()) - .map(|var| Binding::BindScalar(var)) -}); - -def_parser!(Bind, variable_or_placeholder, VariableOrPlaceholder, { - Query::variable().map(VariableOrPlaceholder::Variable) - .or(Find::placeholder().map(|_| VariableOrPlaceholder::Placeholder)) -}); - -def_parser!(Bind, bind_coll, Binding, { - vector() - .of_exactly(Query::variable() - .skip(Find::ellipsis())) - .map(Binding::BindColl) -}); - -def_parser!(Bind, bind_rel, Binding, { - vector().of_exactly( - vector().of_exactly( - many1::, _>(Bind::variable_or_placeholder()) - .map(Binding::BindRel))) -}); - -def_parser!(Bind, bind_tuple, Binding, { - vector().of_exactly( - many1::, _>(Bind::variable_or_placeholder()) - .map(Binding::BindTuple)) -}); - -def_parser!(Bind, binding, Binding, { - // Any one of the four binding types might apply, so we combine them with `choice`. Our parsers - // consume input, so we need to wrap them in `try` so that they operate independently. - choice([try(Bind::bind_scalar()), - try(Bind::bind_coll()), - try(Bind::bind_tuple()), - try(Bind::bind_rel())]) -}); - -pub fn parse_find_string(string: &str) -> Result { - let expr = edn::parse::value(string)?; - Find::query() - .parse(expr.atom_stream()) - .map(|x| x.0) - .map_err(|e| Error::from_kind(ErrorKind::FindParseError(e.into()))) -} - -#[cfg(test)] -mod test { - extern crate combine; - extern crate edn; - extern crate mentat_query; - - use std::rc::Rc; - - use self::combine::Parser; - use self::edn::OrderedFloat; - use self::mentat_query::{ - Binding, - Element, - FindSpec, - NonIntegerConstant, - Pattern, - PatternNonValuePlace, - PatternValuePlace, - SrcVar, - Variable, - VariableOrPlaceholder, - }; - - use super::*; - - fn variable(x: edn::PlainSymbol) -> Variable { - Variable(Rc::new(x)) - } - - fn ident_kw(kw: edn::Keyword) -> PatternNonValuePlace { - PatternNonValuePlace::Ident(kw.into()) - } - - fn ident(ns: &str, name: &str) -> PatternNonValuePlace { - ident_kw(edn::Keyword::namespaced(ns, name)) - } - - #[test] - fn test_pattern_mixed() { - let e = edn::PlainSymbol::plain("_"); - let a = edn::Keyword::namespaced("foo", "bar"); - let v = OrderedFloat(99.9); - let tx = edn::PlainSymbol::plain("?tx"); - let input = edn::Value::Vector(vec!(edn::Value::PlainSymbol(e.clone()), - edn::Value::Keyword(a.clone()), - edn::Value::Float(v.clone()), - edn::Value::PlainSymbol(tx.clone()))); - assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Placeholder, - attribute: ident_kw(a), - value: PatternValuePlace::Constant(NonIntegerConstant::Float(v)), - tx: PatternNonValuePlace::Variable(variable(tx)), - })); - } - - #[test] - fn test_pattern_vars() { - let s = edn::PlainSymbol::plain("$x"); - let e = edn::PlainSymbol::plain("?e"); - let a = edn::PlainSymbol::plain("?a"); - let v = edn::PlainSymbol::plain("?v"); - let tx = edn::PlainSymbol::plain("?tx"); - let input = edn::Value::Vector(vec!(edn::Value::PlainSymbol(s.clone()), - edn::Value::PlainSymbol(e.clone()), - edn::Value::PlainSymbol(a.clone()), - edn::Value::PlainSymbol(v.clone()), - edn::Value::PlainSymbol(tx.clone()))); - assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { - source: Some(SrcVar::NamedSrc("x".to_string())), - entity: PatternNonValuePlace::Variable(variable(e)), - attribute: PatternNonValuePlace::Variable(variable(a)), - value: PatternValuePlace::Variable(variable(v)), - tx: PatternNonValuePlace::Variable(variable(tx)), - })); - } - - #[test] - fn test_pattern_reversed_invalid() { - let e = edn::PlainSymbol::plain("_"); - let a = edn::Keyword::namespaced("foo", "_bar"); - let v = OrderedFloat(99.9); - let tx = edn::PlainSymbol::plain("?tx"); - let input = edn::Value::Vector(vec!(edn::Value::PlainSymbol(e.clone()), - edn::Value::Keyword(a.clone()), - edn::Value::Float(v.clone()), - edn::Value::PlainSymbol(tx.clone()))); - - let input = input.with_spans(); - let mut par = Where::pattern(); - let result = par.parse(input.atom_stream()); - match result { - Err(_) => (), - _ => assert!(false, "Expected a parse error"), - } - } - - #[test] - fn test_pattern_reversed() { - let e = edn::PlainSymbol::plain("_"); - let a = edn::Keyword::namespaced("foo", "_bar"); - let v = edn::PlainSymbol::plain("?v"); - let tx = edn::PlainSymbol::plain("?tx"); - let input = edn::Value::Vector(vec!(edn::Value::PlainSymbol(e.clone()), - edn::Value::Keyword(a.clone()), - edn::Value::PlainSymbol(v.clone()), - edn::Value::PlainSymbol(tx.clone()))); - - // Note that the attribute is no longer reversed, and the entity and value have - // switched places. - assert_parses_to!(Where::pattern, input, WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(variable(v)), - attribute: ident("foo", "bar"), - value: PatternValuePlace::Placeholder, - tx: PatternNonValuePlace::Variable(variable(tx)), - })); - } - - #[test] - fn test_rule_vars() { - let e = edn::PlainSymbol::plain("?e"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone())]); - assert_parses_to!(Where::rule_vars, input, - btreeset!{variable(e.clone())}); - } - - #[test] - fn test_repeated_vars() { - let e = edn::PlainSymbol::plain("?e"); - let f = edn::PlainSymbol::plain("?f"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone()), - edn::Value::PlainSymbol(f.clone()),]); - assert_parses_to!(|| vector().of_exactly(Find::vars()), input, - btreeset!{variable(e.clone()), variable(f.clone())}); - - let g = edn::PlainSymbol::plain("?g"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(g.clone()), - edn::Value::PlainSymbol(g.clone()),]); - - let input = input.with_spans(); - let mut par = vector().of_exactly(Find::vars()); - let result = par.parse(input.atom_stream()) - .map(|x| x.0) - .map_err(|e| if let Some(combine::primitives::Error::Other(x)) = e.errors.into_iter().next() { - // Pattern matching on boxes is rocket science until Rust Nightly features hit - // stable. ErrorKind isn't Clone, so convert to strings. We could pattern match - // for exact comparison here. - x.downcast::().ok().map(|e| e.to_string()) - } else { - None - }); - assert_eq!(result, Err(Some("duplicates in variable list".to_string()))); - } - - #[test] - fn test_or() { - let oj = edn::PlainSymbol::plain("or"); - let e = edn::PlainSymbol::plain("?e"); - let a = edn::PlainSymbol::plain("?a"); - let v = edn::PlainSymbol::plain("?v"); - let input = edn::Value::List( - vec![edn::Value::PlainSymbol(oj), - edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone()), - edn::Value::PlainSymbol(a.clone()), - edn::Value::PlainSymbol(v.clone())])].into_iter().collect()); - assert_parses_to!(Where::or_clause, input, - WhereClause::OrJoin( - OrJoin::new(UnifyVars::Implicit, - vec![OrWhereClause::Clause( - WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(variable(e)), - attribute: PatternNonValuePlace::Variable(variable(a)), - value: PatternValuePlace::Variable(variable(v)), - tx: PatternNonValuePlace::Placeholder, - }))]))); - } - - #[test] - fn test_or_join() { - let oj = edn::PlainSymbol::plain("or-join"); - let e = edn::PlainSymbol::plain("?e"); - let a = edn::PlainSymbol::plain("?a"); - let v = edn::PlainSymbol::plain("?v"); - let input = edn::Value::List( - vec![edn::Value::PlainSymbol(oj), - edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone())]), - edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone()), - edn::Value::PlainSymbol(a.clone()), - edn::Value::PlainSymbol(v.clone())])].into_iter().collect()); - assert_parses_to!(Where::or_join_clause, input, - WhereClause::OrJoin( - OrJoin::new(UnifyVars::Explicit(btreeset!{variable(e.clone())}), - vec![OrWhereClause::Clause( - WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(variable(e)), - attribute: PatternNonValuePlace::Variable(variable(a)), - value: PatternValuePlace::Variable(variable(v)), - tx: PatternNonValuePlace::Placeholder, - }))]))); - } - - #[test] - fn test_not() { - let e = edn::PlainSymbol::plain("?e"); - let a = edn::PlainSymbol::plain("?a"); - let v = edn::PlainSymbol::plain("?v"); - - assert_edn_parses_to!(Where::not_clause, - "(not [?e ?a ?v])", - WhereClause::NotJoin( - NotJoin { - unify_vars: UnifyVars::Implicit, - clauses: vec![ - WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(variable(e)), - attribute: PatternNonValuePlace::Variable(variable(a)), - value: PatternValuePlace::Variable(variable(v)), - tx: PatternNonValuePlace::Placeholder, - })], - })); - } - - #[test] - fn test_not_join() { - let e = edn::PlainSymbol::plain("?e"); - let a = edn::PlainSymbol::plain("?a"); - let v = edn::PlainSymbol::plain("?v"); - - assert_edn_parses_to!(Where::not_join_clause, - "(not-join [?e] [?e ?a ?v])", - WhereClause::NotJoin( - NotJoin { - unify_vars: UnifyVars::Explicit(btreeset!{variable(e.clone())}), - clauses: vec![WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(variable(e)), - attribute: PatternNonValuePlace::Variable(variable(a)), - value: PatternValuePlace::Variable(variable(v)), - tx: PatternNonValuePlace::Placeholder, - })], - })); - } - - #[test] - fn test_find_sp_variable() { - let sym = edn::PlainSymbol::plain("?x"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(sym.clone())]); - assert_parses_to!(|| vector().of_exactly(Query::variable()), input, variable(sym)); - } - - #[test] - fn test_find_scalar() { - let sym = edn::PlainSymbol::plain("?x"); - let period = edn::PlainSymbol::plain("."); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(sym.clone()), edn::Value::PlainSymbol(period.clone())]); - assert_parses_to!(|| vector().of_exactly(Find::find_scalar()), - input, - FindSpec::FindScalar(Element::Variable(variable(sym)))); - } - - #[test] - fn test_find_coll() { - let sym = edn::PlainSymbol::plain("?x"); - let period = edn::PlainSymbol::plain("..."); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(sym.clone()), - edn::Value::PlainSymbol(period.clone())]); - assert_parses_to!(Find::find_coll, - input, - FindSpec::FindColl(Element::Variable(variable(sym)))); - } - - #[test] - fn test_find_rel() { - let vx = edn::PlainSymbol::plain("?x"); - let vy = edn::PlainSymbol::plain("?y"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(vx.clone()), edn::Value::PlainSymbol(vy.clone())]); - assert_parses_to!(|| vector().of_exactly(Find::find_rel()), - input, - FindSpec::FindRel(vec![Element::Variable(variable(vx)), - Element::Variable(variable(vy))])); - } - - #[test] - fn test_find_tuple() { - let vx = edn::PlainSymbol::plain("?x"); - let vy = edn::PlainSymbol::plain("?y"); - let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(vx.clone()), - edn::Value::PlainSymbol(vy.clone())]); - assert_parses_to!(Find::find_tuple, - input, - FindSpec::FindTuple(vec![Element::Variable(variable(vx)), - Element::Variable(variable(vy))])); - } - - #[test] - fn test_natural_numbers() { - let text = edn::Value::Text("foo".to_string()); - let neg = edn::Value::Integer(-10); - let zero = edn::Value::Integer(0); - let pos = edn::Value::Integer(5); - - // This is terrible, but destructuring errors is frustrating. - let input = text.with_spans(); - let mut par = Query::natural_number(); - let x = par.parse(input.atom_stream()).err().expect("an error").errors; - let result = format!("{:?}", x); - assert_eq!(result, "[Other(Error(InvalidLimit(Text(\"foo\")), State { next_error: None, backtrace: None }))]"); - - let input = neg.with_spans(); - let mut par = Query::natural_number(); - let x = par.parse(input.atom_stream()).err().expect("an error").errors; - let result = format!("{:?}", x); - assert_eq!(result, "[Other(Error(InvalidLimit(Integer(-10)), State { next_error: None, backtrace: None }))]"); - - let input = zero.with_spans(); - let mut par = Query::natural_number(); - let x = par.parse(input.atom_stream()).err().expect("an error").errors; - let result = format!("{:?}", x); - assert_eq!(result, "[Other(Error(InvalidLimit(Integer(0)), State { next_error: None, backtrace: None }))]"); - - let input = pos.with_spans(); - let mut par = Query::natural_number(); - assert_eq!(None, par.parse(input.atom_stream()).err()); - } - - #[test] - fn test_fn_arg_collections() { - let vx = edn::PlainSymbol::plain("?x"); - let vy = edn::PlainSymbol::plain("?y"); - let input = edn::Value::Vector(vec![edn::Value::Vector(vec![edn::Value::PlainSymbol(vx.clone()), - edn::Value::PlainSymbol(vy.clone())])]); - - assert_parses_to!(|| vector().of_exactly(Query::fn_arg()), - input, - FnArg::Vector(vec![FnArg::Variable(variable(vx)), - FnArg::Variable(variable(vy)), - ])); - } - - #[test] - fn test_bind_scalar() { - let vx = edn::PlainSymbol::plain("?x"); - assert_edn_parses_to!(|| list().of_exactly(Bind::binding()), - "(?x)", - Binding::BindScalar(variable(vx))); - } - - #[test] - fn test_bind_coll() { - let vx = edn::PlainSymbol::plain("?x"); - assert_edn_parses_to!(|| list().of_exactly(Bind::binding()), - "([?x ...])", - Binding::BindColl(variable(vx))); - } - - #[test] - fn test_bind_rel() { - let vx = edn::PlainSymbol::plain("?x"); - let vy = edn::PlainSymbol::plain("?y"); - let vw = edn::PlainSymbol::plain("?w"); - assert_edn_parses_to!(|| list().of_exactly(Bind::binding()), - "([[?x ?y _ ?w]])", - Binding::BindRel(vec![VariableOrPlaceholder::Variable(variable(vx)), - VariableOrPlaceholder::Variable(variable(vy)), - VariableOrPlaceholder::Placeholder, - VariableOrPlaceholder::Variable(variable(vw)), - ])); - } - - #[test] - fn test_bind_tuple() { - let vx = edn::PlainSymbol::plain("?x"); - let vy = edn::PlainSymbol::plain("?y"); - let vw = edn::PlainSymbol::plain("?w"); - assert_edn_parses_to!(|| list().of_exactly(Bind::binding()), - "([?x ?y _ ?w])", - Binding::BindTuple(vec![VariableOrPlaceholder::Variable(variable(vx)), - VariableOrPlaceholder::Variable(variable(vy)), - VariableOrPlaceholder::Placeholder, - VariableOrPlaceholder::Variable(variable(vw)), - ])); - } - - #[test] - fn test_the() { - assert_edn_parses_to!(Find::seq_elem, - "(the ?y)", - Element::Corresponding(Variable::from_valid_name("?y"))); - assert_edn_parses_to!(Find::find_tuple, - "[(the ?x) ?y]", - FindSpec::FindTuple(vec![Element::Corresponding(Variable::from_valid_name("?x")), - Element::Variable(Variable::from_valid_name("?y"))])); - assert_edn_parses_to!(Find::spec, - "[(the ?x) ?y]", - FindSpec::FindTuple(vec![Element::Corresponding(Variable::from_valid_name("?x")), - Element::Variable(Variable::from_valid_name("?y"))])); - let expected_query = - FindQuery { - find_spec: FindSpec::FindTuple(vec![Element::Corresponding(Variable::from_valid_name("?x")), - Element::Variable(Variable::from_valid_name("?y"))]), - where_clauses: vec![ - WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), - attribute: PatternNonValuePlace::Placeholder, - value: PatternValuePlace::Variable(Variable::from_valid_name("?y")), - tx: PatternNonValuePlace::Placeholder, - })], - - default_source: SrcVar::DefaultSrc, - with: Default::default(), - in_vars: Default::default(), - in_sources: Default::default(), - limit: Limit::None, - order: None, - }; - assert_edn_parses_to!(Find::query, - "[:find [(the ?x) ?y] - :where [?x _ ?y]]", - expected_query); - - // If we give a malformed pull expression, we don't fall through to aggregates. - assert_parse_failure_contains!(Find::elem, - "(pull x [])", - r#"errors: [Unexpected(Token(ValueAndSpan { inner: PlainSymbol(PlainSymbol("x")), span: Span(6, 7) })), Expected(Borrowed("variable"))]"#); - } - - #[test] - fn test_where_fn() { - assert_edn_parses_to!(Where::where_fn, - "[(f ?x 1) ?y]", - WhereClause::WhereFn(WhereFn { - operator: edn::PlainSymbol::plain("f"), - args: vec![FnArg::Variable(Variable::from_valid_name("?x")), - FnArg::EntidOrInteger(1)], - binding: Binding::BindScalar(Variable::from_valid_name("?y")), - })); - - assert_edn_parses_to!(Where::where_fn, - "[(f ?x) [?y ...]]", - WhereClause::WhereFn(WhereFn { - operator: edn::PlainSymbol::plain("f"), - args: vec![FnArg::Variable(Variable::from_valid_name("?x"))], - binding: Binding::BindColl(Variable::from_valid_name("?y")), - })); - - assert_edn_parses_to!(Where::where_fn, - "[(f) [?y _]]", - WhereClause::WhereFn(WhereFn { - operator: edn::PlainSymbol::plain("f"), - args: vec![], - binding: Binding::BindTuple(vec![VariableOrPlaceholder::Variable(Variable::from_valid_name("?y")), - VariableOrPlaceholder::Placeholder]), - })); - - assert_edn_parses_to!(Where::where_fn, - "[(f) [[_ ?y]]]", - WhereClause::WhereFn(WhereFn { - operator: edn::PlainSymbol::plain("f"), - args: vec![], - binding: Binding::BindRel(vec![VariableOrPlaceholder::Placeholder, - VariableOrPlaceholder::Variable(Variable::from_valid_name("?y"))]), - })); - } - - #[test] - fn test_type_anno() { - assert_edn_parses_to!(Where::type_annotation, - "[(type ?x :db.type/string)]", - WhereClause::TypeAnnotation(TypeAnnotation { - value_type: edn::Keyword::namespaced("db.type", "string"), - variable: Variable::from_valid_name("?x"), - })); - // We don't check for valid types, or even that the type is namespaced. - assert_edn_parses_to!(Where::clause, - "[[type ?foo :db_type_long]]", - WhereClause::TypeAnnotation(TypeAnnotation { - value_type: edn::Keyword::plain("db_type_long"), - variable: Variable::from_valid_name("?foo"), - })); - - } - - #[test] - fn test_pull() { - assert_edn_parses_to!(Query::pull_attribute, - "*", - PullAttributeSpec::Wildcard); - assert_edn_parses_to!(Query::pull_attributes, - "[*]", - vec![PullAttributeSpec::Wildcard]); - assert_edn_parses_to!(Find::elem, - "(pull ?v [*])", - Element::Pull(Pull { - var: Variable::from_valid_name("?v"), - patterns: vec![PullAttributeSpec::Wildcard], - })); - - let foo_bar = ::std::rc::Rc::new(edn::Keyword::namespaced("foo", "bar")); - let foo_baz = ::std::rc::Rc::new(edn::Keyword::namespaced("foo", "baz")); - let foo_horse = ::std::rc::Rc::new(edn::Keyword::namespaced("foo", "horse")); - let horse = ::std::rc::Rc::new(edn::Keyword::plain("horse")); - assert_edn_parses_to!(Query::pull_concrete_attribute, - ":foo/bar", - PullAttributeSpec::Attribute( - PullConcreteAttribute::Ident(foo_bar.clone()).into())); - assert_edn_parses_to!(Query::pull_attribute, - ":foo/bar", - PullAttributeSpec::Attribute( - PullConcreteAttribute::Ident(foo_bar.clone()).into())); - assert_edn_parses_to!(Find::elem, - "(pull ?v [:foo/bar :as :foo/horse, :foo/baz])", - Element::Pull(Pull { - var: Variable::from_valid_name("?v"), - patterns: vec![ - PullAttributeSpec::Attribute( - NamedPullAttribute { - attribute: PullConcreteAttribute::Ident(foo_bar.clone()), - alias: Some(foo_horse), - }), - PullAttributeSpec::Attribute( - PullConcreteAttribute::Ident(foo_baz.clone()).into()), - ], - })); - assert_edn_parses_to!(Find::elem, - "(pull ?v [:foo/bar :as :horse, :foo/baz])", - Element::Pull(Pull { - var: Variable::from_valid_name("?v"), - patterns: vec![ - PullAttributeSpec::Attribute( - NamedPullAttribute { - attribute: PullConcreteAttribute::Ident(foo_bar.clone()), - alias: Some(horse), - }), - PullAttributeSpec::Attribute( - PullConcreteAttribute::Ident(foo_baz.clone()).into()), - ], - })); - assert_parse_failure_contains!(Find::elem, - "(pull ?x [* :foo/bar])", - r#"errors: [Unexpected(Borrowed("wildcard with specified attributes"))]"#); - } - - #[test] - fn test_query_with_pull() { - let q = "[:find ?x (pull ?x [:foo/bar]) :where [?x _ _]]"; - let expected_query = - FindQuery { - find_spec: FindSpec::FindRel(vec![ - Element::Variable(Variable::from_valid_name("?x")), - Element::Pull(Pull { - var: Variable::from_valid_name("?x"), - patterns: vec![ - PullAttributeSpec::Attribute( - PullConcreteAttribute::Ident( - ::std::rc::Rc::new(edn::Keyword::namespaced("foo", "bar")) - ).into() - ), - ] })]), - where_clauses: vec![ - WhereClause::Pattern(Pattern { - source: None, - entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), - attribute: PatternNonValuePlace::Placeholder, - value: PatternValuePlace::Placeholder, - tx: PatternNonValuePlace::Placeholder, - })], - - default_source: SrcVar::DefaultSrc, - with: Default::default(), - in_vars: Default::default(), - in_sources: Default::default(), - limit: Limit::None, - order: None, - }; - assert_edn_parses_to!(Find::query, - q, - expected_query); - } -} From 47441f56dc19c0c3963f80360dd85676f0576a4b Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 31 May 2018 14:10:49 -0700 Subject: [PATCH 10/13] Part 5: Push FindQuery into query-algebrizer; structure errors. This is a big deck-chair re-arrangement. This puts FindQuery into query-algebrizer and puts the validation from ParsedFindQuery -> FindQuery their as well. Some tests were re-homed for this. In addition, the little-used maplit crate dependency was replaced with inline expressions. --- core/src/lib.rs | 4 + edn/src/query.rs | 74 ++---------------- .../find_tests.rs => edn/tests/query_tests.rs | 54 ++++++------- query-algebrizer/Cargo.toml | 1 - query-algebrizer/src/clauses/not.rs | 3 +- query-algebrizer/src/clauses/or.rs | 5 +- query-algebrizer/src/clauses/pattern.rs | 9 +-- query-algebrizer/src/errors.rs | 15 ++++ query-algebrizer/src/lib.rs | 76 +++++++++++++++++-- query-algebrizer/src/types.rs | 34 +++++++-- query-algebrizer/src/validate.rs | 14 ++-- query-algebrizer/tests/utils/mod.rs | 5 +- query-parser/Cargo.toml | 1 - query-parser/src/lib.rs | 9 --- query-projector/tests/aggregates.rs | 5 +- query-translator/tests/translate.rs | 2 +- src/query.rs | 9 +-- 17 files changed, 164 insertions(+), 156 deletions(-) rename query-parser/tests/find_tests.rs => edn/tests/query_tests.rs (88%) diff --git a/core/src/lib.rs b/core/src/lib.rs index f13341b3..b9646ed9 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -46,6 +46,10 @@ pub use edn::{ Utc, ValueRc, }; +pub use edn::parse::{ + query as parse_query, + ParseError as EdnParseError, +}; pub use cache::{ CachedAttributes, diff --git a/edn/src/query.rs b/edn/src/query.rs index 418dea2e..0baa48e9 100644 --- a/edn/src/query.rs +++ b/edn/src/query.rs @@ -976,20 +976,6 @@ pub enum WhereClause { TypeAnnotation(TypeAnnotation), } -#[allow(dead_code)] -#[derive(Debug, Eq, PartialEq)] -pub struct FindQuery { - pub find_spec: FindSpec, - pub default_source: SrcVar, - pub with: BTreeSet, - pub in_vars: BTreeSet, - pub in_sources: BTreeSet, - pub limit: Limit, - pub where_clauses: Vec, - pub order: Option>, - // TODO: in_rules; -} - #[allow(dead_code)] #[derive(Debug, Eq, PartialEq)] pub struct ParsedFindQuery { @@ -1013,6 +999,12 @@ pub(crate) enum QueryPart { Order(Vec), } +/// A `ParsedFindQuery` represents a parsed but potentially invalid query to the query algebrizer. +/// Such a query is syntactically valid but might be semantically invalid, for example because +/// constraints on the set of variables are not respected. +/// +/// We split `ParsedFindQuery` from `FindQuery` because it's not easy to generalize over containers +/// (here, `Vec` and `BTreeSet`) in Rust. impl ParsedFindQuery { pub(crate) fn from_parts(parts: Vec) -> std::result::Result { let mut find_spec: Option = None; @@ -1074,60 +1066,6 @@ impl ParsedFindQuery { order, }) } - - - pub fn into_find_query(self: ParsedFindQuery) -> Result { - let in_vars = { - let len = self.in_vars.len(); - let set: BTreeSet = self.in_vars.into_iter().collect(); - if len != set.len() { - return Err("find query has repeated :in variable".into()); - } - set - }; - - let with = { - let len = self.with.len(); - let set: BTreeSet = self.with.into_iter().collect(); - if len != set.len() { - return Err("find query has repeated :with variable".into()); - } - set - }; - - // Make sure that if we have `:limit ?x`, `?x` appears in `:in`. - if let Limit::Variable(ref v) = self.limit { - if !in_vars.contains(v) { - return Err("limit var not present in :in"); - } - } - - Ok(FindQuery { - find_spec: self.find_spec, - default_source: self.default_source, - with, - in_vars, - in_sources: self.in_sources, - limit: self.limit, - where_clauses: self.where_clauses, - order: self.order, - }) - } -} - -impl FindQuery { - pub fn simple(spec: FindSpec, where_clauses: Vec) -> FindQuery { - FindQuery { - find_spec: spec, - default_source: SrcVar::DefaultSrc, - with: BTreeSet::default(), - in_vars: BTreeSet::default(), - in_sources: BTreeSet::default(), - limit: Limit::None, - where_clauses: where_clauses, - order: None, - } - } } impl OrJoin { diff --git a/query-parser/tests/find_tests.rs b/edn/tests/query_tests.rs similarity index 88% rename from query-parser/tests/find_tests.rs rename to edn/tests/query_tests.rs index 95e5bc66..da6d54b6 100644 --- a/query-parser/tests/find_tests.rs +++ b/edn/tests/query_tests.rs @@ -8,19 +8,14 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -#[macro_use] -extern crate maplit; - extern crate edn; -extern crate mentat_query; -extern crate mentat_query_parser; use edn::{ Keyword, PlainSymbol, }; -use mentat_query::{ +use edn::query::{ Direction, Element, FindSpec, @@ -39,7 +34,9 @@ use mentat_query::{ WhereClause, }; -use mentat_query_parser::parse_find_string; +use edn::parse::{ + query as parse_query, +}; ///! N.B., parsing a query can be done without reference to a DB. ///! Processing the parsed query into something we can work with @@ -49,7 +46,7 @@ use mentat_query_parser::parse_find_string; #[test] fn can_parse_predicates() { let s = "[:find [?x ...] :where [?x _ ?y] [(< ?y 10)]]"; - let p = parse_find_string(s).unwrap(); + let p = parse_query(s).unwrap(); assert_eq!(p.find_spec, FindSpec::FindColl(Element::Variable(Variable::from_valid_name("?x")))); @@ -71,7 +68,7 @@ fn can_parse_predicates() { #[test] fn can_parse_simple_or() { let s = "[:find ?x . :where (or [?x _ 10] [?x _ 15])]"; - let p = parse_find_string(s).unwrap(); + let p = parse_query(s).unwrap(); assert_eq!(p.find_spec, FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); @@ -104,14 +101,14 @@ fn can_parse_simple_or() { #[test] fn can_parse_unit_or_join() { let s = "[:find ?x . :where (or-join [?x] [?x _ 15])]"; - let p = parse_find_string(s).expect("to be able to parse find"); + let p = parse_query(s).expect("to be able to parse find"); assert_eq!(p.find_spec, FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin::new( - UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?x")}), + UnifyVars::Explicit(std::iter::once(Variable::from_valid_name("?x")).collect()), vec![ OrWhereClause::Clause( WhereClause::Pattern(Pattern { @@ -129,14 +126,14 @@ fn can_parse_unit_or_join() { #[test] fn can_parse_simple_or_join() { let s = "[:find ?x . :where (or-join [?x] [?x _ 10] [?x _ -15])]"; - let p = parse_find_string(s).unwrap(); + let p = parse_query(s).unwrap(); assert_eq!(p.find_spec, FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); assert_eq!(p.where_clauses, vec![ WhereClause::OrJoin(OrJoin::new( - UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?x")}), + UnifyVars::Explicit(std::iter::once(Variable::from_valid_name("?x")).collect()), vec![ OrWhereClause::Clause( WhereClause::Pattern(Pattern { @@ -167,7 +164,7 @@ fn ident(ns: &str, name: &str) -> PatternNonValuePlace { #[test] fn can_parse_simple_or_and_join() { let s = "[:find ?x . :where (or [?x _ 10] (and (or [?x :foo/bar ?y] [?x :foo/baz ?y]) [(< ?y 1)]))]"; - let p = parse_find_string(s).unwrap(); + let p = parse_query(s).unwrap(); assert_eq!(p.find_spec, FindSpec::FindScalar(Element::Variable(Variable::from_valid_name("?x")))); @@ -219,23 +216,23 @@ fn can_parse_simple_or_and_join() { #[test] fn can_parse_order_by() { let invalid = "[:find ?x :where [?x :foo/baz ?y] :order]"; - assert!(parse_find_string(invalid).is_err()); + assert!(parse_query(invalid).is_err()); // Defaults to ascending. let default = "[:find ?x :where [?x :foo/baz ?y] :order ?y]"; - assert_eq!(parse_find_string(default).unwrap().order, + assert_eq!(parse_query(default).unwrap().order, Some(vec![Order(Direction::Ascending, Variable::from_valid_name("?y"))])); let ascending = "[:find ?x :where [?x :foo/baz ?y] :order (asc ?y)]"; - assert_eq!(parse_find_string(ascending).unwrap().order, + assert_eq!(parse_query(ascending).unwrap().order, Some(vec![Order(Direction::Ascending, Variable::from_valid_name("?y"))])); let descending = "[:find ?x :where [?x :foo/baz ?y] :order (desc ?y)]"; - assert_eq!(parse_find_string(descending).unwrap().order, + assert_eq!(parse_query(descending).unwrap().order, Some(vec![Order(Direction::Descending, Variable::from_valid_name("?y"))])); let mixed = "[:find ?x :where [?x :foo/baz ?y] :order (desc ?y) (asc ?x)]"; - assert_eq!(parse_find_string(mixed).unwrap().order, + assert_eq!(parse_query(mixed).unwrap().order, Some(vec![Order(Direction::Descending, Variable::from_valid_name("?y")), Order(Direction::Ascending, Variable::from_valid_name("?x"))])); } @@ -243,40 +240,37 @@ fn can_parse_order_by() { #[test] fn can_parse_limit() { let invalid = "[:find ?x :where [?x :foo/baz ?y] :limit]"; - assert!(parse_find_string(invalid).is_err()); + assert!(parse_query(invalid).is_err()); let zero_invalid = "[:find ?x :where [?x :foo/baz ?y] :limit 00]"; - assert!(parse_find_string(zero_invalid).is_err()); + assert!(parse_query(zero_invalid).is_err()); let none = "[:find ?x :where [?x :foo/baz ?y]]"; - assert_eq!(parse_find_string(none).unwrap().limit, + assert_eq!(parse_query(none).unwrap().limit, Limit::None); let one = "[:find ?x :where [?x :foo/baz ?y] :limit 1]"; - assert_eq!(parse_find_string(one).unwrap().limit, + assert_eq!(parse_query(one).unwrap().limit, Limit::Fixed(1)); let onethousand = "[:find ?x :where [?x :foo/baz ?y] :limit 1000]"; - assert_eq!(parse_find_string(onethousand).unwrap().limit, + assert_eq!(parse_query(onethousand).unwrap().limit, Limit::Fixed(1000)); let variable_with_in = "[:find ?x :in ?limit :where [?x :foo/baz ?y] :limit ?limit]"; - assert_eq!(parse_find_string(variable_with_in).unwrap().limit, + assert_eq!(parse_query(variable_with_in).unwrap().limit, Limit::Variable(Variable::from_valid_name("?limit"))); let variable_with_in_used = "[:find ?x :in ?limit :where [?x :foo/baz ?limit] :limit ?limit]"; - assert_eq!(parse_find_string(variable_with_in_used).unwrap().limit, + assert_eq!(parse_query(variable_with_in_used).unwrap().limit, Limit::Variable(Variable::from_valid_name("?limit"))); - - let variable_without_in = "[:find ?x :where [?x :foo/baz ?y] :limit ?limit]"; - assert!(parse_find_string(variable_without_in).is_err()); } #[test] fn can_parse_uuid() { let expected = edn::Uuid::parse_str("4cb3f828-752d-497a-90c9-b1fd516d5644").expect("valid uuid"); let s = "[:find ?x :where [?x :foo/baz #uuid \"4cb3f828-752d-497a-90c9-b1fd516d5644\"]]"; - assert_eq!(parse_find_string(s).expect("parsed").where_clauses.pop().expect("a where clause"), + assert_eq!(parse_query(s).expect("parsed").where_clauses.pop().expect("a where clause"), WhereClause::Pattern( Pattern::new(None, PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), diff --git a/query-algebrizer/Cargo.toml b/query-algebrizer/Cargo.toml index 310af20d..ef1b0965 100644 --- a/query-algebrizer/Cargo.toml +++ b/query-algebrizer/Cargo.toml @@ -18,4 +18,3 @@ path = "../query-parser" [dev-dependencies] itertools = "0.7" -maplit = "0.1" diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 43c0a901..dfddd879 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -106,8 +106,6 @@ mod testing { Variable }; - use self::mentat_query_parser::parse_find_string; - use clauses::{ QueryInputs, add_attribute, @@ -135,6 +133,7 @@ mod testing { use { algebrize, algebrize_with_inputs, + parse_find_string, }; fn alg(schema: &Schema, input: &str) -> ConjoiningClauses { diff --git a/query-algebrizer/src/clauses/or.rs b/query-algebrizer/src/clauses/or.rs index e680a783..ad6662d3 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -767,10 +767,6 @@ mod testing { Variable, }; - use self::mentat_query_parser::{ - parse_find_string, - }; - use clauses::{ add_attribute, associate_ident, @@ -789,6 +785,7 @@ mod testing { use { algebrize, algebrize_with_counter, + parse_find_string, }; fn alg(known: Known, input: &str) -> ConjoiningClauses { diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index dd4561a4..d713ea7c 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -670,10 +670,6 @@ mod testing { Variable, }; - use self::mentat_query_parser::{ - parse_find_string, - }; - use clauses::{ QueryInputs, add_attribute, @@ -690,7 +686,10 @@ mod testing { SourceAlias, }; - use algebrize; + use { + algebrize, + parse_find_string, + }; fn alg(schema: &Schema, input: &str) -> ConjoiningClauses { let parsed = parse_find_string(input).expect("parse failed"); diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index ba1986a2..6e2e1154 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -11,6 +11,7 @@ extern crate mentat_query; use mentat_core::{ + EdnParseError, ValueType, ValueTypeSet, }; @@ -44,6 +45,10 @@ error_chain! { Error, ErrorKind, ResultExt, Result; } + foreign_links { + EdnParseError(EdnParseError); + } + errors { UnsupportedArgument { description("unexpected FnArg") @@ -117,6 +122,16 @@ error_chain! { description("non-matching variables in 'not' clause") display("non-matching variables in 'not' clause") } + + DuplicateVariableError(name: PlainSymbol, clause: &'static str) { + description("duplicate variables") + display("{} var {} is duplicated", clause, name) + } + + UnknownLimitVar(name: PlainSymbol) { + description(":limit var not present in :in") + display(":limit var {} not present in :in", name) + } } } diff --git a/query-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 6414e71b..80c89824 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -13,10 +13,6 @@ #[macro_use] extern crate error_chain; -#[cfg(test)] -#[macro_use] -extern crate maplit; - extern crate mentat_core; extern crate mentat_query; @@ -35,18 +31,20 @@ use mentat_core::{ Schema, TypedValue, ValueType, + parse_query, }; use mentat_core::counter::RcCounter; use mentat_query::{ Element, - FindQuery, FindSpec, Limit, Order, + ParsedFindQuery, SrcVar, Variable, + WhereClause, }; pub use errors::{ @@ -63,6 +61,7 @@ pub use clauses::{ pub use types::{ EmptyBecause, + FindQuery, }; /// A convenience wrapper around things known in memory: the schema and caches. @@ -336,3 +335,70 @@ pub use types::{ TableAlias, VariableColumn, }; + + +impl FindQuery { + pub fn simple(spec: FindSpec, where_clauses: Vec) -> FindQuery { + FindQuery { + find_spec: spec, + default_source: SrcVar::DefaultSrc, + with: BTreeSet::default(), + in_vars: BTreeSet::default(), + in_sources: BTreeSet::default(), + limit: Limit::None, + where_clauses: where_clauses, + order: None, + } + } + + pub fn from_parsed_find_query(parsed: ParsedFindQuery) -> Result { + let in_vars = { + let mut set: BTreeSet = BTreeSet::default(); + + for var in parsed.in_vars.into_iter() { + if !set.insert(var.clone()) { + bail!(ErrorKind::DuplicateVariableError(var.name(), ":in")); + } + } + + set + }; + + let with = { + let mut set: BTreeSet = BTreeSet::default(); + + for var in parsed.with.into_iter() { + if !set.insert(var.clone()) { + bail!(ErrorKind::DuplicateVariableError(var.name(), ":with")); + } + } + + set + }; + + // Make sure that if we have `:limit ?x`, `?x` appears in `:in`. + if let Limit::Variable(ref v) = parsed.limit { + if !in_vars.contains(v) { + bail!(ErrorKind::UnknownLimitVar(v.name())); + } + } + + Ok(FindQuery { + find_spec: parsed.find_spec, + default_source: parsed.default_source, + with, + in_vars, + in_sources: parsed.in_sources, + limit: parsed.limit, + where_clauses: parsed.where_clauses, + order: parsed.order, + }) + } +} + +pub fn parse_find_string(string: &str) -> Result { + parse_query(string) + // .and_then( + .map_err(|e| e.into()) + .and_then(|parsed| FindQuery::from_parsed_find_query(parsed)) // .map_err(|e| e.into())) +} diff --git a/query-algebrizer/src/types.rs b/query-algebrizer/src/types.rs index d9690859..397170e6 100644 --- a/query-algebrizer/src/types.rs +++ b/query-algebrizer/src/types.rs @@ -12,7 +12,6 @@ use std::collections::BTreeSet; use std::fmt::{ Debug, Formatter, - Result, }; use mentat_core::{ @@ -25,10 +24,13 @@ use mentat_core::{ use mentat_query::{ Direction, + FindSpec, Keyword, + Limit, Order, SrcVar, Variable, + WhereClause, }; /// This enum models the fixed set of default tables we have -- two @@ -174,7 +176,7 @@ impl ColumnName for VariableColumn { } impl Debug for VariableColumn { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { match self { // These should agree with VariableColumn::column_name. &VariableColumn::Variable(ref v) => write!(f, "{}", v.as_str()), @@ -184,13 +186,13 @@ impl Debug for VariableColumn { } impl Debug for DatomsColumn { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { write!(f, "{}", self.as_str()) } } impl Debug for Column { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { match self { &Column::Fixed(ref c) => c.fmt(f), &Column::Fulltext(ref c) => c.fmt(f), @@ -217,7 +219,7 @@ impl ColumnName for FulltextColumn { } impl Debug for FulltextColumn { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { write!(f, "{}", self.as_str()) } } @@ -251,7 +253,7 @@ impl ColumnName for TransactionsColumn { } impl Debug for TransactionsColumn { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { write!(f, "{}", self.as_str()) } } @@ -264,7 +266,7 @@ pub type TableAlias = String; pub struct SourceAlias(pub DatomsTable, pub TableAlias); impl Debug for SourceAlias { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { write!(f, "SourceAlias({:?}, {})", self.0, self.1) } } @@ -274,7 +276,7 @@ impl Debug for SourceAlias { pub struct QualifiedAlias(pub TableAlias, pub Column); impl Debug for QualifiedAlias { - fn fmt(&self, f: &mut Formatter) -> Result { + fn fmt(&self, f: &mut Formatter) -> ::std::fmt::Result { write!(f, "{}.{:?}", self.0, self.1) } } @@ -691,6 +693,22 @@ impl Debug for EmptyBecause { } } + +/// A `FindQuery` represents a valid query to the query algebrizer. +#[allow(dead_code)] +#[derive(Debug, Eq, PartialEq)] +pub struct FindQuery { + pub find_spec: FindSpec, + pub default_source: SrcVar, + pub with: BTreeSet, + pub in_vars: BTreeSet, + pub in_sources: BTreeSet, + pub limit: Limit, + pub where_clauses: Vec, + pub order: Option>, + // TODO: in_rules; +} + // Intermediate data structures for resolving patterns. #[derive(Debug, Eq, PartialEq)] diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index ddae881f..44c4c131 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -99,7 +99,6 @@ mod tests { extern crate mentat_query_parser; use self::mentat_query::{ - FindQuery, Keyword, OrWhereClause, Pattern, @@ -110,13 +109,12 @@ mod tests { WhereClause, }; - use self::mentat_query_parser::parse_find_string; - use clauses::ident; - use super::{ - validate_not_join, - validate_or_join, + use super::*; + use parse_find_string; + use types::{ + FindQuery, }; fn value_ident(ns: &str, name: &str) -> PatternValuePlace { @@ -212,7 +210,7 @@ mod tests { (and [?artist :artist/type ?type] [?type :artist/role :artist.role/parody]))]"#; let parsed = parse_find_string(query).expect("expected successful parse"); - let clauses = valid_or_join(parsed, UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?artist")})); + let clauses = valid_or_join(parsed, UnifyVars::Explicit(::std::iter::once(Variable::from_valid_name("?artist")).collect())); // Let's do some detailed parse checks. let mut arms = clauses.into_iter(); @@ -322,7 +320,7 @@ mod tests { [?release :release/artists ?artist] [?release :release/year 1970])]"#; let parsed = parse_find_string(query).expect("expected successful parse"); - let clauses = valid_not_join(parsed, UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?artist")})); + let clauses = valid_not_join(parsed, UnifyVars::Explicit(::std::iter::once(Variable::from_valid_name("?artist")).collect())); let release = PatternNonValuePlace::Variable(Variable::from_valid_name("?release")); let artist = PatternValuePlace::Variable(Variable::from_valid_name("?artist")); diff --git a/query-algebrizer/tests/utils/mod.rs b/query-algebrizer/tests/utils/mod.rs index a643c556..9ca8185d 100644 --- a/query-algebrizer/tests/utils/mod.rs +++ b/query-algebrizer/tests/utils/mod.rs @@ -20,10 +20,6 @@ use mentat_core::{ ValueType, }; -use mentat_query_parser::{ - parse_find_string, -}; - use mentat_query::{ Keyword, }; @@ -35,6 +31,7 @@ use mentat_query_algebrizer::{ QueryInputs, algebrize, algebrize_with_inputs, + parse_find_string, }; // Common utility functions used in multiple test files. diff --git a/query-parser/Cargo.toml b/query-parser/Cargo.toml index 2a91dbc4..ffaa753b 100644 --- a/query-parser/Cargo.toml +++ b/query-parser/Cargo.toml @@ -6,7 +6,6 @@ workspace = ".." [dependencies] combine = "2.3.2" error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } -maplit = "0.1" [dependencies.edn] path = "../edn" diff --git a/query-parser/src/lib.rs b/query-parser/src/lib.rs index 2896dc50..c9a12fa7 100644 --- a/query-parser/src/lib.rs +++ b/query-parser/src/lib.rs @@ -10,9 +10,6 @@ #![allow(unused_imports)] -#[macro_use] -extern crate maplit; - #[macro_use] extern crate error_chain; @@ -29,9 +26,3 @@ pub use errors::{ Result, ResultExt, }; - -pub fn parse_find_string(string: &str) -> Result { - edn::parse::query(string) - .map_err(|e| e.into()) - .and_then(|parsed| parsed.into_find_query().map_err(|e| e.into())) -} diff --git a/query-projector/tests/aggregates.rs b/query-projector/tests/aggregates.rs index c1313ea6..0a17ee55 100644 --- a/query-projector/tests/aggregates.rs +++ b/query-projector/tests/aggregates.rs @@ -21,10 +21,6 @@ use mentat_core::{ ValueType, }; -use mentat_query_parser::{ - parse_find_string, -}; - use mentat_query::{ Keyword, }; @@ -32,6 +28,7 @@ use mentat_query::{ use mentat_query_algebrizer::{ Known, algebrize, + parse_find_string, }; use mentat_query_projector::{ diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index 3fa6ffe6..d80e06bf 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -34,12 +34,12 @@ use mentat_core::{ ValueType, }; -use mentat_query_parser::parse_find_string; use mentat_query_algebrizer::{ Known, QueryInputs, algebrize, algebrize_with_inputs, + parse_find_string, }; use mentat_query_projector::{ diff --git a/src/query.rs b/src/query.rs index a0cbd2f8..77322472 100644 --- a/src/query.rs +++ b/src/query.rs @@ -14,18 +14,20 @@ use rusqlite::types::ToSql; use std::rc::Rc; use mentat_core::{ + Binding, Entid, HasSchema, KnownEntid, Schema, - Binding, TypedValue, }; use mentat_query_algebrizer::{ AlgebraicQuery, EmptyBecause, + FindQuery, algebrize_with_inputs, + parse_find_string, }; pub use mentat_query_algebrizer::{ @@ -40,7 +42,6 @@ pub use mentat_query::{ use mentat_query::{ Element, - FindQuery, FindSpec, Pattern, PatternNonValuePlace, @@ -48,10 +49,6 @@ use mentat_query::{ WhereClause, }; -use mentat_query_parser::{ - parse_find_string, -}; - use mentat_query_projector::{ ConstantProjector, Projector, From d4166cc67c1a9bd2b3a35a67e8c5540ba515eb96 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 31 May 2018 15:02:32 -0700 Subject: [PATCH 11/13] Part 6: Remove query-parser entirely. --- Cargo.toml | 3 -- README.md | 10 ++--- query-algebrizer/Cargo.toml | 4 -- query-algebrizer/src/clauses/not.rs | 2 - query-algebrizer/src/clauses/or.rs | 2 - query-algebrizer/src/clauses/pattern.rs | 2 - query-algebrizer/src/validate.rs | 1 - query-algebrizer/tests/fulltext.rs | 1 - query-algebrizer/tests/ground.rs | 1 - query-algebrizer/tests/predicate.rs | 1 - query-algebrizer/tests/type_reqs.rs | 1 - query-parser/Cargo.toml | 17 --------- query-parser/README.md | 2 - query-parser/src/errors.rs | 49 ------------------------- query-parser/src/lib.rs | 28 -------------- query-projector/Cargo.toml | 4 -- query-projector/tests/aggregates.rs | 1 - query-pull/Cargo.toml | 4 -- query-sql/Cargo.toml | 4 -- query-translator/Cargo.toml | 4 -- query-translator/tests/translate.rs | 1 - src/errors.rs | 2 - src/lib.rs | 1 - 23 files changed, 3 insertions(+), 142 deletions(-) delete mode 100644 query-parser/Cargo.toml delete mode 100644 query-parser/README.md delete mode 100644 query-parser/src/errors.rs delete mode 100644 query-parser/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index a072e4bb..f9c43124 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,9 +58,6 @@ path = "query" [dependencies.mentat_query_algebrizer] path = "query-algebrizer" -[dependencies.mentat_query_parser] -path = "query-parser" - [dependencies.mentat_query_projector] path = "query-projector" diff --git a/README.md b/README.md index 720ce17d..0acc6822 100644 --- a/README.md +++ b/README.md @@ -133,12 +133,12 @@ To run tests use: # Run tests for everything. cargo test --all -# Run tests for just the query-parser folder (specify the crate, not the folder), +# Run tests for just the query-algebrizer folder (specify the crate, not the folder), # printing debug output. -cargo test -p mentat_query_parser -- --nocapture +cargo test -p mentat_query_algebrizer -- --nocapture ```` -For most `cargo` commands you can pass the `-p` argument to run the command just on that package. So, `cargo build -p mentat_query_parser` will build just the "query-parser" folder. +For most `cargo` commands you can pass the `-p` argument to run the command just on that package. So, `cargo build -p mentat_query_algebrizer` will build just the "query-algebrizer" folder. ## What are all of these crates? @@ -189,10 +189,6 @@ Mentat has two main inputs: reads (queries) and writes (transacts). Just as `men ### Query processing -#### `mentat_query_parser` - -This is a `combine` parser that uses `mentat_parser_utils` and `mentat_query` to turn a stream of EDN values into a more usable representation of a query. - #### `mentat_query_algebrizer` This is the biggest piece of the query engine. It takes a parsed query, which at this point is _independent of a database_, and combines it with the current state of the schema and data. This involves translating keywords into attributes, abstract values into concrete values with a known type, and producing an `AlgebraicQuery`, which is a representation of how a query's Datalog semantics can be satisfied as SQL table joins and constraints over Mentat's SQL schema. An algebrized query is tightly coupled with both the disk schema and the vocabulary present in the store when the work is done. diff --git a/query-algebrizer/Cargo.toml b/query-algebrizer/Cargo.toml index ef1b0965..d544a29d 100644 --- a/query-algebrizer/Cargo.toml +++ b/query-algebrizer/Cargo.toml @@ -12,9 +12,5 @@ path = "../core" [dependencies.mentat_query] path = "../query" -# Only for tests. -[dev-dependencies.mentat_query_parser] -path = "../query-parser" - [dev-dependencies] itertools = "0.7" diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index dfddd879..299c62ec 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -86,8 +86,6 @@ impl ConjoiningClauses { #[cfg(test)] mod testing { - extern crate mentat_query_parser; - use std::collections::BTreeSet; use super::*; diff --git a/query-algebrizer/src/clauses/or.rs b/query-algebrizer/src/clauses/or.rs index ad6662d3..53918e52 100644 --- a/query-algebrizer/src/clauses/or.rs +++ b/query-algebrizer/src/clauses/or.rs @@ -751,8 +751,6 @@ fn union_types(into: &mut BTreeMap, #[cfg(test)] mod testing { - extern crate mentat_query_parser; - use super::*; use mentat_core::{ diff --git a/query-algebrizer/src/clauses/pattern.rs b/query-algebrizer/src/clauses/pattern.rs index d713ea7c..7cf62c6a 100644 --- a/query-algebrizer/src/clauses/pattern.rs +++ b/query-algebrizer/src/clauses/pattern.rs @@ -651,8 +651,6 @@ impl ConjoiningClauses { #[cfg(test)] mod testing { - extern crate mentat_query_parser; - use super::*; use std::collections::BTreeMap; diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index 44c4c131..5bac7fde 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -96,7 +96,6 @@ pub(crate) fn validate_not_join(not_join: &NotJoin) -> Result<()> { mod tests { extern crate mentat_core; extern crate mentat_query; - extern crate mentat_query_parser; use self::mentat_query::{ Keyword, diff --git a/query-algebrizer/tests/fulltext.rs b/query-algebrizer/tests/fulltext.rs index 083f7121..02bc493f 100644 --- a/query-algebrizer/tests/fulltext.rs +++ b/query-algebrizer/tests/fulltext.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; mod utils; diff --git a/query-algebrizer/tests/ground.rs b/query-algebrizer/tests/ground.rs index a07ed081..b7e8341f 100644 --- a/query-algebrizer/tests/ground.rs +++ b/query-algebrizer/tests/ground.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; mod utils; diff --git a/query-algebrizer/tests/predicate.rs b/query-algebrizer/tests/predicate.rs index 7482deed..6aa0d929 100644 --- a/query-algebrizer/tests/predicate.rs +++ b/query-algebrizer/tests/predicate.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; mod utils; diff --git a/query-algebrizer/tests/type_reqs.rs b/query-algebrizer/tests/type_reqs.rs index a8f5179b..b2cd66a1 100644 --- a/query-algebrizer/tests/type_reqs.rs +++ b/query-algebrizer/tests/type_reqs.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; mod utils; diff --git a/query-parser/Cargo.toml b/query-parser/Cargo.toml deleted file mode 100644 index ffaa753b..00000000 --- a/query-parser/Cargo.toml +++ /dev/null @@ -1,17 +0,0 @@ -[package] -name = "mentat_query_parser" -version = "0.0.1" -workspace = ".." - -[dependencies] -combine = "2.3.2" -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } - -[dependencies.edn] - path = "../edn" - -[dependencies.mentat_parser_utils] - path = "../parser-utils" - -[dependencies.mentat_query] - path = "../query" diff --git a/query-parser/README.md b/query-parser/README.md deleted file mode 100644 index feb48f62..00000000 --- a/query-parser/README.md +++ /dev/null @@ -1,2 +0,0 @@ -See for a description of -what's going on in this crate, as well as `query` and `query-executor`. diff --git a/query-parser/src/errors.rs b/query-parser/src/errors.rs deleted file mode 100644 index 990112ba..00000000 --- a/query-parser/src/errors.rs +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2018 Mozilla -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// Unless required by applicable law or agreed to in writing, software distributed -// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR -// CONDITIONS OF ANY KIND, either express or implied. See the License for the -// specific language governing permissions and limitations under the License. - -#![allow(dead_code)] - -use edn; - -use mentat_parser_utils::{ - ValueParseError, -}; - -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - foreign_links { - EdnParseError(edn::parse::ParseError); - } - - errors { - DuplicateVariableError { - description("duplicate variable") - display("duplicates in variable list") - } - - FindParseError(e: ValueParseError) { - description(":find parse error") - display(":find parse error") - } - - UnknownLimitVar(var: edn::PlainSymbol) { - description("limit var not present in :in") - display("limit var {} not present in :in", var) - } - - InvalidLimit(val: edn::Value) { - description("limit value not valid") - display("expected natural number, got {}", val) - } - } -} diff --git a/query-parser/src/lib.rs b/query-parser/src/lib.rs deleted file mode 100644 index c9a12fa7..00000000 --- a/query-parser/src/lib.rs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2016 Mozilla -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// Unless required by applicable law or agreed to in writing, software distributed -// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR -// CONDITIONS OF ANY KIND, either express or implied. See the License for the -// specific language governing permissions and limitations under the License. - -#![allow(unused_imports)] - -#[macro_use] -extern crate error_chain; - -extern crate edn; - -#[macro_use] -extern crate mentat_parser_utils; - -mod errors; - -pub use errors::{ - Error, - ErrorKind, - Result, - ResultExt, -}; diff --git a/query-projector/Cargo.toml b/query-projector/Cargo.toml index e5e2563e..1bdcc4c1 100644 --- a/query-projector/Cargo.toml +++ b/query-projector/Cargo.toml @@ -29,10 +29,6 @@ path = "../query-algebrizer" [dependencies.mentat_query_pull] path = "../query-pull" -# Only for tests. -[dev-dependencies.mentat_query_parser] -path = "../query-parser" - [dependencies.mentat_query_sql] path = "../query-sql" diff --git a/query-projector/tests/aggregates.rs b/query-projector/tests/aggregates.rs index 0a17ee55..6b148ed3 100644 --- a/query-projector/tests/aggregates.rs +++ b/query-projector/tests/aggregates.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; extern crate mentat_query_projector; use mentat_core::{ diff --git a/query-pull/Cargo.toml b/query-pull/Cargo.toml index 9b89a6af..cc5e8b68 100644 --- a/query-pull/Cargo.toml +++ b/query-pull/Cargo.toml @@ -27,7 +27,3 @@ path = "../query-sql" [dependencies.mentat_sql] path = "../sql" - -# Only for tests. -[dev-dependencies.mentat_query_parser] -path = "../query-parser" diff --git a/query-sql/Cargo.toml b/query-sql/Cargo.toml index 7b951f11..583612e2 100644 --- a/query-sql/Cargo.toml +++ b/query-sql/Cargo.toml @@ -17,7 +17,3 @@ path = "../query" [dependencies.mentat_query_algebrizer] path = "../query-algebrizer" - -# Only for tests. -[dev-dependencies.mentat_query_parser] -path = "../query-parser" diff --git a/query-translator/Cargo.toml b/query-translator/Cargo.toml index 1edc8728..b729a12d 100644 --- a/query-translator/Cargo.toml +++ b/query-translator/Cargo.toml @@ -18,10 +18,6 @@ path = "../query" [dependencies.mentat_query_algebrizer] path = "../query-algebrizer" -# Only for tests. -[dev-dependencies.mentat_query_parser] -path = "../query-parser" - [dependencies.mentat_query_projector] path = "../query-projector" diff --git a/query-translator/tests/translate.rs b/query-translator/tests/translate.rs index d80e06bf..30306904 100644 --- a/query-translator/tests/translate.rs +++ b/query-translator/tests/translate.rs @@ -11,7 +11,6 @@ extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; extern crate mentat_query_projector; extern crate mentat_query_translator; extern crate mentat_sql; diff --git a/src/errors.rs b/src/errors.rs index bcf340c6..126faa00 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -24,7 +24,6 @@ use mentat_core::{ use mentat_db; use mentat_query; use mentat_query_algebrizer; -use mentat_query_parser; use mentat_query_projector; use mentat_query_pull; use mentat_query_translator; @@ -46,7 +45,6 @@ error_chain! { links { DbError(mentat_db::Error, mentat_db::ErrorKind); QueryError(mentat_query_algebrizer::Error, mentat_query_algebrizer::ErrorKind); // Let's not leak the term 'algebrizer'. - QueryParseError(mentat_query_parser::Error, mentat_query_parser::ErrorKind); ProjectorError(mentat_query_projector::errors::Error, mentat_query_projector::errors::ErrorKind); PullError(mentat_query_pull::errors::Error, mentat_query_pull::errors::ErrorKind); TranslatorError(mentat_query_translator::Error, mentat_query_translator::ErrorKind); diff --git a/src/lib.rs b/src/lib.rs index d73b9529..77e4ec32 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,7 +25,6 @@ extern crate mentat_core; extern crate mentat_db; extern crate mentat_query; extern crate mentat_query_algebrizer; -extern crate mentat_query_parser; extern crate mentat_query_projector; extern crate mentat_query_pull; extern crate mentat_query_translator; From e68cc4016c8147f59769d769981f083bc53c9260 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Thu, 31 May 2018 15:10:49 -0700 Subject: [PATCH 12/13] Part 7: Remove tx entirely. This was left over from #681. --- Cargo.toml | 3 --- README.md | 4 ---- db/Cargo.toml | 3 --- db/src/bootstrap.rs | 2 +- db/src/cache.rs | 2 +- db/src/db.rs | 2 +- db/src/debug.rs | 4 +++- db/src/errors.rs | 2 +- db/src/internal_types.rs | 4 ++-- db/src/lib.rs | 1 - db/src/tx.rs | 4 ++-- db/src/tx_observer.rs | 2 +- db/src/types.rs | 2 +- db/src/upsert_resolution.rs | 2 +- db/src/watcher.rs | 2 +- src/entity_builder.rs | 4 ++-- src/lib.rs | 1 - tx/Cargo.toml | 8 -------- tx/README.md | 1 - tx/src/lib.rs | 13 ------------- 20 files changed, 17 insertions(+), 49 deletions(-) delete mode 100644 tx/Cargo.toml delete mode 100644 tx/README.md delete mode 100644 tx/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index f9c43124..9f86a5de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,9 +70,6 @@ path = "query-sql" [dependencies.mentat_query_translator] path = "query-translator" -[dependencies.mentat_tx] -path = "tx" - [dependencies.mentat_tolstoy] path = "tolstoy" diff --git a/README.md b/README.md index 0acc6822..c25e2914 100644 --- a/README.md +++ b/README.md @@ -183,10 +183,6 @@ This crate defines the structs and enums that are the output of the query parser Similarly, this crate defines an abstract representation of a SQL query as understood by Mentat. This bridges between Mentat's types (_e.g._, `TypedValue`) and SQL concepts (`ColumnOrExpression`, `GroupBy`). It's produced by the algebrizer and consumed by the translator. -#### `mentat_tx` - -Mentat has two main inputs: reads (queries) and writes (transacts). Just as `mentat_query` defines the types produced by the query parser, `mentat_tx` defines the types produced by the tx parser. - ### Query processing #### `mentat_query_algebrizer` diff --git a/db/Cargo.toml b/db/Cargo.toml index 76b2d5c1..668da59b 100644 --- a/db/Cargo.toml +++ b/db/Cargo.toml @@ -27,9 +27,6 @@ path = "../core" [dependencies.mentat_sql] path = "../sql" -[dependencies.mentat_tx] -path = "../tx" - # Should be dev-dependencies. [dependencies.tabwriter] version = "1.0.3" diff --git a/db/src/bootstrap.rs b/db/src/bootstrap.rs index bc910d41..d133f26f 100644 --- a/db/src/bootstrap.rs +++ b/db/src/bootstrap.rs @@ -16,7 +16,7 @@ use edn::types::Value; use edn::symbols; use entids; use db::TypedSQLValue; -use mentat_tx::entities::Entity; +use edn::entities::Entity; use mentat_core::{ IdentMap, Schema, diff --git a/db/src/cache.rs b/db/src/cache.rs index 8a8a2c40..dad957bd 100644 --- a/db/src/cache.rs +++ b/db/src/cache.rs @@ -97,7 +97,7 @@ use mentat_sql::{ SQLQuery, }; -use mentat_tx::entities::{ +use edn::entities::{ OpType, }; diff --git a/db/src/db.rs b/db/src/db.rs index 17b74938..de2ea52d 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -1096,7 +1096,7 @@ mod tests { InternSet, }; use mentat_core::util::Either::*; - use mentat_tx::entities::{ + use edn::entities::{ OpType, TempId, }; diff --git a/db/src/debug.rs b/db/src/debug.rs index 70e9b1d2..a8e330b9 100644 --- a/db/src/debug.rs +++ b/db/src/debug.rs @@ -31,7 +31,9 @@ use mentat_core::{ TypedValue, ValueType, }; -use mentat_tx::entities::{Entid}; +use edn::entities::{ + Entid, +}; use schema::{ SchemaBuilding, }; diff --git a/db/src/errors.rs b/db/src/errors.rs index 7dfca817..d198081b 100644 --- a/db/src/errors.rs +++ b/db/src/errors.rs @@ -17,7 +17,7 @@ use std::collections::{ use rusqlite; -use mentat_tx::entities::{ +use edn::entities::{ TempId, }; use mentat_core::{ diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 306cb04a..34f73d09 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -47,8 +47,8 @@ use types::{ TypedValue, ValueType, }; -use mentat_tx::entities; -use mentat_tx::entities::{ +use edn::entities; +use edn::entities::{ EntityPlace, OpType, TempId, diff --git a/db/src/lib.rs b/db/src/lib.rs index 57be7490..39ec8820 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -26,7 +26,6 @@ extern crate time; #[macro_use] extern crate edn; #[macro_use] extern crate mentat_core; extern crate mentat_sql; -extern crate mentat_tx; use std::iter::repeat; diff --git a/db/src/tx.rs b/db/src/tx.rs index 769f7d55..028e0652 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -103,8 +103,8 @@ use mentat_core::{ use mentat_core::intern_set::InternSet; -use mentat_tx::entities as entmod; -use mentat_tx::entities::{ +use edn::entities as entmod; +use edn::entities::{ AttributePlace, Entity, OpType, diff --git a/db/src/tx_observer.rs b/db/src/tx_observer.rs index 565a0736..10c49bb8 100644 --- a/db/src/tx_observer.rs +++ b/db/src/tx_observer.rs @@ -32,7 +32,7 @@ use mentat_core::{ TypedValue, }; -use mentat_tx::entities::{ +use edn::entities::{ OpType, }; diff --git a/db/src/types.rs b/db/src/types.rs index af5c7f9f..70ea4964 100644 --- a/db/src/types.rs +++ b/db/src/types.rs @@ -29,7 +29,7 @@ pub use self::mentat_core::{ ValueType, }; -use mentat_tx::entities::{ +use edn::entities::{ EntityPlace, TempId, }; diff --git a/db/src/upsert_resolution.rs b/db/src/upsert_resolution.rs index 7e748757..5f90821c 100644 --- a/db/src/upsert_resolution.rs +++ b/db/src/upsert_resolution.rs @@ -45,7 +45,7 @@ use mentat_core::{ Schema, TypedValue, }; -use mentat_tx::entities::OpType; +use edn::entities::OpType; use schema::SchemaBuilding; /// A "Simple upsert" that looks like [:db/add TEMPID a v], where a is :db.unique/identity. diff --git a/db/src/watcher.rs b/db/src/watcher.rs index 15d8b3b7..d646fbde 100644 --- a/db/src/watcher.rs +++ b/db/src/watcher.rs @@ -23,7 +23,7 @@ use mentat_core::{ TypedValue, }; -use mentat_tx::entities::{ +use edn::entities::{ OpType, }; diff --git a/src/entity_builder.rs b/src/entity_builder.rs index 13516cdc..263ed604 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -15,7 +15,7 @@ // The internal data format for transacting is required to encode the complexities of // processing that format: temporary IDs, lookup refs, input spans, etc. // -// See mentat_tx::entities::Entity and all of its child enums to see how complex this gets. +// See edn::entities::Entity and all of its child enums to see how complex this gets. // // A programmatic consumer doesn't want to build something that looks like: // @@ -75,7 +75,7 @@ use mentat_db::internal_types::{ TypedValueOr, }; -use mentat_tx::entities::{ +use edn::entities::{ OpType, TempId, }; diff --git a/src/lib.rs b/src/lib.rs index 77e4ec32..7d783ea8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,7 +30,6 @@ extern crate mentat_query_pull; extern crate mentat_query_translator; extern crate mentat_sql; extern crate mentat_tolstoy; -extern crate mentat_tx; pub use mentat_core::{ Attribute, diff --git a/tx/Cargo.toml b/tx/Cargo.toml deleted file mode 100644 index cb93106a..00000000 --- a/tx/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "mentat_tx" -version = "0.0.1" -workspace = ".." - -[dependencies] -[dependencies.edn] - path = "../edn" diff --git a/tx/README.md b/tx/README.md deleted file mode 100644 index fc8f6166..00000000 --- a/tx/README.md +++ /dev/null @@ -1 +0,0 @@ -This sub-crate implements the core types used by the transaction processor. diff --git a/tx/src/lib.rs b/tx/src/lib.rs deleted file mode 100644 index ad503e05..00000000 --- a/tx/src/lib.rs +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2016 Mozilla -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// Unless required by applicable law or agreed to in writing, software distributed -// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR -// CONDITIONS OF ANY KIND, either express or implied. See the License for the -// specific language governing permissions and limitations under the License. - -#[allow(unused_imports)] -#[macro_use] extern crate edn; -pub use edn::entities; From cfed968514c75eff058e85ef96a391eda14f9cb3 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Mon, 4 Jun 2018 15:21:27 -0700 Subject: [PATCH 13/13] Review comments. --- core/src/lib.rs | 3 ++- edn/src/edn.rustpeg | 4 ++-- edn/src/query.rs | 13 ++++++------- edn/tests/query_tests.rs | 21 ++++++++++++++++++++- query-algebrizer/src/lib.rs | 7 +++---- query-algebrizer/src/types.rs | 4 +++- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index b9646ed9..5ec155f4 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -46,8 +46,9 @@ pub use edn::{ Utc, ValueRc, }; + pub use edn::parse::{ - query as parse_query, + parse_query, ParseError as EdnParseError, }; diff --git a/edn/src/edn.rustpeg b/edn/src/edn.rustpeg index 5b77672d..48759f07 100644 --- a/edn/src/edn.rustpeg +++ b/edn/src/edn.rustpeg @@ -471,8 +471,8 @@ query_part -> query::QueryPart / __ ":where" ws:where_clause+ { query::QueryPart::WhereClauses(ws) } / __ ":with" with_vars:variable+ { query::QueryPart::WithVars(with_vars) } -pub query -> query::ParsedFindQuery - = __ "[" qps:query_part+ "]" __ {? query::ParsedFindQuery::from_parts(qps) } +pub parse_query -> query::ParsedQuery + = __ "[" qps:query_part+ "]" __ {? query::ParsedQuery::from_parts(qps) } variable -> query::Variable = v:value {? query::Variable::from_value(&v).ok_or("expected variable") } diff --git a/edn/src/query.rs b/edn/src/query.rs index 0baa48e9..ca8ec76b 100644 --- a/edn/src/query.rs +++ b/edn/src/query.rs @@ -978,7 +978,7 @@ pub enum WhereClause { #[allow(dead_code)] #[derive(Debug, Eq, PartialEq)] -pub struct ParsedFindQuery { +pub struct ParsedQuery { pub find_spec: FindSpec, pub default_source: SrcVar, pub with: Vec, @@ -987,7 +987,6 @@ pub struct ParsedFindQuery { pub limit: Limit, pub where_clauses: Vec, pub order: Option>, - // TODO: in_rules; } pub(crate) enum QueryPart { @@ -999,14 +998,14 @@ pub(crate) enum QueryPart { Order(Vec), } -/// A `ParsedFindQuery` represents a parsed but potentially invalid query to the query algebrizer. +/// A `ParsedQuery` represents a parsed but potentially invalid query to the query algebrizer. /// Such a query is syntactically valid but might be semantically invalid, for example because /// constraints on the set of variables are not respected. /// -/// We split `ParsedFindQuery` from `FindQuery` because it's not easy to generalize over containers +/// We split `ParsedQuery` from `FindQuery` because it's not easy to generalize over containers /// (here, `Vec` and `BTreeSet`) in Rust. -impl ParsedFindQuery { - pub(crate) fn from_parts(parts: Vec) -> std::result::Result { +impl ParsedQuery { + pub(crate) fn from_parts(parts: Vec) -> std::result::Result { let mut find_spec: Option = None; let mut with: Option> = None; let mut in_vars: Option> = None; @@ -1055,7 +1054,7 @@ impl ParsedFindQuery { } } - Ok(ParsedFindQuery { + Ok(ParsedQuery { find_spec: find_spec.ok_or("expected :find")?, default_source: SrcVar::DefaultSrc, with: with.unwrap_or(vec![]), diff --git a/edn/tests/query_tests.rs b/edn/tests/query_tests.rs index da6d54b6..ae74140a 100644 --- a/edn/tests/query_tests.rs +++ b/edn/tests/query_tests.rs @@ -35,7 +35,7 @@ use edn::query::{ }; use edn::parse::{ - query as parse_query, + parse_query, }; ///! N.B., parsing a query can be done without reference to a DB. @@ -279,3 +279,22 @@ fn can_parse_uuid() { PatternNonValuePlace::Placeholder) .expect("valid pattern"))); } + +#[test] +fn can_parse_exotic_whitespace() { + let expected = edn::Uuid::parse_str("4cb3f828-752d-497a-90c9-b1fd516d5644").expect("valid uuid"); + // The query string from `can_parse_uuid`, with newlines, commas, and line comments interspersed. + let s = r#"[:find +?x ,, :where, ;atest +[?x :foo/baz #uuid + "4cb3f828-752d-497a-90c9-b1fd516d5644", ;testa +,],, ,],;"#; + assert_eq!(parse_query(s).expect("parsed").where_clauses.pop().expect("a where clause"), + WhereClause::Pattern( + Pattern::new(None, + PatternNonValuePlace::Variable(Variable::from_valid_name("?x")), + Keyword::namespaced("foo", "baz").into(), + PatternValuePlace::Constant(NonIntegerConstant::Uuid(expected)), + PatternNonValuePlace::Placeholder) + .expect("valid pattern"))); +} diff --git a/query-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 80c89824..90704b7a 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -41,7 +41,7 @@ use mentat_query::{ FindSpec, Limit, Order, - ParsedFindQuery, + ParsedQuery, SrcVar, Variable, WhereClause, @@ -351,7 +351,7 @@ impl FindQuery { } } - pub fn from_parsed_find_query(parsed: ParsedFindQuery) -> Result { + pub fn from_parsed_query(parsed: ParsedQuery) -> Result { let in_vars = { let mut set: BTreeSet = BTreeSet::default(); @@ -398,7 +398,6 @@ impl FindQuery { pub fn parse_find_string(string: &str) -> Result { parse_query(string) - // .and_then( .map_err(|e| e.into()) - .and_then(|parsed| FindQuery::from_parsed_find_query(parsed)) // .map_err(|e| e.into())) + .and_then(|parsed| FindQuery::from_parsed_query(parsed)) } diff --git a/query-algebrizer/src/types.rs b/query-algebrizer/src/types.rs index 397170e6..11d33fae 100644 --- a/query-algebrizer/src/types.rs +++ b/query-algebrizer/src/types.rs @@ -695,6 +695,9 @@ impl Debug for EmptyBecause { /// A `FindQuery` represents a valid query to the query algebrizer. +/// +/// We split `FindQuery` from `ParsedQuery` because it's not easy to generalize over containers +/// (here, `Vec` and `BTreeSet`) in Rust. #[allow(dead_code)] #[derive(Debug, Eq, PartialEq)] pub struct FindQuery { @@ -706,7 +709,6 @@ pub struct FindQuery { pub limit: Limit, pub where_clauses: Vec, pub order: Option>, - // TODO: in_rules; } // Intermediate data structures for resolving patterns.