Part 2 - Validate not and not-join pre-algebrization

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<T> {
@@ -586,6 +590,9 @@ impl ConjoiningClauses {
                 //?;
                 //self.apply_or_join(schema, o)
             },
+            WhereClause::NotJoin(n) => {
+                validate_not_join(&n)
+            },
             _ => unimplemented!(),
         }
     }

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")
+        }
     }
 }
 

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<Variable> = vars.iter().cloned().collect();
+            for var in &var_set {
+                if !&not_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<Variable> = 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<WhereNotClause> {
+        // 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(&not_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(&not_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(&not_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(&not_join).is_err()),
+            _ => panic!(),
+        }
+    }
 }