From dde6a670e6fcdf95bec2edc104a384c068ea57ec Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 24 Dec 2021 11:39:32 +0700
Subject: [PATCH] feat: implement user-friendly row constructors

---
 src/ir/expression.rs     | 87 ++++++++++++++++++++++++++++++++++++++--
 src/ir/operator.rs       | 25 +++++-------
 src/ir/operator/tests.rs |  8 ++--
 src/ir/relation.rs       | 17 ++++----
 src/ir/relation/tests.rs | 12 +++---
 5 files changed, 114 insertions(+), 35 deletions(-)

diff --git a/src/ir/expression.rs b/src/ir/expression.rs
index 030239b985..8e2fc1a2b7 100644
--- a/src/ir/expression.rs
+++ b/src/ir/expression.rs
@@ -189,7 +189,7 @@ impl Nodes {
 }
 
 impl Plan {
-    /// Create a new output row from the children nodes output, containing
+    /// Create a new 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.
     ///
@@ -201,7 +201,7 @@ impl Plan {
     /// - relation node contains invalid `Row` in the output
     /// - targets and children are inconsistent
     /// - column names don't exits
-    pub fn add_output_row(
+    fn add_row_from_output(
         &mut self,
         rel_node_id: usize,
         children: &[usize],
@@ -265,7 +265,8 @@ impl Plan {
                             return Err(QueryPlannerError::InvalidRow);
                         };
                     let new_targets: Vec<usize> = if is_join {
-                        // Reference in a join tuple points to the left or right child** left and right children.
+                        // Reference in a join tuple points to at first to the left,
+                        // then to the right child.
                         vec![*target_idx]
                     } else {
                         // Reference in union tuple points to **both** left and right children.
@@ -321,6 +322,86 @@ impl Plan {
         }
         Err(QueryPlannerError::InvalidRow)
     }
+
+    /// Project columns from the child node.
+    ///
+    /// If column names are empty, copy all the columns from the child.
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - child is an inconsistent relational node
+    /// - column names don't exits
+    pub fn add_row_from_single_child(
+        &mut self,
+        id: usize,
+        child: usize,
+        col_names: &[&str],
+    ) -> Result<usize, QueryPlannerError> {
+        self.add_row_from_output(id, &[child], false, &[0], col_names)
+    }
+
+    /// New output row for union node.
+    ///
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - children are inconsistent relational nodes
+    pub fn add_row_for_union(
+        &mut self,
+        id: usize,
+        left: usize,
+        right: usize,
+    ) -> Result<usize, QueryPlannerError> {
+        self.add_row_from_output(id, &[left, right], false, &[0, 1], &[])
+    }
+
+    /// New output row for join node.
+    ///
+    /// Contains all the columns from left and right children.
+    ///
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - children are inconsistent relational nodes
+    pub fn add_row_for_join(
+        &mut self,
+        id: usize,
+        left: usize,
+        right: usize,
+    ) -> Result<usize, QueryPlannerError> {
+        self.add_row_from_output(id, &[left, right], true, &[0, 1], &[])
+    }
+
+    /// Project columns from the join's left branch.
+    ///
+    /// If column names are empty, copy all the columns from the left child.
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - children are inconsistent relational nodes
+    /// - column names don't exits
+    pub fn add_row_from_left_branch(
+        &mut self,
+        id: usize,
+        left: usize,
+        right: usize,
+        col_names: &[&str],
+    ) -> Result<usize, QueryPlannerError> {
+        self.add_row_from_output(id, &[left, right], true, &[0], col_names)
+    }
+
+    /// Project columns from the join's right branch.
+    ///
+    /// If column names are empty, copy all the columns from the right child.
+    /// # Errors
+    /// Returns `QueryPlannerError`:
+    /// - children are inconsistent relational nodes
+    /// - column names don't exits
+    pub fn add_row_from_right_branch(
+        &mut self,
+        id: usize,
+        left: usize,
+        right: usize,
+        col_names: &[&str],
+    ) -> Result<usize, QueryPlannerError> {
+        self.add_row_from_output(id, &[left, right], true, &[1], col_names)
+    }
 }
 
 #[cfg(test)]
diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index 95b8d90f47..948b611b44 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -247,11 +247,10 @@ impl Plan {
             return Err(QueryPlannerError::InvalidBool);
         }
 
-        let children: Vec<usize> = vec![left, right];
-        let output = self.add_output_row(id, &children, true, &[0, 1], &[])?;
+        let output = self.add_row_for_join(id, left, right)?;
 
         let join = Relational::InnerJoin {
-            children,
+            children: vec![left, right],
             condition,
             id,
             output,
@@ -273,11 +272,10 @@ impl Plan {
         col_names: &[&str],
     ) -> Result<usize, QueryPlannerError> {
         let id = self.nodes.next_id();
-        let children: Vec<usize> = vec![child];
-        let output = self.add_output_row(id, &children, false, &[0], col_names)?;
+        let output = self.add_row_from_single_child(id, child, col_names)?;
 
         let proj = Relational::Projection {
-            children,
+            children: vec![child],
             id,
             output,
         };
@@ -301,11 +299,10 @@ impl Plan {
             return Err(QueryPlannerError::InvalidBool);
         }
 
-        let children: Vec<usize> = vec![child];
-        let output = self.add_output_row(id, &children, false, &[0], &[])?;
+        let output = self.add_row_from_single_child(id, child, &[])?;
 
         let select = Relational::Selection {
-            children,
+            children: vec![child],
             filter,
             id,
             output,
@@ -325,12 +322,11 @@ impl Plan {
             return Err(QueryPlannerError::InvalidName);
         }
         let id = self.nodes.next_id();
-        let children: Vec<usize> = vec![child];
-        let output = self.add_output_row(id, &children, false, &[0], &[])?;
+        let output = self.add_row_from_single_child(id, child, &[])?;
 
         let sq = Relational::ScanSubQuery {
             alias: String::from(alias),
-            children,
+            children: vec![child],
             id,
             output,
         };
@@ -361,11 +357,10 @@ impl Plan {
         }
 
         let id = self.nodes.next_id();
-        let children: Vec<usize> = vec![left, right];
-        let output = self.add_output_row(id, &children, false, &[0, 1], &[])?;
+        let output = self.add_row_for_union(id, left, right)?;
 
         let union_all = Relational::UnionAll {
-            children,
+            children: vec![left, right],
             id,
             output,
         };
diff --git a/src/ir/operator/tests.rs b/src/ir/operator/tests.rs
index da39cf078a..a7d025a7b7 100644
--- a/src/ir/operator/tests.rs
+++ b/src/ir/operator/tests.rs
@@ -308,10 +308,10 @@ fn join() {
 
     let logical_id = plan.nodes.next_id();
     let a_row = plan
-        .add_output_row(logical_id, &[scan_t1, scan_t2], true, &[0], &["a"])
+        .add_row_from_left_branch(logical_id, scan_t1, scan_t2, &["a"])
         .unwrap();
     let d_row = plan
-        .add_output_row(logical_id, &[scan_t1, scan_t2], true, &[1], &["d"])
+        .add_row_from_right_branch(logical_id, scan_t1, scan_t2, &["d"])
         .unwrap();
     let condition = plan.nodes.add_bool(a_row, Bool::Eq, d_row).unwrap();
     let join = plan
@@ -364,10 +364,10 @@ fn join_duplicate_columns() {
 
     let logical_id = plan.nodes.next_id();
     let a_row = plan
-        .add_output_row(logical_id, &[scan_t1, scan_t2], true, &[0], &["a"])
+        .add_row_from_left_branch(logical_id, scan_t1, scan_t2, &["a"])
         .unwrap();
     let d_row = plan
-        .add_output_row(logical_id, &[scan_t1, scan_t2], true, &[1], &["d"])
+        .add_row_from_right_branch(logical_id, scan_t1, scan_t2, &["d"])
         .unwrap();
     let condition = plan.nodes.add_bool(a_row, Bool::Eq, d_row).unwrap();
     assert_eq!(
diff --git a/src/ir/relation.rs b/src/ir/relation.rs
index 462f8a4fe2..71e33d87a0 100644
--- a/src/ir/relation.rs
+++ b/src/ir/relation.rs
@@ -3,9 +3,9 @@
 use std::collections::{HashMap, HashSet};
 use std::fmt::Formatter;
 
-use serde::{Deserialize, Deserializer, Serialize};
 use serde::de::{Error, MapAccess, Visitor};
 use serde::ser::{Serialize as SerSerialize, SerializeMap, Serializer};
+use serde::{Deserialize, Deserializer, Serialize};
 use tarantool::hlua::{self, LuaRead};
 
 use crate::errors::QueryPlannerError;
@@ -52,8 +52,8 @@ pub struct Column {
 /// Msgpack serializer for column
 impl SerSerialize for Column {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-        where
-            S: Serializer,
+    where
+        S: Serializer,
     {
         let mut map = serializer.serialize_map(Some(2))?;
         map.serialize_entry("name", &self.name)?;
@@ -78,14 +78,17 @@ impl<'de> Visitor<'de> for ColumnVisitor {
         formatter.write_str("column parsing failed")
     }
 
-    fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error> where A: MapAccess<'de> {
+    fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
+    where
+        A: MapAccess<'de>,
+    {
         let mut column_name = String::new();
         let mut column_type = String::new();
         while let Some((key, value)) = map.next_entry::<String, String>()? {
             match key.as_str() {
                 "name" => column_name.push_str(&value),
                 "type" => column_type.push_str(&value.to_lowercase()),
-                _ => return Err(Error::custom("invalid column param"))
+                _ => return Err(Error::custom("invalid column param")),
             }
         }
 
@@ -102,8 +105,8 @@ impl<'de> Visitor<'de> for ColumnVisitor {
 
 impl<'de> Deserialize<'de> for Column {
     fn deserialize<D>(deserializer: D) -> Result<Column, D::Error>
-        where
-            D: Deserializer<'de>,
+    where
+        D: Deserializer<'de>,
     {
         deserializer.deserialize_map(ColumnVisitor)
     }
diff --git a/src/ir/relation/tests.rs b/src/ir/relation/tests.rs
index 5647457c0d..b2b98cadae 100644
--- a/src/ir/relation/tests.rs
+++ b/src/ir/relation/tests.rs
@@ -28,7 +28,7 @@ fn table_seg() {
         ],
         &["b", "a"],
     )
-        .unwrap();
+    .unwrap();
     if let Table::Segment { key, .. } = &t {
         assert_eq!(2, key.positions.len());
         assert_eq!(0, key.positions[1]);
@@ -55,7 +55,7 @@ fn table_seg_duplicate_columns() {
             ],
             &["b", "a"],
         )
-            .unwrap_err(),
+        .unwrap_err(),
         QueryPlannerError::DuplicateColumn
     );
 }
@@ -73,7 +73,7 @@ fn table_seg_wrong_key() {
             ],
             &["a", "e"],
         )
-            .unwrap_err(),
+        .unwrap_err(),
         QueryPlannerError::InvalidShardingKey
     );
 }
@@ -92,7 +92,7 @@ fn table_seg_serialized() {
         ],
         &["a", "d"],
     )
-        .unwrap();
+    .unwrap();
     let path = Path::new("")
         .join("tests")
         .join("artifactory")
@@ -242,8 +242,8 @@ fn table_converting() {
         ],
         &["b", "a"],
     )
-        .unwrap();
+    .unwrap();
 
     let s = serde_yaml::to_string(&t).unwrap();
     Table::seg_from_yaml(&s).unwrap();
-}
\ No newline at end of file
+}
-- 
GitLab