greg/mentat
1
0
Fork 0
Code Issues 294 Pull requests Packages Projects Releases 9 Wiki Activity

Part 5: Push FindQuery into query-algebrizer; structure errors.

Browse source 
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.
This commit is contained in:
Nick Alexander 2018-05-31 14:10:49 -07:00
parent 09f1d633b5
commit 47441f56dc
17 changed files with 164 additions and 156 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
core/src/lib.rs
View file

@ -46,6 +46,10 @@ pub use edn::{
Utc,
ValueRc,
};
pub use edn::parse::{
query as parse_query,
ParseError as EdnParseError,
};
pub use cache::{
CachedAttributes,

74
edn/src/query.rs
View file

@ -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<Variable>,
pub in_vars: BTreeSet<Variable>,
pub in_sources: BTreeSet<SrcVar>,
pub limit: Limit,
pub where_clauses: Vec<WhereClause>,
pub order: Option<Vec<Order>>,
// TODO: in_rules;
}
#[allow(dead_code)]
#[derive(Debug, Eq, PartialEq)]
pub struct ParsedFindQuery {
@ -1013,6 +999,12 @@ pub(crate) enum QueryPart {
Order(Vec<Order>),
}
/// 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<QueryPart>) -> std::result::Result<ParsedFindQuery, &'static str> {
let mut find_spec: Option<FindSpec> = None;
@ -1074,60 +1066,6 @@ impl ParsedFindQuery {
order,
})
}
pub fn into_find_query(self: ParsedFindQuery) -> Result<FindQuery, &'static str> {
let in_vars = {
let len = self.in_vars.len();
let set: BTreeSet<Variable> = 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<Variable> = 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<WhereClause>) -> 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 {

54
query-parser/tests/find_tests.rs → edn/tests/query_tests.rs
View file

@ -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")),

1
query-algebrizer/Cargo.toml
View file

@ -18,4 +18,3 @@ path = "../query-parser"
[dev-dependencies]
itertools = "0.7"
maplit = "0.1"

3
query-algebrizer/src/clauses/not.rs
View file

@ -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 {

5
query-algebrizer/src/clauses/or.rs
View file

@ -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 {

9
query-algebrizer/src/clauses/pattern.rs
View file

@ -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");

15
query-algebrizer/src/errors.rs
View file

@ -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)
}
}
}

76
query-algebrizer/src/lib.rs
View file

@ -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<WhereClause>) -> 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<FindQuery> {
let in_vars = {
let mut set: BTreeSet<Variable> = 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<Variable> = 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<FindQuery> {
parse_query(string)
// .and_then(
.map_err(|e| e.into())
.and_then(|parsed| FindQuery::from_parsed_find_query(parsed)) // .map_err(|e| e.into()))
}

34
query-algebrizer/src/types.rs
View file

@ -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<Variable>,
pub in_vars: BTreeSet<Variable>,
pub in_sources: BTreeSet<SrcVar>,
pub limit: Limit,
pub where_clauses: Vec<WhereClause>,
pub order: Option<Vec<Order>>,
// TODO: in_rules;
}
// Intermediate data structures for resolving patterns.
#[derive(Debug, Eq, PartialEq)]

14
query-algebrizer/src/validate.rs
View file

@ -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"));

5
query-algebrizer/tests/utils/mod.rs
View file

@ -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.

1
query-parser/Cargo.toml
View file

@ -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"

9
query-parser/src/lib.rs
View file

@ -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::query::FindQuery> {
edn::parse::query(string)
.map_err(|e| e.into())
.and_then(|parsed| parsed.into_find_query().map_err(|e| e.into()))
}

5
query-projector/tests/aggregates.rs
View file

@ -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::{

2
query-translator/tests/translate.rs
View file

@ -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::{

9
src/query.rs
View file

@ -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,