From 41c738dc7a78d227f7570249a2d56b6ddb60c344 Mon Sep 17 00:00:00 2001 From: Tpt Date: Fri, 20 Sep 2019 21:41:20 +0200 Subject: [PATCH] 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 | 78 +++++++++++++++++----------------- lib/src/sparql/plan.rs | 2 +- lib/src/sparql/plan_builder.rs | 44 +++++++++++++------ lib/tests/sparql_test_cases.rs | 6 ++- 4 files changed, 78 insertions(+), 52 deletions(-) diff --git a/lib/src/sparql/eval.rs b/lib/src/sparql/eval.rs index 901e879f..d67f849c 100644 --- a/lib/src/sparql/eval.rs +++ b/lib/src/sparql/eval.rs @@ -245,17 +245,17 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator { })), 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 { - match result { - Ok(result) => { - left_values.push(result); + let left_values = self + .eval_plan(&*left, from.clone()) + .filter_map(|result| match result { + Ok(result) => Some(result), + Err(error) => { + errors.push(Err(error)); + None } - Err(error) => errors.push(Err(error)), - } - } + }) + .collect::>(); Box::new(JoinIterator { left: left_values, right_iter: self.eval_plan(&*right, from), @@ -332,17 +332,17 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator { })) } 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 { - match result { - Ok(result) => { - values.push(result); + let mut values = self + .eval_plan(&*child, from) + .filter_map(|result| match result { + Ok(result) => Some(result), + Err(error) => { + errors.push(Err(error)); + None } - Err(error) => errors.push(Err(error)), - } - } + }) + .collect::>(); values.sort_unstable_by(|a, b| { for comp in by { match comp { @@ -374,14 +374,20 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator { Box::new(self.eval_plan(&*child, from).take(*count)) } PlanNode::Project { child, mapping } => { - Box::new(self.eval_plan(&*child, from).map(move |tuple| { - let tuple = tuple?; - let mut new_tuple = Vec::with_capacity(mapping.len()); - for key in mapping { - new_tuple.push(tuple[*key]); - } - Ok(new_tuple) - })) + //TODO: use from somewhere? + Box::new( + self.eval_plan(&*child, vec![None; mapping.len()]) + .map(move |tuple| { + let tuple = tuple?; + let mut output_tuple = vec![None; from.len()]; + for (input_key, output_key) in mapping.iter() { + if let Some(value) = tuple[*input_key] { + put_value(*output_key, value, &mut output_tuple) + } + } + Ok(output_tuple) + }), + ) } } } @@ -1495,19 +1501,18 @@ impl<'a, S: StoreConnection + 'a> SimpleEvaluator { '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? - .into_iter() - .map(|value| { - Ok(match value { - Some(term) => Some(encoder.decode_term(term)?), - None => None, - }) - }) - .collect() + let mut result = vec![None; tuple_size]; + for (i, value) in values?.into_iter().enumerate() { + if let Some(term) = value { + result[i] = Some(encoder.decode_term(term)?) + } + } + Ok(result) })), ) } @@ -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 diff --git a/lib/src/sparql/plan.rs b/lib/src/sparql/plan.rs index a38656b7..5e567412 100644 --- a/lib/src/sparql/plan.rs +++ b/lib/src/sparql/plan.rs @@ -66,7 +66,7 @@ pub enum PlanNode { }, Project { child: Box, - mapping: Vec, // 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) }, } diff --git a/lib/src/sparql/plan_builder.rs b/lib/src/sparql/plan_builder.rs index 1ca0f785..2f294841 100644 --- a/lib/src/sparql/plan_builder.rs +++ b/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 { - child: Box::new(self.build_for_graph_pattern( - l, - input, - &mut new_variables.clone(), - graph_name, - )?), - mapping: new_variables - .iter() - .map(|variable| variable_key(variables, variable)) - .collect(), - }, + 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 inner_variables, + inner_graph_name, + )?), + mapping: new_variables + .iter() + .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)?), }, diff --git a/lib/tests/sparql_test_cases.rs b/lib/tests/sparql_test_cases.rs index 08542d1a..a68bef2c 100644 --- a/lib/tests/sparql_test_cases.rs +++ b/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> = manifest_10_urls