Adds GraphPattern::Sequence

Safe version of GraphPattern::Join

lib/src/sparql/plan_builder.rs

@@ -43,41 +43,13 @@ impl<'a> PlanBuilder<'a> {
         graph_name: &PatternValue,
     ) -> Result<PlanNode, EvaluationError> {
         Ok(match pattern {
-            GraphPattern::Bgp(p) => self.build_for_bgp(p, variables, graph_name),
+            GraphPattern::Bgp(_) | GraphPattern::Path { .. } | GraphPattern::Sequence { .. } => {
-            GraphPattern::Path {
+                self.build_sequence(PlanNode::Init, pattern, variables, graph_name)?
-                subject,
-                path,
-                object,
-            } => PlanNode::PathPatternJoin {
-                child: Rc::new(PlanNode::Init),
-                subject: self.pattern_value_from_term_or_variable(subject, variables),
-                path: Rc::new(self.build_for_path(path)),
-                object: self.pattern_value_from_term_or_variable(object, variables),
-                graph_name: graph_name.clone(),
-            },
-            GraphPattern::Join { left, right } => {
-                //TODO: improve
-                if let GraphPattern::Path {
-                    subject,
-                    path,
-                    object,
-                } = right.as_ref()
-                {
-                    let left = self.build_for_graph_pattern(left, variables, graph_name)?;
-                    PlanNode::PathPatternJoin {
-                        child: Rc::new(left),
-                        subject: self.pattern_value_from_term_or_variable(subject, variables),
-                        path: Rc::new(self.build_for_path(path)),
-                        object: self.pattern_value_from_term_or_variable(object, variables),
-                        graph_name: graph_name.clone(),
             }
-                } else {
+            GraphPattern::Join { left, right } => PlanNode::Join {
-                    PlanNode::Join {
                 left: Rc::new(self.build_for_graph_pattern(left, variables, graph_name)?),
                 right: Rc::new(self.build_for_graph_pattern(right, variables, graph_name)?),
-                    }
+            },
-                }
-            }
             GraphPattern::LeftJoin { left, right, expr } => {
                 let left = self.build_for_graph_pattern(left, variables, graph_name)?;
                 let right = self.build_for_graph_pattern(right, variables, graph_name)?;
@@ -271,24 +243,53 @@ impl<'a> PlanBuilder<'a> {
         })
     }
 
-    fn build_for_bgp(
+    fn build_sequence(
         &mut self,
-        p: &[TriplePattern],
+        mut plan: PlanNode,
+        pattern: &GraphPattern,
         variables: &mut Vec<Variable>,
         graph_name: &PatternValue,
-    ) -> PlanNode {
+    ) -> Result<PlanNode, EvaluationError> {
-        let mut plan = PlanNode::Init;
+        match pattern {
-        for pattern in sort_bgp(p) {
+            GraphPattern::Bgp(p) => {
+                for triple in sort_bgp(p) {
                     plan = PlanNode::QuadPatternJoin {
                         child: Rc::new(plan),
-                subject: self.pattern_value_from_term_or_variable(&pattern.subject, variables),
+                        subject: self
-                predicate: self
+                            .pattern_value_from_term_or_variable(&triple.subject, variables),
-                    .pattern_value_from_named_node_or_variable(&pattern.predicate, variables),
+                        predicate: self.pattern_value_from_named_node_or_variable(
-                object: self.pattern_value_from_term_or_variable(&pattern.object, variables),
+                            &triple.predicate,
+                            variables,
+                        ),
+                        object: self.pattern_value_from_term_or_variable(&triple.object, variables),
                         graph_name: graph_name.clone(),
                     }
                 }
-        plan
+                Ok(plan)
+            }
+            GraphPattern::Path {
+                subject,
+                path,
+                object,
+            } => Ok(PlanNode::PathPatternJoin {
+                child: Rc::new(plan),
+                subject: self.pattern_value_from_term_or_variable(subject, variables),
+                path: Rc::new(self.build_for_path(path)),
+                object: self.pattern_value_from_term_or_variable(object, variables),
+                graph_name: graph_name.clone(),
+            }),
+            GraphPattern::Graph { inner, graph_name } => {
+                let graph_name =
+                    self.pattern_value_from_named_node_or_variable(graph_name, variables);
+                self.build_sequence(plan, inner, variables, &graph_name)
+            }
+            GraphPattern::Sequence(elements) => elements.iter().fold(Ok(plan), |plan, element| {
+                self.build_sequence(plan?, element, variables, graph_name)
+            }),
+            _ => Err(EvaluationError::msg(
+                "Unexpected element in a sequence: {:?}.",
+            )),
+        }
     }
 
     fn build_for_path(&mut self, path: &PropertyPathExpression) -> PlanPropertyPath {

spargebra/src/algebra.rs

@@ -470,6 +470,9 @@ pub enum GraphPattern {
         path: PropertyPathExpression,
         object: TermPattern,
     },
+    /// A set of SPARQL patterns that can be evaluated sequentially
+    /// It is a safe case of [Join](https://www.w3.org/TR/sparql11-query/#defn_algJoin)
+    Sequence(Vec<GraphPattern>),
     /// [Join](https://www.w3.org/TR/sparql11-query/#defn_algJoin)
     Join { left: Box<Self>, right: Box<Self> },
     /// [LeftJoin](https://www.w3.org/TR/sparql11-query/#defn_algLeftJoin)
@@ -548,6 +551,13 @@ impl fmt::Debug for GraphPattern {
                 path,
                 object,
             } => write!(f, "(path {:?} {:?} {:?})", subject, path, object),
+            Self::Sequence(elements) => {
+                write!(f, "(sequence")?;
+                for e in elements {
+                    write!(f, " {:?}", e)?;
+                }
+                write!(f, ")")
+            }
             Self::Join { left, right } => write!(f, "(join {:?} {:?})", left, right),
             Self::LeftJoin { left, right, expr } => {
                 if let Some(expr) = expr {
@@ -665,6 +675,12 @@ impl fmt::Display for GraphPattern {
                 path,
                 object,
             } => write!(f, "{} {} {} .", subject, path, object),
+            Self::Sequence(elements) => {
+                for e in elements {
+                    write!(f, "{} ", e)?;
+                }
+                Ok(())
+            }
             Self::Join { left, right } => {
                 if matches!(
                     right.as_ref(),
@@ -796,6 +812,11 @@ impl GraphPattern {
                     add_triple_pattern_variables(o, vars)
                 }
             }
+            Self::Sequence(elements) => {
+                for e in elements {
+                    e.add_visible_variables(vars);
+                }
+            }
             Self::Join { left, right }
             | Self::LeftJoin { left, right, .. }
             | Self::Union { left, right } => {

spargebra/src/parser.rs

@@ -11,6 +11,7 @@ use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
 use std::convert::{TryFrom, TryInto};
 use std::error::Error;
+use std::mem::take;
 use std::str::Chars;
 use std::str::FromStr;
 use std::{char, fmt};
@@ -250,8 +251,8 @@ fn add_triple_to_triple_or_path_patterns(
 }
 
 fn build_bgp(patterns: Vec<TripleOrPathPattern>) -> GraphPattern {
-    let mut bgp = Vec::with_capacity(patterns.len());
+    let mut bgp = Vec::new();
-    let mut paths = Vec::with_capacity(patterns.len());
+    let mut elements = Vec::with_capacity(patterns.len());
     for pattern in patterns {
         match pattern {
             TripleOrPathPattern::Triple(t) => bgp.push(t),
@@ -259,21 +260,22 @@ fn build_bgp(patterns: Vec<TripleOrPathPattern>) -> GraphPattern {
                 subject,
                 path,
                 object,
-            } => paths.push((subject, path, object)),
+            } => {
+                if !bgp.is_empty() {
+                    elements.push(GraphPattern::Bgp(take(&mut bgp)));
                 }
-    }
+                elements.push(GraphPattern::Path {
-    let mut graph_pattern = GraphPattern::Bgp(bgp);
-    for (subject, path, object) in paths {
-        graph_pattern = new_join(
-            graph_pattern,
-            GraphPattern::Path {
                     subject,
                     path,
                     object,
-            },
+                })
-        )
             }
-    graph_pattern
+        }
+    }
+    if !bgp.is_empty() {
+        elements.push(GraphPattern::Bgp(bgp));
+    }
+    new_sequence(elements)
 }
 
 enum TripleOrPathPattern {
@@ -364,12 +366,29 @@ fn new_join(l: GraphPattern, r: GraphPattern) -> GraphPattern {
         }
     }
 
-    //Merge BGPs
+    // Some optimizations
+    // TODO: move to a specific optimizer pass
     match (l, r) {
         (GraphPattern::Bgp(mut pl), GraphPattern::Bgp(pr)) => {
             pl.extend(pr);
             GraphPattern::Bgp(pl)
         }
+        (GraphPattern::Sequence(mut e1), GraphPattern::Sequence(e2)) => {
+            e1.extend_from_slice(&e2);
+            GraphPattern::Sequence(e1)
+        }
+        (GraphPattern::Sequence(mut e), r)
+            if matches!(r, GraphPattern::Bgp(_) | GraphPattern::Path { .. }) =>
+        {
+            e.push(r);
+            GraphPattern::Sequence(e)
+        }
+        (l, GraphPattern::Sequence(mut e))
+            if matches!(l, GraphPattern::Bgp(_) | GraphPattern::Path { .. }) =>
+        {
+            e.insert(0, l);
+            GraphPattern::Sequence(e)
+        }
         (
             GraphPattern::Graph {
                 graph_name: g1,
@@ -393,6 +412,14 @@ fn new_join(l: GraphPattern, r: GraphPattern) -> GraphPattern {
     }
 }
 
+fn new_sequence(elements: Vec<GraphPattern>) -> GraphPattern {
+    match elements.len() {
+        0 => GraphPattern::Bgp(vec![]),
+        1 => elements.into_iter().next().unwrap(),
+        _ => GraphPattern::Sequence(elements),
+    }
+}
+
 fn not_empty_fold<T>(
     iter: impl Iterator<Item = T>,
     combine: impl Fn(T, T) -> T,
@@ -1185,14 +1212,14 @@ parser! {
 
         //[40]
         rule DeleteWhere() -> Vec<GraphUpdateOperation> = i("DELETE") _ i("WHERE") _ d:QuadPattern() {?
-            let pattern = d.iter().map(|q| {
+            let pattern = new_sequence(d.iter().map(|q| {
                 let bgp = GraphPattern::Bgp(vec![TriplePattern::new(q.subject.clone(), q.predicate.clone(), q.object.clone())]);
                 match &q.graph_name {
                     GraphNamePattern::NamedNode(graph_name) => GraphPattern::Graph { graph_name: graph_name.clone().into(), inner: Box::new(bgp) },
                     GraphNamePattern::DefaultGraph => bgp,
                     GraphNamePattern::Variable(graph_name) => GraphPattern::Graph { graph_name: graph_name.clone().into(), inner: Box::new(bgp) },
                 }
-            }).fold(GraphPattern::Bgp(Vec::new()), new_join);
+            }).collect());
             let delete = d.into_iter().map(GroundQuadPattern::try_from).collect::<Result<Vec<_>,_>>().map_err(|_| "Blank nodes are not allowed in DELETE WHERE")?;
             Ok(vec![GraphUpdateOperation::DeleteInsert {
                 delete,