Pre: make rule_vars return unique vars.

query-algebrizer/src/clauses/not.rs

@@ -32,7 +32,7 @@ impl ConjoiningClauses {
     pub fn apply_not_join(&mut self, schema: &Schema, not_join: NotJoin) -> Result<()> {
         let unified = match not_join.unify_vars {
             UnifyVars::Implicit => not_join.collect_mentioned_variables(),
-            UnifyVars::Explicit(vs) => vs.into_iter().collect(),
+            UnifyVars::Explicit(vs) => vs,
         };
 
         let mut template = self.use_as_template(&unified);

query-algebrizer/src/clauses/or.rs

@@ -552,7 +552,7 @@ impl ConjoiningClauses {
         let (join_clauses, unify_vars, mentioned_vars) = or_join.dismember();
         let projected = match unify_vars {
             UnifyVars::Implicit => mentioned_vars.into_iter().collect(),
-            UnifyVars::Explicit(vs) => vs.into_iter().collect(),
+            UnifyVars::Explicit(vs) => vs,
         };
 
         let template = self.use_as_template(&projected);

query-algebrizer/src/lib.rs

@@ -13,6 +13,10 @@ extern crate enum_set;
 #[macro_use]
 extern crate error_chain;
 
+#[cfg(test)]
+#[macro_use]
+extern crate maplit;
+
 extern crate mentat_core;
 extern crate mentat_query;
 

query-algebrizer/src/validate.rs

@@ -212,7 +212,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(vec![Variable::from_valid_name("?artist")]));
+        let clauses = valid_or_join(parsed, UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?artist")}));
 
         // Let's do some detailed parse checks.
         let mut arms = clauses.into_iter();
@@ -322,7 +322,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(vec![Variable::from_valid_name("?artist")]));
+        let clauses = valid_not_join(parsed, UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?artist")}));
 
         let release = PatternNonValuePlace::Variable(Variable::from_valid_name("?release"));
         let artist = PatternValuePlace::Variable(Variable::from_valid_name("?artist"));

query-parser/src/lib.rs

@@ -10,6 +10,9 @@
 
 #![allow(unused_imports)]
 
+#[macro_use]
+extern crate maplit;
+
 #[macro_use]
 extern crate error_chain;
 

query-parser/src/parse.rs

@@ -197,9 +197,9 @@ def_matches_plain_symbol!(Where, not, "not");
 
 def_matches_plain_symbol!(Where, not_join, "not-join");
 
-def_parser!(Where, rule_vars, Vec<Variable>, {
+def_parser!(Where, rule_vars, BTreeSet<Variable>, {
     seq()
-        .of_exactly(many1(Query::variable()))
+        .of_exactly(many1(Query::variable()).and_then(unique_vars))
 });
 
 def_parser!(Where, or_pattern_clause, OrWhereClause, {
@@ -392,18 +392,20 @@ def_parser!(Find, spec, FindSpec, {
           &mut try(Find::find_rel())])
 });
 
+fn unique_vars<T, E>(vars: Vec<Variable>) -> std::result::Result<BTreeSet<Variable>, combine::primitives::Error<T, E>> {
+    let given = vars.len();
+    let set: BTreeSet<Variable> = 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<Variable>, {
-    many(Query::variable()).and_then(|vars: Vec<Variable>| {
+    many(Query::variable()).and_then(unique_vars)
-        let given = vars.len();
-        let set: BTreeSet<Variable> = 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)
-        }
-    })
 });
 
 /// This is awkward, but will do for now.  We use `keyword_map()` to optionally accept vector find
@@ -573,7 +575,7 @@ mod test {
         let e = edn::PlainSymbol::new("?e");
         let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(e.clone())]);
         assert_parses_to!(Where::rule_vars, input,
-                          vec![variable(e.clone())]);
+                          btreeset!{variable(e.clone())});
     }
 
     #[test]
@@ -583,7 +585,7 @@ mod test {
         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,
-                          vec![variable(e.clone()), variable(f.clone())].into_iter().collect());
+                          btreeset!{variable(e.clone()), variable(f.clone())});
 
         let g = edn::PlainSymbol::new("?g");
         let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(g.clone()),
@@ -642,7 +644,7 @@ mod test {
                                          edn::Value::PlainSymbol(v.clone())])].into_iter().collect());
         assert_parses_to!(Where::or_join_clause, input,
                           WhereClause::OrJoin(
-                              OrJoin::new(UnifyVars::Explicit(vec![variable(e.clone())]),
+                              OrJoin::new(UnifyVars::Explicit(btreeset!{variable(e.clone())}),
                                           vec![OrWhereClause::Clause(
                                               WhereClause::Pattern(Pattern {
                                                   source: None,
@@ -685,7 +687,7 @@ mod test {
                               "(not-join [?e] [?e ?a ?v])",
                               WhereClause::NotJoin(
                               NotJoin {
-                                  unify_vars: UnifyVars::Explicit(vec![variable(e.clone())]),
+                                  unify_vars: UnifyVars::Explicit(btreeset!{variable(e.clone())}),
                                   clauses: vec![WhereClause::Pattern(Pattern {
                                           source: None,
                                           entity: PatternNonValuePlace::Variable(variable(e)),

query-parser/tests/find_tests.rs

@@ -8,6 +8,9 @@
 // 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_core;
 extern crate mentat_query;
@@ -111,7 +114,7 @@ fn can_parse_unit_or_join() {
     assert_eq!(p.where_clauses,
                vec![
                    WhereClause::OrJoin(OrJoin::new(
-                       UnifyVars::Explicit(vec![Variable::from_valid_name("?x")]),
+                       UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?x")}),
                        vec![
                            OrWhereClause::Clause(
                                WhereClause::Pattern(Pattern {
@@ -136,7 +139,7 @@ fn can_parse_simple_or_join() {
     assert_eq!(p.where_clauses,
                vec![
                    WhereClause::OrJoin(OrJoin::new(
-                       UnifyVars::Explicit(vec![Variable::from_valid_name("?x")]),
+                       UnifyVars::Explicit(btreeset!{Variable::from_valid_name("?x")}),
                        vec![
                            OrWhereClause::Clause(
                                WhereClause::Pattern(Pattern {

query/src/lib.rs

@@ -615,7 +615,7 @@ pub enum UnifyVars {
     /// Only the named variables will be unified with the enclosing query.
     ///
     /// Every 'arm' in an `or-join` must mention the entire set of explicit vars.
-    Explicit(Vec<Variable>),
+    Explicit(BTreeSet<Variable>),
 }
 
 impl WhereClause {