From 83d54b39c48459ffe2c117e834e7b847ed1df58a Mon Sep 17 00:00:00 2001 From: Tpt Date: Wed, 8 Sep 2021 08:46:26 +0200 Subject: [PATCH] GraphPattern: Renames visible_variables into on_in_scope_variable --- spargebra/src/algebra.rs | 73 +++++++++++++++++++++++----------------- spargebra/src/parser.rs | 21 ++++++++++-- 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/spargebra/src/algebra.rs b/spargebra/src/algebra.rs index 4c3eda43..121159a3 100644 --- a/spargebra/src/algebra.rs +++ b/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) } } diff --git a/spargebra/src/parser.rs b/spargebra/src/parser.rs index c7bdf5c7..2a044a51 100644 --- a/spargebra/src/parser.rs +++ b/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 }