From 88236bc2afb9908a4816b51bcd1928012a6ee7ec Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 17 Dec 2021 17:47:21 +0700
Subject: [PATCH] refactoring: make "add_row" plan Nodes method

---
 src/ir/expression.rs | 41 +++++++++++++++++++++++++++--------------
 src/ir/operator.rs   |  2 +-
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/ir/expression.rs b/src/ir/expression.rs
index 7317598167..9422e67e6e 100644
--- a/src/ir/expression.rs
+++ b/src/ir/expression.rs
@@ -98,13 +98,6 @@ impl Expression {
         Err(QueryPlannerError::InvalidRow)
     }
 
-    // TODO: check that doesn't contain top-level aliases with the same names
-    /// Row expression constructor.
-    #[must_use]
-    pub fn new_row(list: Vec<usize>, distribution: Option<Distribution>) -> Self {
-        Expression::Row { list, distribution }
-    }
-
     /// Boolean expression constructor.
     #[must_use]
     pub fn new_bool(left: usize, op: operator::Bool, right: usize) -> Self {
@@ -151,10 +144,34 @@ impl Nodes {
         };
         self.push(Node::Expression(r))
     }
+
+    // TODO: check that doesn't contain top-level aliases with the same names
+    /// Add row node.
+    ///
+    /// # Errors
+    /// - nodes in a list are invalid
+    /// - nodes in a list are not aliases
+    pub fn add_row(
+        &mut self,
+        list: Vec<usize>,
+        distribution: Option<Distribution>,
+    ) -> Result<usize, QueryPlannerError> {
+        for alias_node in &list {
+            if let Node::Expression(Expression::Alias { .. }) = self
+                .arena
+                .get(*alias_node)
+                .ok_or(QueryPlannerError::InvalidNode)?
+            {
+            } else {
+                return Err(QueryPlannerError::InvalidRow);
+            }
+        }
+        Ok(self.push(Node::Expression(Expression::Row { list, distribution })))
+    }
 }
 
 impl Plan {
-    /// Create a new output tuple from the children nodes output, containing
+    /// Create a new output row 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
@@ -229,9 +246,7 @@ impl Plan {
                 aliases.push(a_id);
             }
 
-            let row_node = self
-                .nodes
-                .push(Node::Expression(Expression::new_row(aliases, None)));
+            let row_node = self.nodes.add_row(aliases, None)?;
             return Ok(row_node);
         }
 
@@ -258,9 +273,7 @@ impl Plan {
         });
 
         if all_found {
-            let row_node = self
-                .nodes
-                .push(Node::Expression(Expression::new_row(aliases, None)));
+            let row_node = self.nodes.add_row(aliases, None)?;
             return Ok(row_node);
         }
         Err(QueryPlannerError::InvalidRow)
diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index f582c2c6b1..b473095dde 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -334,7 +334,7 @@ impl Plan {
 
                         let scan = Relational::ScanRelation {
                             id: logical_id,
-                            output: nodes.push(Node::Expression(Expression::new_row(refs, None))),
+                            output: nodes.add_row(refs, None)?,
                             relation: String::from(table),
                         };
 
-- 
GitLab