From ad8d3aa8207b99fc4d0e5af228f4754a75b0c874 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 17 Dec 2021 17:35:16 +0700
Subject: [PATCH] refactoring: make "add_ref" plan Nodes method

---
 src/ir/expression.rs     | 37 +++++++++++++++++--------------------
 src/ir/operator.rs       |  3 +--
 src/ir/operator/tests.rs |  6 +-----
 3 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/src/ir/expression.rs b/src/ir/expression.rs
index e71ed85fe6..7317598167 100644
--- a/src/ir/expression.rs
+++ b/src/ir/expression.rs
@@ -98,16 +98,6 @@ impl Expression {
         Err(QueryPlannerError::InvalidRow)
     }
 
-    /// Reference expression constructor.
-    #[must_use]
-    pub fn new_ref(parent: usize, targets: Option<Vec<usize>>, position: usize) -> Self {
-        Expression::Reference {
-            parent,
-            targets,
-            position,
-        }
-    }
-
     // TODO: check that doesn't contain top-level aliases with the same names
     /// Row expression constructor.
     #[must_use]
@@ -146,6 +136,21 @@ impl Nodes {
     pub fn add_const(&mut self, value: Value) -> usize {
         self.push(Node::Expression(Expression::Constant { value }))
     }
+
+    /// Reference expression constructor.
+    pub fn add_ref(
+        &mut self,
+        parent: usize,
+        targets: Option<Vec<usize>>,
+        position: usize,
+    ) -> usize {
+        let r = Expression::Reference {
+            parent,
+            targets,
+            position,
+        };
+        self.push(Node::Expression(r))
+    }
 }
 
 impl Plan {
@@ -219,11 +224,7 @@ impl Plan {
                 };
                 let new_targets: Vec<usize> = targets.iter().copied().collect();
                 // Create new references and aliases. Save them to the plan nodes arena.
-                let r_id = self.nodes.push(Node::Expression(Expression::new_ref(
-                    rel_node_id,
-                    Some(new_targets),
-                    pos,
-                )));
+                let r_id = self.nodes.add_ref(rel_node_id, Some(new_targets), pos);
                 let a_id = self.nodes.add_alias(&name, r_id)?;
                 aliases.push(a_id);
             }
@@ -246,11 +247,7 @@ impl Plan {
             map.get(*col).map_or(false, |pos| {
                 let new_targets: Vec<usize> = targets.iter().copied().collect();
                 // Create new references and aliases. Save them to the plan nodes arena.
-                let r_id = self.nodes.push(Node::Expression(Expression::new_ref(
-                    rel_node_id,
-                    Some(new_targets),
-                    *pos,
-                )));
+                let r_id = self.nodes.add_ref(rel_node_id, Some(new_targets), *pos);
                 if let Ok(a_id) = self.nodes.add_alias(col, r_id) {
                     aliases.push(a_id);
                     true
diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index 83675b56a3..f582c2c6b1 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -327,8 +327,7 @@ impl Plan {
                         let mut refs: Vec<usize> = Vec::new();
 
                         for (pos, col) in columns.iter().enumerate() {
-                            let r_id = nodes
-                                .push(Node::Expression(Expression::new_ref(logical_id, None, pos)));
+                            let r_id = nodes.add_ref(logical_id, None, pos);
                             let alias_id = nodes.add_alias(&col.name, r_id)?;
                             refs.push(alias_id);
                         }
diff --git a/src/ir/operator/tests.rs b/src/ir/operator/tests.rs
index 8b202c282a..f7327667d5 100644
--- a/src/ir/operator/tests.rs
+++ b/src/ir/operator/tests.rs
@@ -150,11 +150,7 @@ fn selection() {
 
     let scan_id = plan.add_scan("t").unwrap();
 
-    let ref_a_id = plan.nodes.push(Node::Expression(Expression::new_ref(
-        scan_id + 1,
-        Some(vec![0]),
-        0,
-    )));
+    let ref_a_id = plan.nodes.add_ref(scan_id + 1, Some(vec![0]), 0);
     let a_id = plan.nodes.add_alias("a", ref_a_id).unwrap();
     let const_id = plan.nodes.add_const(Value::number_from_str("10").unwrap());
     let gt_id = plan.nodes.push(Node::Expression(Expression::new_bool(
-- 
GitLab