From b6c9a5b42991051e43dea419f3ac0f68c8d8f5ec Mon Sep 17 00:00:00 2001 From: Tpt Date: Sun, 18 Dec 2022 22:02:48 +0100 Subject: [PATCH] Fixes ZeroOrX path evaluation on terms that are not in the dataset but only in the query --- lib/src/sparql/eval.rs | 90 ++++++++++++++----- testsuite/oxigraph-tests/sparql/manifest.ttl | 7 ++ .../sparql/values_property_path_all.rq | 4 + .../sparql/values_property_path_all.srx | 8 ++ 4 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 testsuite/oxigraph-tests/sparql/values_property_path_all.rq create mode 100644 testsuite/oxigraph-tests/sparql/values_property_path_all.srx diff --git a/lib/src/sparql/eval.rs b/lib/src/sparql/eval.rs index 6c76c055..bf452d01 100644 --- a/lib/src/sparql/eval.rs +++ b/lib/src/sparql/eval.rs @@ -769,12 +769,14 @@ impl SimpleEvaluator { .chain(self.eval_path_from(b, start, graph_name)), ), PlanPropertyPath::ZeroOrMore(p) => { - let eval = self.clone(); - let p = p.clone(); - let graph_name2 = graph_name.clone(); - Box::new(transitive_closure(Some(Ok(start.clone())), move |e| { - eval.eval_path_from(&p, &e, &graph_name2) - })) + self.run_if_term_is_a_graph_node(start, graph_name, || { + let eval = self.clone(); + let p = p.clone(); + let graph_name2 = graph_name.clone(); + Box::new(transitive_closure(Some(Ok(start.clone())), move |e| { + eval.eval_path_from(&p, &e, &graph_name2) + })) + }) } PlanPropertyPath::OneOrMore(p) => { let eval = self.clone(); @@ -785,9 +787,13 @@ impl SimpleEvaluator { move |e| eval.eval_path_from(&p, &e, &graph_name2), )) } - PlanPropertyPath::ZeroOrOne(p) => Box::new(hash_deduplicate( - once(Ok(start.clone())).chain(self.eval_path_from(p, start, graph_name)), - )), + PlanPropertyPath::ZeroOrOne(p) => { + self.run_if_term_is_a_graph_node(start, graph_name, || { + Box::new(hash_deduplicate( + once(Ok(start.clone())).chain(self.eval_path_from(p, start, graph_name)), + )) + }) + } PlanPropertyPath::NegatedPropertySet(ps) => { let ps = ps.clone(); Box::new( @@ -835,12 +841,14 @@ impl SimpleEvaluator { .chain(self.eval_path_to(b, end, graph_name)), ), PlanPropertyPath::ZeroOrMore(p) => { - let eval = self.clone(); - let p = p.clone(); - let graph_name2 = graph_name.clone(); - Box::new(transitive_closure(Some(Ok(end.clone())), move |e| { - eval.eval_path_to(&p, &e, &graph_name2) - })) + self.run_if_term_is_a_graph_node(end, graph_name, || { + let eval = self.clone(); + let p = p.clone(); + let graph_name2 = graph_name.clone(); + Box::new(transitive_closure(Some(Ok(end.clone())), move |e| { + eval.eval_path_to(&p, &e, &graph_name2) + })) + }) } PlanPropertyPath::OneOrMore(p) => { let eval = self.clone(); @@ -851,9 +859,13 @@ impl SimpleEvaluator { move |e| eval.eval_path_to(&p, &e, &graph_name2), )) } - PlanPropertyPath::ZeroOrOne(p) => Box::new(hash_deduplicate( - once(Ok(end.clone())).chain(self.eval_path_to(p, end, graph_name)), - )), + PlanPropertyPath::ZeroOrOne(p) => { + self.run_if_term_is_a_graph_node(end, graph_name, || { + Box::new(hash_deduplicate( + once(Ok(end.clone())).chain(self.eval_path_to(p, end, graph_name)), + )) + }) + } PlanPropertyPath::NegatedPropertySet(ps) => { let ps = ps.clone(); Box::new( @@ -959,8 +971,46 @@ impl SimpleEvaluator { ) -> impl Iterator> { self.dataset .encoded_quads_for_pattern(None, None, None, Some(graph_name)) - .flat_map_ok(|t| once(Ok(t.subject)).chain(once(Ok(t.object)))) - .map(|e| e.map(|e| (e.clone(), e))) + .flat_map_ok(|t| { + [ + Ok((t.subject.clone(), t.subject)), + Ok((t.object.clone(), t.object)), + ] + }) + } + + fn run_if_term_is_a_graph_node( + &self, + term: &EncodedTerm, + graph_name: &EncodedTerm, + f: impl FnOnce() -> Box>>, + ) -> Box>> { + match self.is_subject_or_object_in_dataset(term, graph_name) { + Ok(true) => f(), + Ok(false) => { + Box::new(empty()) // Not in the database + } + Err(error) => Box::new(once(Err(error))), + } + } + + fn is_subject_or_object_in_dataset( + &self, + term: &EncodedTerm, + graph_name: &EncodedTerm, + ) -> Result { + Ok(self + .dataset + .encoded_quads_for_pattern(Some(term), None, None, Some(graph_name)) + .next() + .transpose()? + .is_some() + || self + .dataset + .encoded_quads_for_pattern(None, None, Some(term), Some(graph_name)) + .next() + .transpose()? + .is_some()) } #[allow(clippy::cast_possible_truncation, clippy::cast_precision_loss)] diff --git a/testsuite/oxigraph-tests/sparql/manifest.ttl b/testsuite/oxigraph-tests/sparql/manifest.ttl index 156b7e1f..19e6fbc9 100644 --- a/testsuite/oxigraph-tests/sparql/manifest.ttl +++ b/testsuite/oxigraph-tests/sparql/manifest.ttl @@ -30,6 +30,7 @@ :unbound_variable_in_subquery :values_too_many :values_too_few + :values_property_path_all ) . :small_unicode_escape_with_multibytes_char rdf:type mf:NegativeSyntaxTest ; @@ -139,3 +140,9 @@ :values_too_few rdf:type mf:NegativeSyntaxTest11 ; mf:name "Too few values in a VALUE clause compared to the number of variable" ; mf:action . + +:values_property_path_all rdf:type mf:QueryEvaluationTest ; + mf:name "ZeroOrX property paths should only return terms in the graph and not also terms defined in the query" ; + mf:action + [ qt:query ] ; + mf:result . diff --git a/testsuite/oxigraph-tests/sparql/values_property_path_all.rq b/testsuite/oxigraph-tests/sparql/values_property_path_all.rq new file mode 100644 index 00000000..ef4d6c8d --- /dev/null +++ b/testsuite/oxigraph-tests/sparql/values_property_path_all.rq @@ -0,0 +1,4 @@ +SELECT * WHERE { + VALUES ?v { 1 } + ?v ? ?v +} \ No newline at end of file diff --git a/testsuite/oxigraph-tests/sparql/values_property_path_all.srx b/testsuite/oxigraph-tests/sparql/values_property_path_all.srx new file mode 100644 index 00000000..0632c2aa --- /dev/null +++ b/testsuite/oxigraph-tests/sparql/values_property_path_all.srx @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file