Makes subqueries SPARQL 1.1 test cases run and pass

* Fixes subqueries projection (especially in a GRAPH clause)
* Fixes CONSTRUCT behavior when a variable is not bound

lib/src/sparql/eval.rs

@@ -245,17 +245,17 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator<S> {
             })),
             PlanNode::Join { left, right } => {
                 //TODO: very dumb implementation
-                let left_iter = self.eval_plan(&*left, from.clone());
-                let mut left_values = Vec::with_capacity(left_iter.size_hint().0);
                 let mut errors = Vec::default();
-                for result in left_iter {
+                let left_values = self
-                    match result {
+                    .eval_plan(&*left, from.clone())
-                        Ok(result) => {
+                    .filter_map(|result| match result {
-                            left_values.push(result);
+                        Ok(result) => Some(result),
-                        }
+                        Err(error) => {
-                        Err(error) => errors.push(Err(error)),
+                            errors.push(Err(error));
-                    }
+                            None
                         }
+                    })
+                    .collect::<Vec<_>>();
                 Box::new(JoinIterator {
                     left: left_values,
                     right_iter: self.eval_plan(&*right, from),
@@ -332,17 +332,17 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator<S> {
                 }))
             }
             PlanNode::Sort { child, by } => {
-                let iter = self.eval_plan(&*child, from);
-                let mut values = Vec::with_capacity(iter.size_hint().0);
                 let mut errors = Vec::default();
-                for result in iter {
+                let mut values = self
-                    match result {
+                    .eval_plan(&*child, from)
-                        Ok(result) => {
+                    .filter_map(|result| match result {
-                            values.push(result);
+                        Ok(result) => Some(result),
-                        }
+                        Err(error) => {
-                        Err(error) => errors.push(Err(error)),
+                            errors.push(Err(error));
-                    }
+                            None
                         }
+                    })
+                    .collect::<Vec<_>>();
                 values.sort_unstable_by(|a, b| {
                     for comp in by {
                         match comp {
@@ -374,14 +374,20 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator<S> {
                 Box::new(self.eval_plan(&*child, from).take(*count))
             }
             PlanNode::Project { child, mapping } => {
-                Box::new(self.eval_plan(&*child, from).map(move |tuple| {
+                //TODO: use from somewhere?
+                Box::new(
+                    self.eval_plan(&*child, vec![None; mapping.len()])
+                        .map(move |tuple| {
                             let tuple = tuple?;
-                    let mut new_tuple = Vec::with_capacity(mapping.len());
+                            let mut output_tuple = vec![None; from.len()];
-                    for key in mapping {
+                            for (input_key, output_key) in mapping.iter() {
-                        new_tuple.push(tuple[*key]);
+                                if let Some(value) = tuple[*input_key] {
+                                    put_value(*output_key, value, &mut output_tuple)
                                 }
-                    Ok(new_tuple)
+                            }
-                }))
+                            Ok(output_tuple)
+                        }),
+                )
             }
         }
     }
@@ -1495,19 +1501,18 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator<S> {
         'a: 'b,
     {
         let eval = self;
+        let tuple_size = variables.len();
         BindingsIterator::new(
             variables,
             Box::new(iter.map(move |values| {
                 let encoder = eval.dataset.encoder();
-                values?
+                let mut result = vec![None; tuple_size];
-                    .into_iter()
+                for (i, value) in values?.into_iter().enumerate() {
-                    .map(|value| {
+                    if let Some(term) = value {
-                        Ok(match value {
+                        result[i] = Some(encoder.decode_term(term)?)
-                            Some(term) => Some(encoder.decode_term(term)?),
+                    }
-                            None => None,
+                }
-                        })
+                Ok(result)
-                    })
-                    .collect()
             })),
         )
     }
@@ -2140,9 +2145,6 @@ impl<'a, S: StoreConnection> Iterator for ConstructIterator<'a, S> {
                     ) {
                         self.buffered_results
                             .push(decode_triple(&encoder, subject, predicate, object));
-                    } else {
-                        self.buffered_results.clear(); //No match, we do not output any triple for this row
-                        break;
                     }
                 }
                 self.bnodes.clear(); //We do not reuse old bnodes

lib/src/sparql/plan.rs

@@ -66,7 +66,7 @@ pub enum PlanNode {
     },
     Project {
         child: Box<PlanNode>,
-        mapping: Vec<usize>, // for each key in children the key of the returned vector (children is sliced at the vector length)
+        mapping: Vec<(usize, usize)>, // pairs of (variable key in child, variable key in output)
     },
 }
 

lib/src/sparql/plan_builder.rs

@@ -149,18 +149,38 @@ impl<'a, S: StoreConnection> PlanBuilder<'a, S> {
                     by: by?,
                 }
             }
-            GraphPattern::Project(l, new_variables) => PlanNode::Project {
+            GraphPattern::Project(l, new_variables) => {
+                let mut inner_variables = new_variables.clone();
+                let inner_graph_name = match graph_name {
+                    PatternValue::Constant(graph_name) => PatternValue::Constant(graph_name),
+                    PatternValue::Variable(graph_name) => PatternValue::Variable(
+                        new_variables
+                            .iter()
+                            .enumerate()
+                            .find(|(_, var)| *var == &variables[graph_name])
+                            .map(|(new_key, _)| new_key)
+                            .unwrap_or_else(|| {
+                                inner_variables.push(Variable::default());
+                                inner_variables.len() - 1
+                            }),
+                    ),
+                };
+                PlanNode::Project {
                     child: Box::new(self.build_for_graph_pattern(
                         l,
                         input,
-                    &mut new_variables.clone(),
+                        &mut inner_variables,
-                    graph_name,
+                        inner_graph_name,
                     )?),
                     mapping: new_variables
                         .iter()
-                    .map(|variable| variable_key(variables, variable))
+                        .enumerate()
+                        .map(|(new_variable, variable)| {
+                            (new_variable, variable_key(variables, variable))
+                        })
                         .collect(),
-            },
+                }
+            }
             GraphPattern::Distinct(l) => PlanNode::HashDeduplicate {
                 child: Box::new(self.build_for_graph_pattern(l, input, variables, graph_name)?),
             },

lib/tests/sparql_test_cases.rs

@@ -88,12 +88,14 @@ fn sparql_w3c_query_evaluation_testsuite() -> Result<()> {
 
     let manifest_11_urls = vec![
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/bind/manifest.ttl",
+        //TODO "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/bindings/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/construct/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/exists/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/functions/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/negation/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/project-expression/manifest.ttl",
         "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/property-path/manifest.ttl",
+        "http://www.w3.org/2009/sparql/docs/tests/data-sparql11/subquery/manifest.ttl",
     ];
 
     let test_blacklist = vec![
@@ -126,7 +128,9 @@ fn sparql_w3c_query_evaluation_testsuite() -> Result<()> {
         //Decimal precision problem
         NamedNode::parse("http://www.w3.org/2009/sparql/docs/tests/data-sparql11/functions/manifest#coalesce01").unwrap(),
         //Property path with unbound graph name are not supported yet
-        NamedNode::parse("http://www.w3.org/2009/sparql/docs/tests/data-sparql11/property-path/manifest#pp35").unwrap()
+        NamedNode::parse("http://www.w3.org/2009/sparql/docs/tests/data-sparql11/property-path/manifest#pp35").unwrap(),
+        //Aggregate in subquery (TODO when aggregates are implemented)
+        NamedNode::parse("http://www.w3.org/2009/sparql/docs/tests/data-sparql11/subquery/manifest#subquery08").unwrap(),
     ];
 
     let tests: Result<Vec<_>> = manifest_10_urls