GraphPattern: Renames visible_variables into on_in_scope_variable

spargebra/src/algebra.rs

@@ -780,95 +780,106 @@ impl Default for GraphPattern {
 }
 
 impl GraphPattern {
-    /// Returns the list of [in-scope variables](https://www.w3.org/TR/sparql11-query/#variableScope)
-    pub fn visible_variables(&self) -> BTreeSet<&Variable> {
-        let mut vars = BTreeSet::default();
-        self.add_visible_variables(&mut vars);
-        vars
+    /// Calls `callback` on each [in-scope variable](https://www.w3.org/TR/sparql11-query/#variableScope) occurrence
+    pub fn on_in_scope_variable<'a>(&'a self, mut callback: impl FnMut(&'a Variable)) {
+        self.lookup_in_scope_variables(&mut callback)
     }
 
-    fn add_visible_variables<'a>(&'a self, vars: &mut BTreeSet<&'a Variable>) {
+    fn lookup_in_scope_variables<'a>(&'a self, callback: &mut impl FnMut(&'a Variable)) {
         match self {
             Self::Bgp(p) => {
                 for pattern in p {
-                    add_triple_pattern_variables(pattern, vars)
+                    lookup_triple_pattern_variables(pattern, callback)
                 }
             }
             Self::Path {
                 subject, object, ..
             } => {
                 if let TermPattern::Variable(s) = subject {
-                    vars.insert(s);
+                    callback(s);
                 }
                 #[cfg(feature = "rdf-star")]
                 if let TermPattern::Triple(s) = subject {
-                    add_triple_pattern_variables(s, vars)
+                    lookup_triple_pattern_variables(s, callback)
                 }
                 if let TermPattern::Variable(o) = object {
-                    vars.insert(o);
+                    callback(o);
                 }
                 #[cfg(feature = "rdf-star")]
                 if let TermPattern::Triple(o) = object {
-                    add_triple_pattern_variables(o, vars)
+                    lookup_triple_pattern_variables(o, callback)
                 }
             }
             Self::Sequence(elements) => {
                 for e in elements {
-                    e.add_visible_variables(vars);
+                    e.lookup_in_scope_variables(callback);
                 }
             }
             Self::Join { left, right }
             | Self::LeftJoin { left, right, .. }
             | Self::Union { left, right } => {
-                left.add_visible_variables(vars);
-                right.add_visible_variables(vars);
+                left.lookup_in_scope_variables(callback);
+                right.lookup_in_scope_variables(callback);
             }
             Self::Graph { graph_name, inner } => {
                 if let NamedNodePattern::Variable(ref g) = graph_name {
-                    vars.insert(g);
+                    callback(g);
                 }
-                inner.add_visible_variables(vars);
+                inner.lookup_in_scope_variables(callback);
             }
             Self::Extend { inner, var, .. } => {
-                vars.insert(var);
-                inner.add_visible_variables(vars);
+                callback(var);
+                inner.lookup_in_scope_variables(callback);
             }
-            Self::Minus { left, .. } => left.add_visible_variables(vars),
-            Self::Service { pattern, .. } => pattern.add_visible_variables(vars),
+            Self::Minus { left, .. } => left.lookup_in_scope_variables(callback),
+            Self::Service { pattern, .. } => pattern.lookup_in_scope_variables(callback),
             Self::Group { by, aggregates, .. } => {
-                vars.extend(by);
+                for v in by {
+                    callback(v);
+                }
                 for (v, _) in aggregates {
-                    vars.insert(v);
+                    callback(v);
+                }
+            }
+            Self::Table { variables, .. } => {
+                for v in variables {
+                    callback(v);
+                }
+            }
+            Self::Project { projection, .. } => {
+                for v in projection {
+                    callback(v);
                 }
             }
-            Self::Table { variables, .. } => vars.extend(variables),
-            Self::Project { projection, .. } => vars.extend(projection.iter()),
             Self::Filter { inner, .. }
             | Self::OrderBy { inner, .. }
             | Self::Distinct { inner }
             | Self::Reduced { inner }
-            | Self::Slice { inner, .. } => inner.add_visible_variables(vars),
+            | Self::Slice { inner, .. } => inner.lookup_in_scope_variables(callback),
         }
     }
 }
 
-fn add_triple_pattern_variables<'a>(pattern: &'a TriplePattern, vars: &mut BTreeSet<&'a Variable>) {
+fn lookup_triple_pattern_variables<'a>(
+    pattern: &'a TriplePattern,
+    callback: &mut impl FnMut(&'a Variable),
+) {
     if let TermPattern::Variable(s) = &pattern.subject {
-        vars.insert(s);
+        callback(s);
     }
     #[cfg(feature = "rdf-star")]
     if let TermPattern::Triple(s) = &pattern.subject {
-        add_triple_pattern_variables(s, vars)
+        lookup_triple_pattern_variables(s, callback)
     }
     if let NamedNodePattern::Variable(p) = &pattern.predicate {
-        vars.insert(p);
+        callback(p);
     }
     if let TermPattern::Variable(o) = &pattern.object {
-        vars.insert(o);
+        callback(o);
     }
     #[cfg(feature = "rdf-star")]
     if let TermPattern::Triple(o) = &pattern.object {
-        add_triple_pattern_variables(o, vars)
+        lookup_triple_pattern_variables(o, callback)
     }
 }
 

spargebra/src/parser.rs

@@ -514,7 +514,10 @@ fn build_select(
     let mut pv = Vec::new();
     let with_project = match select.variables {
         SelectionVariables::Explicit(sel_items) => {
-            let visible: HashSet<_> = p.visible_variables().into_iter().cloned().collect();
+            let mut visible = HashSet::default();
+            p.on_in_scope_variable(|v| {
+                visible.insert(v.clone());
+            });
             for sel_item in sel_items {
                 let v = match sel_item {
                     SelectionMember::Variable(v) => {
@@ -556,7 +559,13 @@ fn build_select(
             if with_aggregate {
                 return Err("SELECT * is not authorized with GROUP BY");
             }
-            pv.extend(p.visible_variables().into_iter().cloned()); //TODO: is it really useful to do a projection?
+            //TODO: is it really useful to do a projection?
+            p.on_in_scope_variable(|v| {
+                if !pv.contains(v) {
+                    pv.push(v.clone());
+                }
+            });
+            pv.sort();
             true
         }
         SelectionVariables::Everything => false,
@@ -1390,7 +1399,13 @@ parser! {
                         g = GraphPattern::Minus { left: Box::new(g), right: Box::new(p) }
                     }
                     PartialGraphPattern::Bind(expr, var) => {
-                        if g.visible_variables().contains(&var) {
+                        let mut contains = false;
+                        g.on_in_scope_variable(|v| {
+                            if *v == var {
+                                contains = true;
+                            }
+                        });
+                        if contains {
                             return Err("BIND is overriding an existing variable")
                         }
                         g = GraphPattern::Extend { inner: Box::new(g), var, expr }