From 24c9fc527900860da4d3d9e76bbe5a83807c8520 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 17 Dec 2021 18:12:03 +0700
Subject: [PATCH] refactoring: make "add_select" Plan method.

Also fix some documetation style
---
 src/ir/operator.rs       | 60 +++++++++++++++++++---------------------
 src/ir/operator/tests.rs |  6 ++--
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index 58366a7be2..18c398db4a 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -191,35 +191,6 @@ impl Relational {
         }
     }
 
-    /// New `Selection` constructor
-    ///
-    /// # Errors
-    /// Returns `QueryPlannerError`:
-    /// - filter expression is not boolean
-    /// - child node is not relational
-    /// - child output tuple is not valid
-    pub fn new_select(
-        plan: &mut Plan,
-        child: usize,
-        filter: usize,
-    ) -> Result<Self, QueryPlannerError> {
-        if let Node::Expression(Expression::Bool { .. }) = plan.get_node(filter)? {
-        } else {
-            return Err(QueryPlannerError::InvalidBool);
-        }
-
-        let id = plan.nodes.next_id();
-        let children: Vec<usize> = vec![child];
-        let output = plan.add_output_row(id, &children, &[0], &[])?;
-
-        Ok(Relational::Selection {
-            children,
-            filter,
-            id,
-            output,
-        })
-    }
-
     /// New `UnionAll` constructor.
     ///
     /// # Errors
@@ -290,7 +261,7 @@ impl Plan {
     /// Add a scan node.
     ///
     /// # Errors
-    /// Returns `QueryPlannerError` when when relation is invalid.
+    /// - relation is invalid
     pub fn add_scan(&mut self, table: &str) -> Result<usize, QueryPlannerError> {
         let logical_id = self.nodes.next_id();
         let nodes = &mut self.nodes;
@@ -325,10 +296,9 @@ impl Plan {
 
     // TODO: we need a more flexible projection constructor (constants, etc)
 
-    /// New `Projection` constructor.
+    /// Add projection node.
     ///
     /// # Errors
-    /// Returns `QueryPlannerError`:
     /// - child node is not relational
     /// - child output tuple is invalid
     /// - column name do not match the ones in the child output tuple
@@ -348,6 +318,32 @@ impl Plan {
         };
         Ok(self.nodes.push(Node::Relational(proj)))
     }
+
+    /// Add selection node
+    ///
+    /// # Errors
+    /// - filter expression is not boolean
+    /// - child node is not relational
+    /// - child output tuple is not valid
+    pub fn add_select(&mut self, child: usize, filter: usize) -> Result<usize, QueryPlannerError> {
+        if let Node::Expression(Expression::Bool { .. }) = self.get_node(filter)? {
+        } else {
+            return Err(QueryPlannerError::InvalidBool);
+        }
+
+        let id = self.nodes.next_id();
+        let children: Vec<usize> = vec![child];
+        let output = self.add_output_row(id, &children, &[0], &[])?;
+
+        let select = Relational::Selection {
+            children,
+            filter,
+            id,
+            output,
+        };
+
+        Ok(self.nodes.push(Node::Relational(select)))
+    }
 }
 
 #[cfg(test)]
diff --git a/src/ir/operator/tests.rs b/src/ir/operator/tests.rs
index a8d318423e..fffd9282d3 100644
--- a/src/ir/operator/tests.rs
+++ b/src/ir/operator/tests.rs
@@ -155,18 +155,18 @@ fn selection() {
     let gt_id = plan.nodes.add_bool(a_id, Bool::Gt, const_id).unwrap();
 
     // Correct Selection operator
-    Relational::new_select(&mut plan, scan_id, gt_id).unwrap();
+    plan.add_select(scan_id, gt_id).unwrap();
 
     // Non-boolean filter
     assert_eq!(
         QueryPlannerError::InvalidBool,
-        Relational::new_select(&mut plan, scan_id, const_id).unwrap_err()
+        plan.add_select(scan_id, const_id).unwrap_err()
     );
 
     // Non-relational child
     assert_eq!(
         QueryPlannerError::InvalidNode,
-        Relational::new_select(&mut plan, const_id, gt_id).unwrap_err()
+        plan.add_select(const_id, gt_id).unwrap_err()
     );
 }
 
-- 
GitLab