Use iterators instead of materialized values

src/executors.rs

@@ -1,256 +1,208 @@
 use std::collections::{HashMap, HashSet};
+use std::rc::Rc;
 
 use mercator_db::space;
 use mercator_db::Core;
 use mercator_db::CoreQueryParameters;
-use mercator_db::Properties;
+use mercator_db::IterObjects;
+use mercator_db::IterObjectsBySpaces;
 
 use super::expressions::*;
 use super::symbols::*;
 
-impl From<&LiteralPosition> for space::Position {
-    fn from(literal: &LiteralPosition) -> Self {
-        let v: Vec<f64> = literal.into();
-        v.into()
+fn group_by_space<'s>(
+    list: IterObjectsBySpaces<'s>,
+) -> Box<dyn Iterator<Item = (&'s String, IterObjects<'s>)> + 's> {
+    // Filter per Properties, in order to regroup by it, then build
+    // a single SpatialObject per Properties.
+    let mut hashmap = HashMap::new();
+    for (space, objects) in list {
+        hashmap.entry(space).or_insert_with(Vec::new).push(objects);
     }
+
+    Box::new(hashmap.into_iter().map(|(space, objects)| {
+        let objects: IterObjects = Box::new(objects.into_iter().flatten());
+        (space, objects)
+    }))
 }
 
-impl From<&LiteralNumber> for space::Coordinate {
-    fn from(literal: &LiteralNumber) -> Self {
-        match literal {
-            LiteralNumber::Float(f) => (*f).into(),
-            LiteralNumber::Int(i) => (*i as u64).into(),
-        }
-    }
+fn distinct_helper(list: IterObjectsBySpaces) -> IterObjectsBySpaces {
+    // Make sure to collect all objects iterators per space, so that
+    // each space appears only once.
+    group_by_space(list)
+        // We would lose some objects otherwise when creating the
+        // HashMaps. Also this makes sure to keep the values are unique.
+        .map(|(space, iter)| {
+            let uniques: HashSet<_> = iter.collect();
+            let uniques: IterObjects = Box::new(uniques.into_iter());
+            (space, uniques)
+        })
+        .collect()
 }
 
-fn complement_helper<'c>(
-    core: &'c Core,
-    parameters: &CoreQueryParameters<'c>,
-    space_id: &str,
-    inside: Vec<(&'c String, Vec<(space::Position, &'c Properties)>)>,
-) -> mercator_db::ResultSet<'c> {
+fn into_positions_hashset(
+    objects_by_spaces: IterObjectsBySpaces,
+) -> HashMap<&String, Rc<HashSet<space::Position>>> {
+    // Make sure to collect all objects iterators per space, so that
+    // each space appears only once.
+    group_by_space(objects_by_spaces)
+        // We would lose some objects otherwise when creating the HashSets.
+        .map(|(space, iter)| {
+            let hash_set: HashSet<_> = iter.map(|(position, _)| position).collect();
+            (space, Rc::new(hash_set))
+        })
+        .collect::<HashMap<_, _>>()
+}
+
+// Strictly not inside nor on the surface.
+// TODO: inside must contains the valid positions in all expected spaces
+fn complement_helper<'h>(
+    core: &'h Core,
+    parameters: &'h CoreQueryParameters<'h>,
+    space_id: &'h str,
+    inside: IterObjectsBySpaces<'h>,
+) -> mercator_db::ResultSet<'h> {
     let (low, high) = parameters.db.space(space_id)?.bounding_box();
-    match core.get_by_shape(parameters, &space::Shape::BoundingBox(low, high), space_id) {
-        e @ Err(_) => e,
-        Ok(points) => {
-            let hashmap = inside.into_iter().collect::<HashMap<_, _>>();
+    let inside = into_positions_hashset(inside);
+    let points = core.get_by_shape(parameters, space::Shape::BoundingBox(low, high), space_id)?;
 
-            Ok(points
+    let results = points
         .into_iter()
-                .filter_map(|(space, v)| match hashmap.get(space) {
-                    None => None,
-                    Some(list) => {
-                        Some((space, v.into_iter().filter(|t| !list.contains(t)).collect()))
+        .filter_map(move |(space, v)| match inside.get(space) {
+            None => None, // Space not found, so no point might exist!
+            Some(volume) => {
+                let volume = volume.clone();
+                let iter: IterObjects = Box::new(v.filter(move |a| !volume.contains(&a.0)));
+
+                Some((space, iter))
             }
         })
-                .collect::<Vec<_>>())
-        }
-    }
+        .collect();
+    Ok(results)
 }
 
-fn view_port<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    bag: &Bag,
-) -> mercator_db::ResultSet<'c> {
-    if let Some((low, high)) = parameters.view_port {
-        let vp = Bag::Inside(Shape::HyperRectangle(
-            bag.space().clone(),
-            vec![low.into(), high.into()],
-        ));
-        intersection(core_id, parameters, &vp, bag)
-    } else {
-        bag.execute(core_id, parameters)
-    }
-}
+// Intersection based only on spatial positions!
+fn intersect_helper<'h>(
+    smaller: IterObjectsBySpaces<'h>,
+    bigger: IterObjectsBySpaces<'h>,
+) -> IterObjectsBySpaces<'h> {
+    let smaller = into_positions_hashset(smaller);
 
-fn distinct<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    bag: &Bag,
-) -> mercator_db::ResultSet<'c> {
-    match bag.execute(core_id, parameters) {
-        e @ Err(_) => e,
-        Ok(mut v) => {
-            let set: HashSet<_> = v.drain(..).collect(); // dedup
-            v.extend(set.into_iter());
-
-            Ok(v)
-        }
-    }
-}
-fn filter_helper<'c>(
-    predicate: &Predicate,
-    bag: &Bag,
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-) -> mercator_db::ResultSet<'c> {
-    match bag.execute(core_id, parameters) {
-        e @ Err(_) => e,
-        Ok(results) => Ok(results
+    bigger
         .into_iter()
-            .filter_map(|(space, positions)| {
-                let filtered = positions
-                    .into_iter()
-                    .filter(|(position, properties)| predicate.eval((space, position, properties)))
-                    .collect::<Vec<_>>();
-                if filtered.is_empty() {
-                    None
-                } else {
+        .filter_map(
+            move |(space, bigger_object_iter)| match smaller.get(space) {
+                None => None,
+                Some(volume) => {
+                    let volume = volume.clone();
+                    let filtered: IterObjects =
+                        Box::new(bigger_object_iter.filter(move |a| volume.contains(&a.0)));
+
                     Some((space, filtered))
                 }
-            })
-            .collect::<Vec<_>>()),
-    }
-}
-
-fn filter<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    predicate: &Option<Predicate>,
-    bag: &Option<Box<Bag>>,
-) -> mercator_db::ResultSet<'c> {
-    match predicate {
-        None => {
-            if let Some(bag) = bag {
-                bag.execute(core_id, parameters)
-            } else {
-                Err("Filter without predicate nor data set.".to_string())
-            }
-        }
-        Some(predicate) => match bag {
-            None => {
-                let (low, high) = space::Space::universe().bounding_box();
-                let low: Vec<_> = low.into();
-                let high: Vec<_> = high.into();
-                let shape = Shape::HyperRectangle(
-                    space::Space::universe().name().clone(),
-                    vec![
-                        LiteralPosition(
-                            low.into_iter()
-                                .map(LiteralNumber::Float)
-                                .collect::<Vec<_>>(),
-                        ),
-                        LiteralPosition(
-                            high.into_iter()
-                                .map(LiteralNumber::Float)
-                                .collect::<Vec<_>>(),
-                        ),
-                    ],
-                );
-                filter_helper(predicate, &Bag::Inside(shape), core_id, parameters)
-            }
-            Some(bag) => filter_helper(predicate, bag.as_ref(), core_id, parameters),
             },
-    }
+        )
+        .collect()
 }
 
-fn complement<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    core: &'c Core,
-    bag: &Bag,
-) -> mercator_db::ResultSet<'c> {
-    match bag.execute(core_id, parameters) {
-        // FIXME: The complement of a set is computed within its definition space.
-        e @ Err(_) => e,
-        Ok(inside) => complement_helper(
+impl Bag {
+    fn distinct<'b>(
+        &'b self,
+        core_id: &'b str,
+        parameters: &'b CoreQueryParameters<'b>,
+    ) -> mercator_db::ResultSet<'b> {
+        let results = self.execute(core_id, parameters)?;
+
+        Ok(distinct_helper(results))
+    }
+
+    fn complement<'b>(
+        &'b self,
+        core_id: &'b str,
+        parameters: &'b CoreQueryParameters<'b>,
+        core: &'b Core,
+    ) -> mercator_db::ResultSet<'b> {
+        let inside = self.execute(core_id, parameters)?;
+
+        // FIXME: The complement of a set should be computed within its
+        //        definition space. We don't know here so we use universe
+        complement_helper(
             core,
             parameters,
             mercator_db::space::Space::universe().name(),
             inside,
-        ),
+        )
     }
-}
 
-fn intersection<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    rh: &Bag,
-    lh: &Bag,
-) -> mercator_db::ResultSet<'c> {
-    let l = lh.execute(core_id, parameters);
-    if let Ok(l) = l {
-        let r = rh.execute(core_id, parameters);
-        if let Ok(r) = r {
-            let mut v = vec![];
+    fn intersection<'b>(
+        &'b self,
+        core_id: &'b str,
+        parameters: &'b CoreQueryParameters<'b>,
+        rh: &'b Bag,
+    ) -> mercator_db::ResultSet<'b> {
+        let left = self.execute(core_id, parameters)?;
+        let right = rh.execute(core_id, parameters)?;
 
-            if rh.predict(parameters.db) < lh.predict(parameters.db) {
-                for o in r {
-                    if l.contains(&o) {
-                        v.push(o);
-                    }
-                }
+        let v = if rh.predict(parameters.db) < self.predict(parameters.db) {
+            intersect_helper(right, left)
         } else {
-                for o in l {
-                    if r.contains(&o) {
-                        v.push(o);
-                    }
-                }
-            }
-            Ok(v)
-        } else {
-            r
-        }
-    } else {
-        l
-    }
-}
-
-fn union<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    rh: &Bag,
-    lh: &Bag,
-) -> mercator_db::ResultSet<'c> {
-    let l = lh.execute(core_id, parameters);
-    if let Ok(mut l) = l {
-        let r = rh.execute(core_id, parameters);
-        if let Ok(mut r) = r {
-            if rh.predict(parameters.db) < lh.predict(parameters.db) {
-                l.append(&mut r);
-                Ok(l)
-            } else {
-                r.append(&mut l);
-                Ok(r)
-            }
-        } else {
-            r
-        }
-    } else {
-        l
-    }
-}
-
-fn bag<'c>(
-    core_id: &str,
-    parameters: &CoreQueryParameters<'c>,
-    bags: &[Bag],
-) -> mercator_db::ResultSet<'c> {
-    let mut v = vec![];
-    for bag in bags {
-        let b = bag.execute(core_id, parameters);
-        match b {
-            e @ Err(_) => {
-                return e;
-            }
-            Ok(mut b) => {
-                v.append(&mut b);
-            }
-        }
-    }
+            intersect_helper(left, right)
+        };
 
         Ok(v)
+    }
+
+    fn union<'b>(
+        &'b self,
+        core_id: &'b str,
+        parameters: &'b CoreQueryParameters<'b>,
+        rh: &'b Bag,
+    ) -> mercator_db::ResultSet<'b> {
+        let mut left = self.execute(core_id, parameters)?;
+        let mut right = rh.execute(core_id, parameters)?;
+
+        let union = if rh.predict(parameters.db) < self.predict(parameters.db) {
+            left.append(&mut right);
+            left
+        } else {
+            right.append(&mut left);
+            right
+        };
+
+        Ok(union)
+    }
+
+    fn filter<'b>(
+        &'b self,
+        predicate: &'b Predicate,
+        core_id: &'b str,
+        parameters: &'b CoreQueryParameters<'b>,
+    ) -> mercator_db::ResultSet<'b> {
+        let results = self.execute(core_id, parameters)?;
+
+        Ok(results
+            .into_iter()
+            .map(move |(space, positions)| {
+                let positions = positions.collect::<Vec<_>>();
+                (
+                    space,
+                    Box::new(positions.into_iter().filter(move |(position, properties)| {
+                        predicate.eval((space, position, properties))
+                    })) as IterObjects,
+                )
+            })
+            .collect())
+    }
 }
 
-fn inside<'c>(
-    parameters: &CoreQueryParameters<'c>,
-    core: &'c Core,
-    shape: &Shape,
-) -> mercator_db::ResultSet<'c> {
+impl Shape {
+    fn inside<'s>(
+        &'s self,
+        parameters: &'s CoreQueryParameters<'s>,
+        core: &'s Core,
+    ) -> mercator_db::ResultSet<'s> {
         let db = parameters.db;
-    let param = match shape {
+        let param = match self {
             Shape::Point(space_id, position) => {
                 let space = db.space(space_id)?;
                 let position: Vec<f64> = position.into();
@@ -259,7 +211,11 @@ fn inside<'c>(
             }
             Shape::HyperRectangle(space_id, bounding_box) => {
                 if bounding_box.len() != 2 {
-                Err("The number of position is different from 2, which is unsupported.".to_string())
+                    //FIXME: Support arbitrary HyperRectangles
+                    Err(
+                        "The number of position is different from 2, which is unsupported."
+                            .to_string(),
+                    )
                 } else {
                     let space = db.space(space_id)?;
                     let low: Vec<f64> = (&bounding_box[0]).into();
@@ -274,13 +230,13 @@ fn inside<'c>(
                 let space = db.space(space_id)?;
                 let position: Vec<f64> = position.into();
                 let position = space.encode(&position)?;
-            let mut r = vec![];
-            for _ in 0..position.dimensions() {
-                r.push(radius.into());
-            }
+
+                // We have to provide a position with all the dimensions
+                // for the encoding to work as expected.
+                let mut r = vec![0f64; position.dimensions()];
+                r[0] = radius.into();
                 let radius = space.encode(&r)?[0];
 
-            //FIXME: RADIUS IS A LENGTH, HOW TO ENCODE IT INTO THE SPACE?
                 Ok((space_id, space::Shape::HyperSphere(position, radius)))
             }
             Shape::Label(_, id) => {
@@ -291,23 +247,24 @@ fn inside<'c>(
         };
 
         match param {
-        Ok((space_id, shape)) => core.get_by_shape(parameters, &shape, space_id),
+            Ok((space_id, shape)) => core.get_by_shape(parameters, shape, space_id),
             Err(e) => Err(e),
         }
-}
+    }
 
-fn outside<'c>(
-    parameters: &CoreQueryParameters<'c>,
-    core: &'c Core,
-    shape: &Shape,
-) -> mercator_db::ResultSet<'c> {
-    match shape {
+    fn outside<'s>(
+        &'s self,
+        parameters: &'s CoreQueryParameters<'s>,
+        core: &'s Core,
+    ) -> mercator_db::ResultSet<'s> {
+        let (space_id, inside) = match self {
             Shape::Point(space_id, position) => {
                 let position: Vec<f64> = position.into();
-            match core.get_by_positions(parameters, &[position.into()], space_id) {
-                e @ Err(_) => e,
-                Ok(inside) => complement_helper(core, parameters, space_id, inside),
-            }
+                let mut positions = Vec::with_capacity(1);
+                positions.push(position.into());
+                let inside = core.get_by_positions(parameters, positions, space_id)?;
+
+                Ok((space_id, inside))
             }
             Shape::HyperRectangle(space_id, bounding_box) => {
                 // We need to adapt the bounding_box to ensure the
@@ -329,10 +286,10 @@ fn outside<'c>(
                 let mut high: space::Position = (&bounding_box[1]).into();
                 high -= increment.into();
 
-            match core.get_by_shape(parameters, &space::Shape::BoundingBox(low, high), space_id) {
-                e @ Err(_) => e,
-                Ok(inside) => complement_helper(core, parameters, space_id, inside),
-            }
+                let inside =
+                    core.get_by_shape(parameters, space::Shape::BoundingBox(low, high), space_id)?;
+
+                Ok((space_id, inside))
             }
             Shape::HyperSphere(space_id, center, radius) => {
                 // Smallest decrement possible, to exclude the surface
@@ -340,27 +297,59 @@ fn outside<'c>(
                 radius -= std::f64::EPSILON;
                 let center: space::Position = center.into();
 
-            match core.get_by_shape(
+                let inside = core.get_by_shape(
                     parameters,
-                &space::Shape::HyperSphere(center, radius.into()),
+                    space::Shape::HyperSphere(center, radius.into()),
                     space_id,
-            ) {
-                e @ Err(_) => e,
-                Ok(inside) => complement_helper(core, parameters, space_id, inside),
+                )?;
+
+                Ok((space_id, inside))
             }
+            Shape::Label(space_id, id) => {
+                let inside = core.get_by_label(parameters, id)?;
+
+                Ok((space_id, inside))
             }
-        Shape::Label(_, _) => Err("Label: not yet implemented".to_string()),
             Shape::Nifti(_space_id) => Err("Outside-nifti: not yet implemented".to_string()),
+        }?;
+
+        complement_helper(core, parameters, space_id, inside)
     }
 }
 
+fn filter<'c>(
+    core_id: &'c str,
+    parameters: &'c CoreQueryParameters<'c>,
+    predicate: &'c Option<Predicate>,
+    bag: &'c Bag,
+) -> mercator_db::ResultSet<'c> {
+    match predicate {
+        None => bag.execute(core_id, parameters),
+        Some(predicate) => bag.filter(predicate, core_id, parameters),
+    }
+}
+
+fn bag<'c>(
+    core_id: &'c str,
+    parameters: &'c CoreQueryParameters<'c>,
+    bags: &'c [Bag],
+) -> mercator_db::ResultSet<'c> {
+    let mut results = Vec::new();
+    for bag in bags {
+        let mut result = bag.execute(core_id, parameters)?;
+        results.append(&mut result);
+    }
+
+    Ok(results)
+}
+
 impl<'e> Executor<'e> for Projection {
     type ResultSet = mercator_db::ResultSet<'e>;
 
-    fn execute<'f: 'e>(
-        &self,
-        core_id: &str,
-        parameters: &CoreQueryParameters<'f>,
+    fn execute(
+        &'e self,
+        core_id: &'e str,
+        parameters: &'e CoreQueryParameters<'e>,
     ) -> Self::ResultSet {
         match self {
             Projection::Nifti(_, _, _bag) => Err("Proj-Nifti: not yet implemented".to_string()),
@@ -375,27 +364,26 @@ impl<'e> Executor<'e> for Projection {
 impl<'e> Executor<'e> for Bag {
     type ResultSet = mercator_db::ResultSet<'e>;
 
-    fn execute<'f: 'e>(
-        &self,
-        core_id: &str,
-        parameters: &CoreQueryParameters<'f>,
+    fn execute(
+        &'e self,
+        core_id: &'e str,
+        parameters: &'e CoreQueryParameters<'e>,
     ) -> Self::ResultSet {
         let core = parameters.db.core(core_id)?;
 
         match self {
-            Bag::ViewPort(bag) => view_port(core_id, parameters, bag),
-            Bag::Distinct(bag) => distinct(core_id, parameters, bag),
+            Bag::Distinct(bag) => bag.distinct(core_id, parameters),
             Bag::Filter(predicate, bag) => filter(core_id, parameters, predicate, bag),
-            Bag::Complement(bag) => complement(core_id, parameters, core, bag),
-            Bag::Intersection(lh, rh) => intersection(core_id, parameters, rh, lh),
-            Bag::Union(lh, rh) => union(core_id, parameters, rh, lh),
+            Bag::Complement(bag) => bag.complement(core_id, parameters, core),
+            Bag::Intersection(lh, rh) => lh.intersection(core_id, parameters, rh),
+            Bag::Union(lh, rh) => lh.union(core_id, parameters, rh),
             Bag::Bag(list) => bag(core_id, parameters, list),
-            Bag::Inside(shape) => inside(parameters, core, shape),
+            Bag::Inside(shape) => shape.inside(parameters, core),
             Bag::Outside(shape) => {
                 //FIXME: This is currently computed as the complement of the values within the shape, except its surface.
                 //       Should this be instead a list of positions within the shape?
                 //FIXME: Should we use the Shape's Space to get the maximum bounds or the output Space requested?
-                outside(parameters, core, shape)
+                shape.outside(parameters, core)
             }
         }
     }

src/lib.rs

@@ -59,6 +59,8 @@ pub use expressions::Predictor;
 pub use expressions::Validator;
 pub use queries::FiltersParser;
 pub use queries::QueryParser;
+pub use symbols::Bag;
+pub use symbols::Projection;
 pub use validators::ValidationResult;
 
 #[cfg(test)]

src/main.rs

@@ -93,12 +93,17 @@ fn main() {
                             execute = t.execute(core, &parameters);
                         }
 
-                        if let Ok(r) = execute {
-                            //let r = mercator_db::json::model::to_spatial_objects(r);
+                        match execute {
+                            Ok(r) => {
+                                let r = r
+                                    .into_iter()
+                                    .map(|(space, objects)| (space, objects.collect::<Vec<_>>()))
+                                    .collect::<Vec<_>>();
+
                                 info!("Execution: \n{:#?}", r);
-                            info!("NB results: {:?}", r.len());
-                        } else {
-                            info!("Execution: \n{:?}", execute);
+                                info!("NB results: {:?}", r[0].1.len());
+                            }
+                            Err(e) => info!("Execution: \n{:?}", e),
                         }
                     }
                 }

src/predictors.rs

@@ -15,7 +15,6 @@ impl Predictor for Projection {
 impl Predictor for Bag {
     fn predict(&self, db: &DataBase) -> Result<f64, String> {
         match self {
-            Bag::ViewPort(bag) => bag.predict(db),
             Bag::Distinct(bag) => bag.predict(db),
             Bag::Filter(_, bag) => bag.predict(db),
             Bag::Complement(bag) => Ok(db.space(bag.space())?.volume() - bag.predict(db)?),

src/queries.lalrpop

@@ -137,8 +137,6 @@ Aggregations: symbols::Aggregation = {
 //*********************************************************************/
 pub Filters: symbols::Bag = {
     <Bags>
-    //<Bags> =>
-    //    symbols::Bag::ViewPort(Box::new(<>))
 };
 
 // All these expressions generate bags.

src/symbols.rs

@@ -57,8 +57,6 @@ struct Transform {
 /**********************************************************************/
 #[derive(Clone, Debug)]
 pub enum Bag {
-    // This is an implicit operator, inserted by the parser. Never to be used directly.
-    ViewPort(Box<Bag>),
     // Bags
     Distinct(Box<Bag>),
     Filter(Option<Predicate>, Box<Bag>),
@@ -75,7 +73,6 @@ pub enum Bag {
 impl Bag {
     pub fn space(&self) -> &String {
         match self {
-            Bag::ViewPort(bag) => bag.space(),
             Bag::Distinct(bag) => bag.space(),
             Bag::Filter(_, bag) => bag.space(),
             Bag::Complement(bag) => bag.space(),
@@ -240,7 +237,11 @@ impl Position {
                     Ordering::Greater => 1,
                     Ordering::Less => -1,
                 };
-                LiteralPosition(vec![LiteralNumber::Int(x)])
+
+                let mut v = Vec::with_capacity(1);
+                v.push(LiteralNumber::Int(x));
+
+                LiteralPosition(v)
             }
         }
     }
@@ -259,14 +260,33 @@ pub enum LiteralNumber {
     Float(f64),
 }
 
-impl From<&LiteralNumber> for Vec<f64> {
+impl From<&LiteralNumber> for f64 {
     fn from(l: &LiteralNumber) -> Self {
-        let r = match l {
+        match l {
             LiteralNumber::Int(x) => (*x) as f64,
             LiteralNumber::Float(x) => *x,
-        };
+        }
+    }
+}
 
-        vec![r]
+impl From<LiteralNumber> for f64 {
+    fn from(l: LiteralNumber) -> Self {
+        (&l).into()
+    }
+}
+
+impl From<&LiteralNumber> for space::Coordinate {
+    fn from(literal: &LiteralNumber) -> Self {
+        match literal {
+            LiteralNumber::Float(f) => (*f).into(),
+            LiteralNumber::Int(i) => (*i as u64).into(),
+        }
+    }
+}
+
+impl From<LiteralNumber> for space::Coordinate {
+    fn from(literal: LiteralNumber) -> Self {
+        (&literal).into()
     }
 }
 
@@ -291,7 +311,7 @@ pub struct LiteralPosition(pub Vec<LiteralNumber>);
 impl LiteralPosition {
     pub fn get_type(&self) -> LiteralTypes {
         let Self(v) = self;
-        let mut t = Vec::new();
+        let mut t = Vec::with_capacity(v.len());
 
         for n in v {
             t.push(match n {
@@ -324,34 +344,35 @@ impl LiteralPosition {
     }
 }
 
-impl From<&LiteralNumber> for f64 {
-    fn from(l: &LiteralNumber) -> Self {
-        match l {
-            LiteralNumber::Int(x) => (*x) as f64,
-            LiteralNumber::Float(x) => *x,
+impl From<&LiteralPosition> for Vec<f64> {
+    fn from(l: &LiteralPosition) -> Self {
+        // Speed-wise this should be the same, the downside is the newly
+        // allocated vector might be suboptimal in terms of space.
+        //let LiteralPosition(v) = l;
+        //v.iter().map(|literal| literal.into()).collect()
+
+        let LiteralPosition(v) = l;
+        let mut lv = Vec::with_capacity(v.len());
+        for value in v {
+            lv.push(value.into());
         }
+
+        lv
     }
 }
 
-impl From<&LiteralPosition> for Vec<f64> {
-    fn from(l: &LiteralPosition) -> Self {
-        let LiteralPosition(v) = l;
-        let mut r = Vec::with_capacity(v.len());
-
-        for x in v {
-            let x = match x {
-                LiteralNumber::Int(x) => (*x) as f64,
-                LiteralNumber::Float(x) => *x,
-            };
-            r.push(x);
-        }
-
-        r
+impl From<LiteralPosition> for Vec<f64> {
+    fn from(l: LiteralPosition) -> Self {
+        (&l).into()
     }
 }
 
 impl From<&Vec<f64>> for LiteralPosition {
     fn from(v: &Vec<f64>) -> Self {
+        // Speed-wise this should be the same, the downside is the newly
+        // allocated vector might be suboptimal in terms of space.
+        //LiteralPosition(v.iter().map(|value| LiteralNumber::Float(*value)).collect())
+
         let mut lv = Vec::with_capacity(v.len());
         for value in v {
             lv.push(LiteralNumber::Float(*value));
@@ -360,10 +381,36 @@ impl From<&Vec<f64>> for LiteralPosition {
         LiteralPosition(lv)
     }
 }
+
+impl From<Vec<f64>> for LiteralPosition {
+    fn from(v: Vec<f64>) -> Self {
+        (&v).into()
+    }
+}
+
 impl From<&space::Position> for LiteralPosition {
     fn from(position: &space::Position) -> Self {
-        let lv: Vec<f64> = position.into();
-        (&lv).into()
+        let position: Vec<f64> = position.into();
+        position.into()
+    }
+}
+
+impl From<space::Position> for LiteralPosition {
+    fn from(position: space::Position) -> Self {
+        (&position).into()
+    }
+}
+
+impl From<&LiteralPosition> for space::Position {
+    fn from(position: &LiteralPosition) -> Self {
+        let position: Vec<f64> = position.into();
+        position.into()
+    }
+}
+
+impl From<LiteralPosition> for space::Position {
+    fn from(position: LiteralPosition) -> Self {
+        (&position).into()
     }
 }
 

src/validators.rs

@@ -54,7 +54,6 @@ impl Validator for Bag {
         }
 
         match self {
-            Bag::ViewPort(bag) => bag.validate(),
             Bag::Distinct(bag) => bag.validate(),
             Bag::Filter(_, bag) => bag.validate(),
             Bag::Complement(bag) => bag.validate(),