From 72053ba066a3fab2d1a36a9b48dab3c061cdae68 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 17 Dec 2021 13:28:27 +0700
Subject: [PATCH] refactoring: rename "add_row" to "add_output_row".

Also move this Plan implementation method to expressions.
---
 src/ir.rs            | 123 ------------------------------------------
 src/ir/expression.rs | 126 +++++++++++++++++++++++++++++++++++++++++++
 src/ir/operator.rs   |   8 +--
 3 files changed, 130 insertions(+), 127 deletions(-)

diff --git a/src/ir.rs b/src/ir.rs
index b092c3ea6c..dab7612c39 100644
--- a/src/ir.rs
+++ b/src/ir.rs
@@ -92,129 +92,6 @@ impl Plan {
         }
     }
 
-    /// Create a new tuple from the children nodes output, containing only
-    /// a specified list of column names. If the column list is empty then
-    /// just copy all the columns to a new tuple.
-    /// # Errors
-    /// Returns `QueryPlannerError`:
-    /// - relation node contains invalid `Row` in the output
-    /// - targets and children are inconsistent
-    /// - column names don't exits
-    pub fn add_row(
-        &mut self,
-        rel_node_id: usize,
-        children: &[usize],
-        targets: &[usize],
-        col_names: &[&str],
-    ) -> Result<usize, QueryPlannerError> {
-        // We can pass two target children nodes only in a case
-        // of `UnionAll`. Even for a `NaturalJoin` we work with
-        // each child independently. In fact, we need only the
-        // first child in a `UnionAll` operator to get correct
-        // column names for a new tuple (second child aliases
-        // would be shadowed). But each reference should point
-        // to both children to give us additional information
-        // during transformations.
-        if (targets.len() > 2) || targets.is_empty() {
-            return Err(QueryPlannerError::InvalidRow);
-        }
-
-        if let Some(max) = targets.iter().max() {
-            if *max >= children.len() {
-                return Err(QueryPlannerError::InvalidRow);
-            }
-        }
-
-        let target_child: usize = if let Some(target) = targets.get(0) {
-            *target
-        } else {
-            return Err(QueryPlannerError::InvalidRow);
-        };
-        let child_node: usize = if let Some(child) = children.get(target_child) {
-            *child
-        } else {
-            return Err(QueryPlannerError::InvalidRow);
-        };
-
-        if col_names.is_empty() {
-            let child_row_list: Vec<usize> =
-                if let Node::Relational(relational_op) = self.get_node(child_node)? {
-                    if let Node::Expression(Expression::Row { list, .. }) =
-                        self.get_node(relational_op.output())?
-                    {
-                        list.clone()
-                    } else {
-                        return Err(QueryPlannerError::InvalidRow);
-                    }
-                } else {
-                    return Err(QueryPlannerError::InvalidNode);
-                };
-
-            let mut aliases: Vec<usize> = Vec::new();
-
-            for (pos, alias_node) in child_row_list.iter().enumerate() {
-                let name: String = if let Node::Expression(Expression::Alias { ref name, .. }) =
-                    self.get_node(*alias_node)?
-                {
-                    String::from(name)
-                } else {
-                    return Err(QueryPlannerError::InvalidRow);
-                };
-                let new_targets: Vec<usize> = targets.iter().copied().collect();
-                // Create new references and aliases. Save them to the plan nodes arena.
-                let r_id = vec_alloc(
-                    &mut self.nodes,
-                    Node::Expression(Expression::new_ref(rel_node_id, Some(new_targets), pos)),
-                );
-                let a_id = vec_alloc(
-                    &mut self.nodes,
-                    Node::Expression(Expression::new_alias(&name, r_id)),
-                );
-                aliases.push(a_id);
-            }
-
-            let row_node = vec_alloc(
-                &mut self.nodes,
-                Node::Expression(Expression::new_row(aliases, None)),
-            );
-            return Ok(row_node);
-        }
-
-        let map = if let Node::Relational(relational_op) = self.get_node(child_node)? {
-            relational_op.output_alias_position_map(self)?
-        } else {
-            return Err(QueryPlannerError::InvalidNode);
-        };
-
-        let mut aliases: Vec<usize> = Vec::new();
-
-        let all_found = col_names.iter().all(|col| {
-            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 = vec_alloc(
-                    &mut self.nodes,
-                    Node::Expression(Expression::new_ref(rel_node_id, Some(new_targets), *pos)),
-                );
-                let a_id = vec_alloc(
-                    &mut self.nodes,
-                    Node::Expression(Expression::new_alias(col, r_id)),
-                );
-                aliases.push(a_id);
-                true
-            })
-        });
-
-        if all_found {
-            let row_node = vec_alloc(
-                &mut self.nodes,
-                Node::Expression(Expression::new_row(aliases, None)),
-            );
-            return Ok(row_node);
-        }
-        Err(QueryPlannerError::InvalidRow)
-    }
-
     /// Check that plan tree is valid.
     ///
     /// # Errors
diff --git a/src/ir/expression.rs b/src/ir/expression.rs
index 8255665174..36c383b96a 100644
--- a/src/ir/expression.rs
+++ b/src/ir/expression.rs
@@ -8,6 +8,7 @@
 use super::distribution::Distribution;
 use super::operator;
 use super::value::Value;
+use super::{vec_alloc, Node, Plan};
 use crate::errors::QueryPlannerError;
 use serde::{Deserialize, Serialize};
 
@@ -135,3 +136,128 @@ impl Expression {
         Expression::Bool { left, op, right }
     }
 }
+
+impl Plan {
+   /// Create a new output tuple from the children nodes output, containing
+    /// a specified list of column names. If the column list is empty then
+    /// just copy all the columns to a new tuple.
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - relation node contains invalid `Row` in the output
+    /// - targets and children are inconsistent
+    /// - column names don't exits
+    pub fn add_output_row(
+        &mut self,
+        rel_node_id: usize,
+        children: &[usize],
+        targets: &[usize],
+        col_names: &[&str],
+    ) -> Result<usize, QueryPlannerError> {
+        // We can pass two target children nodes only in a case
+        // of `UnionAll`. Even for a `NaturalJoin` we work with
+        // each child independently. In fact, we need only the
+        // first child in a `UnionAll` operator to get correct
+        // column names for a new tuple (second child aliases
+        // would be shadowed). But each reference should point
+        // to both children to give us additional information
+        // during transformations.
+        if (targets.len() > 2) || targets.is_empty() {
+            return Err(QueryPlannerError::InvalidRow);
+        }
+
+        if let Some(max) = targets.iter().max() {
+            if *max >= children.len() {
+                return Err(QueryPlannerError::InvalidRow);
+            }
+        }
+
+        let target_child: usize = if let Some(target) = targets.get(0) {
+            *target
+        } else {
+            return Err(QueryPlannerError::InvalidRow);
+        };
+        let child_node: usize = if let Some(child) = children.get(target_child) {
+            *child
+        } else {
+            return Err(QueryPlannerError::InvalidRow);
+        };
+
+        if col_names.is_empty() {
+            let child_row_list: Vec<usize> =
+                if let Node::Relational(relational_op) = self.get_node(child_node)? {
+                    if let Node::Expression(Expression::Row { list, .. }) =
+                        self.get_node(relational_op.output())?
+                    {
+                        list.clone()
+                    } else {
+                        return Err(QueryPlannerError::InvalidRow);
+                    }
+                } else {
+                    return Err(QueryPlannerError::InvalidNode);
+                };
+
+            let mut aliases: Vec<usize> = Vec::new();
+
+            for (pos, alias_node) in child_row_list.iter().enumerate() {
+                let name: String = if let Node::Expression(Expression::Alias { ref name, .. }) =
+                    self.get_node(*alias_node)?
+                {
+                    String::from(name)
+                } else {
+                    return Err(QueryPlannerError::InvalidRow);
+                };
+                let new_targets: Vec<usize> = targets.iter().copied().collect();
+                // Create new references and aliases. Save them to the plan nodes arena.
+                let r_id = vec_alloc(
+                    &mut self.nodes,
+                    Node::Expression(Expression::new_ref(rel_node_id, Some(new_targets), pos)),
+                );
+                let a_id = vec_alloc(
+                    &mut self.nodes,
+                    Node::Expression(Expression::new_alias(&name, r_id)),
+                );
+                aliases.push(a_id);
+            }
+
+            let row_node = vec_alloc(
+                &mut self.nodes,
+                Node::Expression(Expression::new_row(aliases, None)),
+            );
+            return Ok(row_node);
+        }
+
+        let map = if let Node::Relational(relational_op) = self.get_node(child_node)? {
+            relational_op.output_alias_position_map(self)?
+        } else {
+            return Err(QueryPlannerError::InvalidNode);
+        };
+
+        let mut aliases: Vec<usize> = Vec::new();
+
+        let all_found = col_names.iter().all(|col| {
+            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 = vec_alloc(
+                    &mut self.nodes,
+                    Node::Expression(Expression::new_ref(rel_node_id, Some(new_targets), *pos)),
+                );
+                let a_id = vec_alloc(
+                    &mut self.nodes,
+                    Node::Expression(Expression::new_alias(col, r_id)),
+                );
+                aliases.push(a_id);
+                true
+            })
+        });
+
+        if all_found {
+            let row_node = vec_alloc(
+                &mut self.nodes,
+                Node::Expression(Expression::new_row(aliases, None)),
+            );
+            return Ok(row_node);
+        }
+        Err(QueryPlannerError::InvalidRow)
+    }
+}
diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index 3900302dc2..f6591999d3 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -207,7 +207,7 @@ impl Relational {
     ) -> Result<Self, QueryPlannerError> {
         let id = plan.next_node_id();
         let children: Vec<usize> = vec![child];
-        let output = plan.add_row(id, &children, &[0], col_names)?;
+        let output = plan.add_output_row(id, &children, &[0], col_names)?;
 
         Ok(Relational::Projection {
             children,
@@ -235,7 +235,7 @@ impl Relational {
 
         let id = plan.next_node_id();
         let children: Vec<usize> = vec![child];
-        let output = plan.add_row(id, &children, &[0], &[])?;
+        let output = plan.add_output_row(id, &children, &[0], &[])?;
 
         Ok(Relational::Selection {
             children,
@@ -274,7 +274,7 @@ impl Relational {
 
         let id = plan.next_node_id();
         let children: Vec<usize> = vec![left, right];
-        let output = plan.add_row(id, &children, &[0, 1], &[])?;
+        let output = plan.add_output_row(id, &children, &[0, 1], &[])?;
 
         Ok(Relational::UnionAll {
             children,
@@ -300,7 +300,7 @@ impl Relational {
         }
         let id = plan.next_node_id();
         let children: Vec<usize> = vec![child];
-        let output = plan.add_row(id, &children, &[0], &[])?;
+        let output = plan.add_output_row(id, &children, &[0], &[])?;
 
         Ok(Relational::ScanSubQuery {
             alias: String::from(alias),
-- 
GitLab