From e96672a2a88af9951d037b7ae40de423d705700e Mon Sep 17 00:00:00 2001 From: Tpt Date: Wed, 3 May 2023 13:57:44 +0200 Subject: [PATCH] SPARQL plan: allows AND and OR to have more than 2 children Adds tests on VALUES cardinality validation --- lib/src/sparql/eval.rs | 52 ++++++++++++++++++++--------- lib/src/sparql/plan.rs | 38 ++++++++++++++++----- lib/src/sparql/plan_builder.rs | 61 +++++++++++++++++++--------------- 3 files changed, 100 insertions(+), 51 deletions(-) diff --git a/lib/src/sparql/eval.rs b/lib/src/sparql/eval.rs index 3650e2d0..e95d2092 100644 --- a/lib/src/sparql/eval.rs +++ b/lib/src/sparql/eval.rs @@ -954,23 +954,45 @@ impl SimpleEvaluator { stat_children.push(stats); Rc::new(move |tuple| Some(eval(tuple.clone()).next().is_some().into())) } - PlanExpression::Or(a, b) => { - let a = self.expression_evaluator(a, stat_children); - let b = self.expression_evaluator(b, stat_children); - Rc::new(move |tuple| match a(tuple).and_then(|v| to_bool(&v)) { - Some(true) => Some(true.into()), - Some(false) => b(tuple), - None => (Some(true) == a(tuple).and_then(|v| to_bool(&v))).then(|| true.into()), + PlanExpression::Or(inner) => { + let children = inner + .iter() + .map(|i| self.expression_evaluator(i, stat_children)) + .collect::>(); + Rc::new(move |tuple| { + let mut error = true; + for child in children.iter() { + match child(tuple).and_then(|v| to_bool(&v)) { + Some(true) => return Some(true.into()), + Some(false) => continue, + None => error = true, + } + } + if error { + None + } else { + Some(false.into()) + } }) } - PlanExpression::And(a, b) => { - let a = self.expression_evaluator(a, stat_children); - let b = self.expression_evaluator(b, stat_children); - Rc::new(move |tuple| match a(tuple).and_then(|v| to_bool(&v)) { - Some(true) => b(tuple), - Some(false) => Some(false.into()), - None => { - (Some(false) == b(tuple).and_then(|v| to_bool(&v))).then(|| false.into()) + PlanExpression::And(inner) => { + let children = inner + .iter() + .map(|i| self.expression_evaluator(i, stat_children)) + .collect::>(); + Rc::new(move |tuple| { + let mut error = false; + for child in children.iter() { + match child(tuple).and_then(|v| to_bool(&v)) { + Some(true) => continue, + Some(false) => return Some(false.into()), + None => error = true, + } + } + if error { + None + } else { + Some(true.into()) } }) } diff --git a/lib/src/sparql/plan.rs b/lib/src/sparql/plan.rs index c9af7f22..88469433 100644 --- a/lib/src/sparql/plan.rs +++ b/lib/src/sparql/plan.rs @@ -13,7 +13,7 @@ use std::rc::Rc; use std::time::Duration; use std::{fmt, io}; -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum PlanNode { StaticBindings { encoded_tuples: Vec, @@ -447,8 +447,8 @@ pub enum PlanExpression { Literal(PlanTerm), Variable(PlanVariable), Exists(Rc), - Or(Box, Box), - And(Box, Box), + Or(Vec), + And(Vec), Equal(Box, Box), Greater(Box, Box), GreaterOrEqual(Box, Box), @@ -597,9 +597,7 @@ impl PlanExpression { | Self::YearMonthDurationCast(e) | Self::DayTimeDurationCast(e) | Self::StringCast(e) => e.lookup_used_variables(callback), - Self::Or(a, b) - | Self::And(a, b) - | Self::Equal(a, b) + Self::Equal(a, b) | Self::Greater(a, b) | Self::GreaterOrEqual(a, b) | Self::Less(a, b) @@ -639,7 +637,11 @@ impl PlanExpression { c.lookup_used_variables(callback); d.lookup_used_variables(callback); } - Self::Concat(es) | Self::Coalesce(es) | Self::CustomFunction(_, es) => { + Self::Or(es) + | Self::And(es) + | Self::Concat(es) + | Self::Coalesce(es) + | Self::CustomFunction(_, es) => { for e in es { e.lookup_used_variables(callback); } @@ -723,8 +725,26 @@ impl fmt::Display for PlanExpression { Self::YearMonthDurationCast(e) => write!(f, "YearMonthDurationCast({e})"), Self::DayTimeDurationCast(e) => write!(f, "DayTimeDurationCast({e})"), Self::StringCast(e) => write!(f, "StringCast({e})"), - Self::Or(a, b) => write!(f, "Or({a}, {b})"), - Self::And(a, b) => write!(f, "And({a}, {b})"), + Self::Or(es) => { + write!(f, "Or(")?; + for (i, e) in es.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{e}")?; + } + write!(f, ")") + } + Self::And(es) => { + write!(f, "And(")?; + for (i, e) in es.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{e}")?; + } + write!(f, ")") + } Self::Equal(a, b) => write!(f, "Equal({a}, {b})"), Self::Greater(a, b) => write!(f, "Greater({a}, {b})"), Self::GreaterOrEqual(a, b) => write!(f, "GreaterOrEqual({a}, {b})"), diff --git a/lib/src/sparql/plan_builder.rs b/lib/src/sparql/plan_builder.rs index 9e6472ec..a0d6b603 100644 --- a/lib/src/sparql/plan_builder.rs +++ b/lib/src/sparql/plan_builder.rs @@ -400,14 +400,14 @@ impl<'a> PlanBuilder<'a> { plain: l.clone(), }), Expression::Variable(v) => PlanExpression::Variable(build_plan_variable(variables, v)), - Expression::Or(a, b) => PlanExpression::Or( - Box::new(self.build_for_expression(a, variables, graph_name)?), - Box::new(self.build_for_expression(b, variables, graph_name)?), - ), - Expression::And(a, b) => PlanExpression::And( - Box::new(self.build_for_expression(a, variables, graph_name)?), - Box::new(self.build_for_expression(b, variables, graph_name)?), - ), + Expression::Or(a, b) => PlanExpression::Or(vec![ + self.build_for_expression(a, variables, graph_name)?, + self.build_for_expression(b, variables, graph_name)?, + ]), + Expression::And(a, b) => PlanExpression::And(vec![ + self.build_for_expression(a, variables, graph_name)?, + self.build_for_expression(b, variables, graph_name)?, + ]), Expression::Equal(a, b) => PlanExpression::Equal( Box::new(self.build_for_expression(a, variables, graph_name)?), Box::new(self.build_for_expression(b, variables, graph_name)?), @@ -433,23 +433,23 @@ impl<'a> PlanBuilder<'a> { Box::new(self.build_for_expression(b, variables, graph_name)?), ), Expression::In(e, l) => { + if l.is_empty() { + return Ok(PlanExpression::Literal(PlanTerm { + encoded: false.into(), + plain: false.into(), + })); + } let e = self.build_for_expression(e, variables, graph_name)?; - l.iter() - .map(|v| { - Ok(PlanExpression::Equal( - Box::new(e.clone()), - Box::new(self.build_for_expression(v, variables, graph_name)?), - )) - }) - .reduce(|a: Result<_, EvaluationError>, b| { - Ok(PlanExpression::Or(Box::new(a?), Box::new(b?))) - }) - .unwrap_or_else(|| { - Ok(PlanExpression::Literal(PlanTerm { - encoded: false.into(), - plain: false.into(), - })) - })? + PlanExpression::Or( + l.iter() + .map(|v| { + Ok(PlanExpression::Equal( + Box::new(e.clone()), + Box::new(self.build_for_expression(v, variables, graph_name)?), + )) + }) + .collect::>()?, + ) } Expression::Add(a, b) => PlanExpression::Add( Box::new(self.build_for_expression(a, variables, graph_name)?), @@ -1402,8 +1402,12 @@ impl<'a> PlanBuilder<'a> { expression: filter, }; } - if let PlanExpression::And(f1, f2) = *filter { - return self.push_filter(Rc::new(self.push_filter(node, f1)), f2); + if let PlanExpression::And(filters) = *filter { + return filters + .into_iter() + .fold((*node.as_ref()).clone(), |acc, f| { + self.push_filter(Rc::new(acc), Box::new(f)) + }); } let mut filter_variables = BTreeSet::new(); filter.lookup_used_variables(&mut |v| { @@ -1492,7 +1496,10 @@ impl<'a> PlanBuilder<'a> { } else { PlanNode::Filter { child: Rc::clone(child), - expression: Box::new(PlanExpression::And(expression.clone(), filter)), + expression: Box::new(PlanExpression::And(vec![ + *expression.clone(), + *filter, + ])), } } }