From 9fe31d443d161b72d1ecde699fc5cc9475199eb8 Mon Sep 17 00:00:00 2001
From: Nick Alexander <nalexander@mozilla.com>
Date: Wed, 19 Apr 2017 11:10:24 -0700
Subject: [PATCH] Pre: Accept EDN vectors in FnArg arguments.

Datomic accepts mostly-arbitrary EDN, and it is actually used: for
example, the following are all valid, and all mean different things:
* `(ground 1 ?x)`
* `(ground [1 2 3] [?x ?y ?z])`
* `(ground [[1 2 3] [4 5 6]] [[?x ?y ?z]])`

We could probably introduce new syntax that expresses these patterns
while avoiding collection arguments, but I don't see one right now.
I've elected to support only vectors for simplicity; I'm hoping to
avoid parsing edn::Value in the query-algebrizer.
---
 query-algebrizer/src/clauses/resolve.rs |  4 +++-
 query-parser/src/parse.rs               | 16 +++++++++++++++-
 query/src/lib.rs                        |  3 +++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/query-algebrizer/src/clauses/resolve.rs b/query-algebrizer/src/clauses/resolve.rs
index b64ce9cd..58e0d902 100644
--- a/query-algebrizer/src/clauses/resolve.rs
+++ b/query-algebrizer/src/clauses/resolve.rs
@@ -56,7 +56,8 @@ impl ConjoiningClauses {
             Constant(NonIntegerConstant::Text(_)) |
             Constant(NonIntegerConstant::Uuid(_)) |
             Constant(NonIntegerConstant::Instant(_)) |        // Instants are covered elsewhere.
-            Constant(NonIntegerConstant::BigInteger(_)) => {
+            Constant(NonIntegerConstant::BigInteger(_)) |
+            Vector(_) => {
                 self.mark_known_empty(EmptyBecause::NonNumericArgument);
                 bail!(ErrorKind::NonNumericArgument(function.clone(), position));
             },
@@ -86,6 +87,7 @@ impl ConjoiningClauses {
             Constant(NonIntegerConstant::Instant(u)) => Ok(QueryValue::TypedValue(TypedValue::Instant(u))),
             Constant(NonIntegerConstant::BigInteger(_)) => unimplemented!(),
             SrcVar(_) => unimplemented!(),
+            Vector(_) => unimplemented!(),    // TODO
         }
     }
 }
diff --git a/query-parser/src/parse.rs b/query-parser/src/parse.rs
index ab2d857c..f5a58447 100644
--- a/query-parser/src/parse.rs
+++ b/query-parser/src/parse.rs
@@ -134,7 +134,7 @@ def_parser!(Query, predicate_fn, PredicateFn, {
 });
 
 def_parser!(Query, fn_arg, FnArg, {
-    satisfy_map(FnArg::from_value)
+    satisfy_map(FnArg::from_value).or(vector().of_exactly(many::<Vec<FnArg>, _>(Query::fn_arg())).map(FnArg::Vector))
 });
 
 def_parser!(Query, arguments, Vec<FnArg>, {
@@ -777,4 +777,18 @@ mod test {
         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::new("?x");
+        let vy = edn::PlainSymbol::new("?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)),
+                          ]));
+    }
 }
diff --git a/query/src/lib.rs b/query/src/lib.rs
index bc4134a6..414090f0 100644
--- a/query/src/lib.rs
+++ b/query/src/lib.rs
@@ -216,6 +216,9 @@ pub enum FnArg {
     EntidOrInteger(i64),
     IdentOrKeyword(NamespacedKeyword),
     Constant(NonIntegerConstant),
+    // The collection values representable in EDN.  There's no advantage to destructuring up front,
+    // since consumers will need to handle arbitrarily nested EDN themselves anyway.
+    Vector(Vec<FnArg>),
 }
 
 impl FromValue<FnArg> for FnArg {