diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index 1872b8f9..054369ef 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -56,11 +56,15 @@ use types::{ }; mod or; +mod not; mod pattern; mod predicate; mod resolve; -use validate::validate_or_join; +use validate::{ + validate_or_join, + validate_not_join, +}; // We do this a lot for errors. trait RcCloned { @@ -586,6 +590,9 @@ impl ConjoiningClauses { //?; //self.apply_or_join(schema, o) }, + WhereClause::NotJoin(n) => { + validate_not_join(&n) + }, _ => unimplemented!(), } } diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index dff08556..33e9f9fc 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -45,6 +45,12 @@ error_chain! { description("non-matching variables in 'or' clause") display("non-matching variables in 'or' clause") } + + NonMatchingVariablesInNotClause { + // TODO: flesh out. + description("non-matching variables in 'not' clause") + display("non-matching variables in 'not' clause") + } } } diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index d10ff237..22d60c6a 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -13,6 +13,7 @@ use std::collections::BTreeSet; use mentat_query::{ ContainsVariables, OrJoin, + NotJoin, Variable, UnifyVars, }; @@ -74,6 +75,46 @@ pub fn validate_or_join(or_join: &OrJoin) -> Result<()> { } } +pub fn validate_not_join(not_join: &NotJoin) -> Result<()> { + // Grab our mentioned variables and ensure that the rules are followed. + match not_join.unify_vars { + UnifyVars::Implicit => { + if not_join.clauses.len() < 2 { + Ok(()) + } else { + let mut clauses = not_join.clauses.iter(); + let template = clauses.next().unwrap().collect_mentioned_variables(); + for clause in clauses { + if template != clause.collect_mentioned_variables() { + bail!(ErrorKind::NonMatchingVariablesInNotClause); + } + } + Ok(()) + } + }, + UnifyVars::Explicit(ref vars) => { + // The joined vars must each appear somewhere in the clauses mentioned variables. + let var_set: BTreeSet = vars.iter().cloned().collect(); + for var in &var_set { + if !¬_join.clauses.iter().any(|clause| clause.collect_mentioned_variables().contains(&var)) { + bail!(ErrorKind::NonMatchingVariablesInNotClause); + } + } + + // any other mentioned variables must also unify + let mut clauses = not_join.clauses.iter(); + let template: BTreeSet = clauses.next().unwrap().collect_mentioned_variables().difference(&var_set).cloned().collect(); + for clause in clauses { + let mentioned_vars = clause.collect_mentioned_variables().difference(&var_set).cloned().collect(); + if template != mentioned_vars { + bail!(ErrorKind::NonMatchingVariablesInOrClause); + } + } + Ok(()) + }, + } +} + #[cfg(test)] mod tests { extern crate mentat_core; @@ -84,6 +125,7 @@ mod tests { FindQuery, NamespacedKeyword, OrWhereClause, + WhereNotClause, Pattern, PatternNonValuePlace, PatternValuePlace, @@ -96,7 +138,10 @@ mod tests { use clauses::ident; - use super::validate_or_join; + use super::{ + validate_or_join, + validate_not_join, + }; fn value_ident(ns: &str, name: &str) -> PatternValuePlace { PatternValuePlace::IdentOrKeyword(::std::rc::Rc::new(NamespacedKeyword::new(ns, name))) @@ -229,4 +274,177 @@ mod tests { _ => panic!(), }; } + + + /// Tests that the top-level form is a valid `not`, returning the clauses. + fn valid_not_join(parsed: FindQuery, expected_unify: UnifyVars) -> Vec { + // filter all the not clauses + let mut nots = parsed.where_clauses.iter().filter(|&x| match *x { + WhereClause::NotJoin(_) => true, + _ => false, + }); + + // there should be only one not clause + let clause = nots.next().unwrap().clone(); + assert_eq!(None, nots.next()); + + match clause { + WhereClause::NotJoin(not_join) => { + // It's valid: the variables are the same in each branch. + assert_eq!((), validate_not_join(¬_join).unwrap()); + assert_eq!(expected_unify, not_join.unify_vars); + not_join.clauses + }, + _ => panic!(), + } + } + + /// Test that a `not` is valid if all of its arms refer to the same variables. + #[test] + fn test_success_not() { + let query = r#"[:find ?name + :where [?id :artist/name ?name] + (not [?id :artist/country :country/CA] + [?id :artist/country :country/GB])]"#; + let parsed = parse_find_string(query).expect("expected successful parse"); + let clauses = valid_not_join(parsed, UnifyVars::Implicit); + + // Check each part of the body + let mut parts = clauses.into_iter(); + match (parts.next(), parts.next(), parts.next()) { + (Some(clause1), Some(clause2), None) => { + assert_eq!( + clause1, + WhereNotClause::Clause( + WhereClause::Pattern(Pattern { + source: None, + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?id")), + attribute: ident("artist", "country"), + value: value_ident("country", "CA"), + tx: PatternNonValuePlace::Placeholder, + }), + )); + assert_eq!( + clause2, + WhereNotClause::Clause( + WhereClause::Pattern(Pattern { + source: None, + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?id")), + attribute: ident("artist", "country"), + value: value_ident("country", "GB"), + tx: PatternNonValuePlace::Placeholder, + }), + )); + }, + _ => panic!(), + }; + } + + + /// Test that a `not` with differing variable sets in each body part will fail to validate. + #[test] + fn test_invalid_implicit_not() { + let query = r#"[:find ?name + :where [?id :artist/name ?name] + (not [?id :artist/country :country/CA] + [?release :release/year 1970])]"#; + let parsed = parse_find_string(query).expect("expected successful parse"); + let mut nots = parsed.where_clauses.iter().filter(|&x| match *x { + WhereClause::NotJoin(_) => true, + _ => false, + }); + + let clause = nots.next().unwrap().clone(); + assert_eq!(None, nots.next()); + + match clause { + WhereClause::NotJoin(not_join) => assert!(validate_not_join(¬_join).is_err()), + _ => panic!(), + } + } + + #[test] + fn test_success_not_join() { + let query = r#"[:find ?artist + :where [?artist :artist/name] + (not-join [?artist] + [?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(vec![Variable::from_valid_name("?artist")])); + + // // Let's do some detailed parse checks. + let mut parts = clauses.into_iter(); + match (parts.next(), parts.next(), parts.next()) { + (Some(clause1), Some(clause2), None) => { + assert_eq!( + clause1, + WhereNotClause::Clause( + WhereClause::Pattern(Pattern { + source: None, + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?release")), + attribute: ident("release", "artists"), + value: PatternValuePlace::Variable(Variable::from_valid_name("?artist")), + tx: PatternNonValuePlace::Placeholder, + }))); + assert_eq!( + clause2, + WhereNotClause::Clause( + WhereClause::Pattern(Pattern { + source: None, + entity: PatternNonValuePlace::Variable(Variable::from_valid_name("?release")), + attribute: ident("release", "year"), + value: PatternValuePlace::EntidOrInteger(1970), + tx: PatternNonValuePlace::Placeholder, + }))); + }, + _ => panic!(), + }; + } + + /// Test that a `not-join` that does not use the joining var fails to validate. + #[test] + fn test_invalid_explicit_not_join_non_matching_join_vars() { + let query = r#"[:find ?artist + :where [?artist :artist/name] + (not-join [?artist] + [?release :release/artists "Pink Floyd"] + [?release :release/year 1970])]"#; + let parsed = parse_find_string(query).expect("expected successful parse"); + let mut nots = parsed.where_clauses.iter().filter(|&x| match *x { + WhereClause::NotJoin(_) => true, + _ => false, + }); + + let clause = nots.next().unwrap().clone(); + assert_eq!(None, nots.next()); + + match clause { + WhereClause::NotJoin(not_join) => assert!(validate_not_join(¬_join).is_err()), + _ => panic!(), + } + } + + /// Test that a `not-join` that does not match any vars declared inside it fails to validate. + #[test] + fn test_invalid_explicit_not_join_non_matching_internal_vars() { + let query = r#"[:find ?artist + :where [?artist :artist/name] + (not-join [?artist] + [?release :release/artists ?artist] + [?year :release/year 1970])]"#; + let parsed = parse_find_string(query).expect("expected successful parse"); + let mut nots = parsed.where_clauses.iter().filter(|&x| match *x { + WhereClause::NotJoin(_) => true, + _ => false, + }); + + let clause = nots.next().unwrap().clone(); + assert_eq!(None, nots.next()); + + match clause { + WhereClause::NotJoin(not_join) => assert!(validate_not_join(¬_join).is_err()), + _ => panic!(), + } + } } \ No newline at end of file