Fixes RDF-star support

fuzz/fuzz_targets/sparql_eval.rs

@@ -3,7 +3,7 @@
 use lazy_static::lazy_static;
 use libfuzzer_sys::fuzz_target;
 use oxigraph::io::DatasetFormat;
-use oxigraph::sparql::{Query, QueryOptions, QueryResults, QuerySolutionIter};
+use oxigraph::sparql::{Query, QueryOptions, QueryResults, QuerySolutionIter, RuleSet};
 use oxigraph::store::Store;
 
 lazy_static! {
@@ -26,24 +26,39 @@ fuzz_target!(|data: sparql_smith::Query| {
         let options = QueryOptions::default();
         let with_opt = STORE.query_opt(query.clone(), options.clone()).unwrap();
         let without_opt = STORE
-            .query_opt(query, options.without_optimizations())
+            .query_opt(query.clone(), options.clone().without_optimizations())
             .unwrap();
-        match (with_opt, without_opt) {
+        compare_results(with_opt, without_opt, &query_str);
-            (QueryResults::Solutions(with_opt), QueryResults::Solutions(without_opt)) => {
+        let with_opt_and_reasoning = STORE
-                assert_eq!(
+            .query_opt(
-                    query_solutions_key(with_opt, query_str.contains(" REDUCED ")),
+                query.clone(),
-                    query_solutions_key(without_opt, query_str.contains(" REDUCED "))
+                options
-                )
+                    .clone()
-            }
+                    .with_inference_rules(RuleSet::default())
-            (QueryResults::Graph(_), QueryResults::Graph(_)) => unimplemented!(),
+                    .clone(),
-            (QueryResults::Boolean(with_opt), QueryResults::Boolean(without_opt)) => {
+            )
-                assert_eq!(with_opt, without_opt)
+            .unwrap();
-            }
+        let with_opt = STORE.query_opt(query.clone(), options).unwrap();
-            _ => panic!("Different query result types"),
+        compare_results(with_opt, with_opt_and_reasoning, &query_str);
-        }
     }
 });
 
+fn compare_results(a: QueryResults, b: QueryResults, query_str: &str) {
+    match (a, b) {
+        (QueryResults::Solutions(a), QueryResults::Solutions(b)) => {
+            assert_eq!(
+                query_solutions_key(a, query_str.contains(" REDUCED ")),
+                query_solutions_key(b, query_str.contains(" REDUCED "))
+            )
+        }
+        (QueryResults::Graph(_), QueryResults::Graph(_)) => unimplemented!(),
+        (QueryResults::Boolean(a), QueryResults::Boolean(b)) => {
+            assert_eq!(a, b)
+        }
+        _ => panic!("Different query result types"),
+    }
+}
+
 fn query_solutions_key(iter: QuerySolutionIter, is_reduced: bool) -> String {
     // TODO: ordering
     let mut b = iter

lib/sparopt/src/algebra.rs

@@ -1406,6 +1406,56 @@ impl FixedPointGraphPattern {
             Self::FixedPointEntry(tid) => id == tid,
         }
     }
+
+    pub(crate) fn lookup_used_variables(&self, callback: &mut impl FnMut(&Variable)) {
+        match self {
+            FixedPointGraphPattern::QuadPattern {
+                subject,
+                predicate,
+                object,
+                graph_name,
+            } => {
+                lookup_ground_term_pattern_used_variables(subject, callback);
+                if let NamedNodePattern::Variable(v) = predicate {
+                    callback(v);
+                }
+                lookup_ground_term_pattern_used_variables(object, callback);
+                if let Some(NamedNodePattern::Variable(v)) = graph_name {
+                    callback(v);
+                }
+            }
+            FixedPointGraphPattern::Join { left, right } => {
+                left.lookup_used_variables(callback);
+                right.lookup_used_variables(callback);
+            }
+            FixedPointGraphPattern::Filter { inner, .. } => {
+                inner.lookup_used_variables(callback);
+                // TODO: we assume all expression variables are bound
+            }
+            FixedPointGraphPattern::Union { inner } => {
+                for inner in inner {
+                    inner.lookup_used_variables(callback);
+                }
+            }
+            FixedPointGraphPattern::Extend {
+                inner, variable, ..
+            } => {
+                inner.lookup_used_variables(callback);
+                callback(variable);
+                // TODO: we assume all expression variables are bound
+            }
+            FixedPointGraphPattern::Values { variables, .. }
+            | FixedPointGraphPattern::Project { variables, .. }
+            | FixedPointGraphPattern::FixedPoint { variables, .. } => {
+                for v in variables {
+                    callback(v);
+                }
+            }
+            FixedPointGraphPattern::FixedPointEntry(_) => {
+                //TODO
+            }
+        }
+    }
 }
 
 impl TryFrom<FixedPointGraphPattern> for GraphPattern {
@@ -1837,3 +1887,18 @@ impl From<&OrderExpression> for AlOrderExpression {
 fn new_var() -> Variable {
     Variable::new_unchecked(format!("{:x}", random::<u128>()))
 }
+
+fn lookup_ground_term_pattern_used_variables(
+    pattern: &GroundTermPattern,
+    callback: &mut impl FnMut(&Variable),
+) {
+    if let GroundTermPattern::Variable(v) = pattern {
+        callback(v)
+    } else if let GroundTermPattern::Triple(t) = pattern {
+        lookup_ground_term_pattern_used_variables(&t.subject, callback);
+        if let NamedNodePattern::Variable(v) = &t.predicate {
+            callback(v);
+        }
+        lookup_ground_term_pattern_used_variables(&t.object, callback);
+    }
+}

lib/sparopt/src/reasoning.rs

@@ -292,17 +292,12 @@ impl QueryRewriter {
                 self.rewrite_expression(then),
                 self.rewrite_expression(els),
             ),
-            Expression::Coalesce(inners) => Expression::coalesce(
+            Expression::Coalesce(inners) => {
-                inners
+                Expression::coalesce(inners.iter().map(|a| self.rewrite_expression(a)).collect())
-                    .into_iter()
+            }
-                    .map(|a| self.rewrite_expression(a))
-                    .collect(),
-            ),
             Expression::FunctionCall(name, args) => Expression::call(
                 name.clone(),
-                args.into_iter()
+                args.iter().map(|a| self.rewrite_expression(a)).collect(),
-                    .map(|a| self.rewrite_expression(a))
-                    .collect(),
             ),
         }
     }
@@ -369,9 +364,7 @@ impl QueryRewriter {
             }
             let mut plan = FixedPointGraphPattern::FixedPointEntry(*fixed_point_id);
             for (from, to) in &variable_mapping {
-                if from != to {
+                plan = Self::copy_variable(plan, from.clone().into(), to.clone());
-                    plan = FixedPointGraphPattern::extend(plan, to.clone(), from.clone().into());
-                }
             }
             return FixedPointGraphPattern::project(
                 plan,
@@ -390,15 +383,11 @@ impl QueryRewriter {
 
         // We get the output variables list:
         let mut output_variables = Vec::new();
-        if let GroundTermPattern::Variable(v) = subject {
+        Self::add_pattern_variables(subject, &mut output_variables);
-            output_variables.push(v.clone());
-        }
         if let NamedNodePattern::Variable(v) = predicate {
             output_variables.push(v.clone());
         }
-        if let GroundTermPattern::Variable(v) = object {
+        Self::add_pattern_variables(object, &mut output_variables);
-            output_variables.push(v.clone());
-        }
         if let Some(NamedNodePattern::Variable(v)) = graph_name {
             output_variables.push(v.clone());
         }
@@ -453,12 +442,8 @@ impl QueryRewriter {
             }
             PropertyPathExpression::Sequence(left, right) => {
                 let mut final_variables = Vec::new();
-                if let GroundTermPattern::Variable(v) = subject {
+                Self::add_pattern_variables(subject, &mut final_variables);
-                    final_variables.push(v.clone());
+                Self::add_pattern_variables(object, &mut final_variables);
-                }
-                if let GroundTermPattern::Variable(v) = object {
-                    final_variables.push(v.clone());
-                }
                 if let Some(NamedNodePattern::Variable(v)) = graph_name {
                     final_variables.push(v.clone());
                 }
@@ -552,8 +537,25 @@ impl QueryRewriter {
                 ),
                 _ => FixedPointGraphPattern::empty(),
             },
-            GroundTermPattern::Triple(_) => todo!(),
+            GroundTermPattern::Triple(_) => {
-            GroundTermPattern::Variable(subject) => match object {
+                let new_var = new_var();
+                let mut output_variables = Vec::new();
+                Self::add_pattern_variables(subject, &mut output_variables);
+                Self::add_pattern_variables(object, &mut output_variables);
+                FixedPointGraphPattern::project(
+                    Self::pattern_mapping(
+                        self.zero_graph_pattern(
+                            &GroundTermPattern::Variable(new_var.clone()),
+                            object,
+                            graph_name,
+                        ),
+                        new_var.into(),
+                        subject.clone(),
+                    ),
+                    output_variables,
+                )
+            }
+            parent_subject @ GroundTermPattern::Variable(subject) => match object {
                 GroundTermPattern::NamedNode(object) => FixedPointGraphPattern::values(
                     vec![subject.clone()],
                     vec![vec![Some(object.clone().into())]],
@@ -562,7 +564,24 @@ impl QueryRewriter {
                     vec![subject.clone()],
                     vec![vec![Some(object.clone().into())]],
                 ),
-                GroundTermPattern::Triple(_) => todo!(),
+                GroundTermPattern::Triple(_) => {
+                    let new_var = new_var();
+                    let mut output_variables = Vec::new();
+                    Self::add_pattern_variables(parent_subject, &mut output_variables);
+                    Self::add_pattern_variables(object, &mut output_variables);
+                    FixedPointGraphPattern::project(
+                        Self::pattern_mapping(
+                            self.zero_graph_pattern(
+                                &GroundTermPattern::Variable(new_var.clone()),
+                                parent_subject,
+                                graph_name,
+                            ),
+                            new_var.into(),
+                            object.clone(),
+                        ),
+                        output_variables,
+                    )
+                }
                 GroundTermPattern::Variable(object) => {
                     let s = new_var();
                     let p = new_var();
@@ -573,30 +592,30 @@ impl QueryRewriter {
                     }
                     let base_pattern = self.rewrite_quad_pattern(
                         &s.clone().into(),
-                        &p.clone().into(),
+                        &p.into(),
                         &o.clone().into(),
                         graph_name,
                         &mut Vec::new(),
                     );
                     FixedPointGraphPattern::project(
                         FixedPointGraphPattern::union(
-                            FixedPointGraphPattern::extend(
+                            Self::copy_variable(
-                                FixedPointGraphPattern::extend(
+                                Self::copy_variable(
                                     base_pattern.clone(),
-                                    subject.clone(),
                                     s.clone().into(),
+                                    subject.clone(),
                                 ),
-                                object.clone(),
                                 s.into(),
+                                object.clone(),
                             ),
-                            FixedPointGraphPattern::extend(
+                            Self::copy_variable(
-                                FixedPointGraphPattern::extend(
+                                Self::copy_variable(
                                     base_pattern,
-                                    subject.clone(),
                                     o.clone().into(),
+                                    subject.clone(),
                                 ),
-                                object.clone(),
                                 o.into(),
+                                object.clone(),
                             ),
                         ),
                         final_variables,
@@ -615,12 +634,8 @@ impl QueryRewriter {
         fix_point_counter: usize,
     ) -> FixedPointGraphPattern {
         let mut final_variables = Vec::new();
-        if let GroundTermPattern::Variable(v) = subject {
+        Self::add_pattern_variables(subject, &mut final_variables);
-            final_variables.push(v.clone());
+        Self::add_pattern_variables(object, &mut final_variables);
-        }
-        if let GroundTermPattern::Variable(v) = object {
-            final_variables.push(v.clone());
-        }
         if let Some(NamedNodePattern::Variable(v)) = graph_name {
             final_variables.push(v.clone());
         }
@@ -653,10 +668,10 @@ impl QueryRewriter {
                             FixedPointGraphPattern::project(
                                 FixedPointGraphPattern::join(
                                     FixedPointGraphPattern::project(
-                                        FixedPointGraphPattern::extend(
+                                        Self::copy_variable(
                                             FixedPointGraphPattern::FixedPointEntry(fix_point_id),
-                                            middle_var.clone(),
                                             end_var.clone().into(),
+                                            middle_var.clone(),
                                         ),
                                         middle_variables,
                                     ),
@@ -674,10 +689,10 @@ impl QueryRewriter {
                         in_loop_variables,
                     ),
                     start_var.into(),
-                    subject,
+                    subject.clone(),
                 ),
                 end_var.into(),
-                object,
+                object.clone(),
             ),
             final_variables,
         )
@@ -686,16 +701,16 @@ impl QueryRewriter {
     fn pattern_mapping(
         pattern: FixedPointGraphPattern,
         pattern_value: FixedPointExpression,
-        target: &GroundTermPattern,
+        target: GroundTermPattern,
     ) -> FixedPointGraphPattern {
         match target {
             GroundTermPattern::NamedNode(target) => FixedPointGraphPattern::filter(
                 pattern,
-                FixedPointExpression::same_term(pattern_value, target.clone().into()),
+                FixedPointExpression::same_term(pattern_value, target.into()),
             ),
             GroundTermPattern::Literal(target) => FixedPointGraphPattern::filter(
                 pattern,
-                FixedPointExpression::same_term(target.clone().into(), pattern_value),
+                FixedPointExpression::same_term(target.into(), pattern_value),
             ),
             GroundTermPattern::Triple(target) => Self::pattern_mapping(
                 Self::pattern_mapping(
@@ -712,26 +727,60 @@ impl QueryRewriter {
                                 ),
                             )
                         }
-                        NamedNodePattern::Variable(target_predicate) => {
+                        NamedNodePattern::Variable(target_predicate) => Self::copy_variable(
-                            FixedPointGraphPattern::extend(
+                            pattern,
-                                pattern,
+                            FixedPointExpression::call(
-                                target_predicate.clone(),
+                                Function::Predicate,
-                                FixedPointExpression::call(
+                                vec![pattern_value.clone()],
-                                    Function::Predicate,
+                            ),
-                                    vec![pattern_value.clone()],
+                            target_predicate.clone(),
-                                ),
+                        ),
-                            )
-                        }
                     },
                     FixedPointExpression::call(Function::Subject, vec![pattern_value.clone()]),
-                    &target.subject,
+                    target.subject,
                 ),
                 FixedPointExpression::call(Function::Object, vec![pattern_value]),
-                &target.object,
+                target.object,
             ),
             GroundTermPattern::Variable(target) => {
-                FixedPointGraphPattern::extend(pattern, target.clone(), pattern_value)
+                Self::copy_variable(pattern, pattern_value, target)
+            }
+        }
+    }
+
+    fn copy_variable(
+        pattern: FixedPointGraphPattern,
+        from_expression: FixedPointExpression,
+        to_variable: Variable,
+    ) -> FixedPointGraphPattern {
+        if from_expression == FixedPointExpression::from(to_variable.clone()) {
+            return pattern;
+        }
+        let mut does_target_exists = false;
+        pattern.lookup_used_variables(&mut |v| {
+            if *v == to_variable {
+                does_target_exists = true;
+            }
+        });
+        if does_target_exists {
+            FixedPointGraphPattern::filter(
+                pattern,
+                FixedPointExpression::same_term(from_expression, to_variable.into()),
+            )
+        } else {
+            FixedPointGraphPattern::extend(pattern, to_variable, from_expression)
+        }
+    }
+
+    fn add_pattern_variables(pattern: &GroundTermPattern, variables: &mut Vec<Variable>) {
+        if let GroundTermPattern::Variable(v) = pattern {
+            variables.push(v.clone())
+        } else if let GroundTermPattern::Triple(t) = pattern {
+            Self::add_pattern_variables(&t.subject, variables);
+            if let NamedNodePattern::Variable(v) = &t.predicate {
+                variables.push(v.clone());
             }
+            Self::add_pattern_variables(&t.object, variables);
         }
     }
 

lib/src/sparql/plan.rs

@@ -438,18 +438,10 @@ impl FixedPointPlanNode {
                 object,
                 graph_name,
             } => {
-                if let PatternValue::Variable(var) = subject {
+                subject.lookup_variables(callback);
-                    callback(var.encoded);
+                predicate.lookup_variables(callback);
-                }
+                object.lookup_variables(callback);
-                if let PatternValue::Variable(var) = predicate {
+                graph_name.lookup_variables(callback);
-                    callback(var.encoded);
-                }
-                if let PatternValue::Variable(var) = object {
-                    callback(var.encoded);
-                }
-                if let PatternValue::Variable(var) = graph_name {
-                    callback(var.encoded);
-                }
             }
             Self::Filter { child, expression } => {
                 expression.lookup_used_variables(callback);

testsuite/src/sparql_evaluator.rs

@@ -198,11 +198,17 @@ fn evaluate_evaluation_test(test: &Test) -> Result<()> {
         false
     };
 
-    for with_query_optimizer in [true, false] {
+    enum Mode {
-        let mut options = options.clone();
+        Base,
-        if !with_query_optimizer {
+        WithoutOptimizer,
-            options = options.without_optimizations();
+        WithInferenceRules,
-        }
+    }
+    for mode in [Mode::Base, Mode::WithoutOptimizer, Mode::WithInferenceRules] {
+        let options = match mode {
+            Mode::Base => options.clone(),
+            Mode::WithoutOptimizer => options.clone().without_optimizations(),
+            Mode::WithInferenceRules => options.clone().with_inference_rules(RuleSet::default()),
+        };
         let actual_results = store
             .query_opt(query.clone(), options)
             .map_err(|e| anyhow!("Failure to execute query of {test} with error: {e}"))?;
@@ -210,7 +216,12 @@ fn evaluate_evaluation_test(test: &Test) -> Result<()> {
 
         if !are_query_results_isomorphic(&expected_results, &actual_results) {
             bail!(
-                "Failure on {test}.\n{}\nParsed query:\n{}\nData:\n{store}\n",
+                "Failure{} on {test}.\n{}\nParsed query:\n{}\nData:\n{store}\n",
+                match mode {
+                    Mode::Base => "",
+                    Mode::WithoutOptimizer => " without optimizer",
+                    Mode::WithInferenceRules => " with inference rules",
+                },
                 results_diff(expected_results, actual_results),
                 Query::parse(&read_file_to_string(query_file)?, Some(query_file)).unwrap()
             );