From 873cbeeb5865bc4a6a98c57e3b1fe5ac773abc4c Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Fri, 17 Dec 2021 18:04:26 +0700
Subject: [PATCH] refactoring: move "add_proj" to Plan methods
Also make a "add_bool" test fixup
---
src/ir/distribution/tests.rs | 4 +--
src/ir/operator.rs | 51 ++++++++++++++++++------------------
src/ir/operator/tests.rs | 11 ++++----
3 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/src/ir/distribution/tests.rs b/src/ir/distribution/tests.rs
index 15bcf5d8b7..dc3ca3edff 100644
--- a/src/ir/distribution/tests.rs
+++ b/src/ir/distribution/tests.rs
@@ -23,9 +23,7 @@ fn proj_preserve_dist_key() {
plan.add_rel(t);
let scan_id = plan.add_scan("t").unwrap();
-
- let proj = Relational::new_proj(&mut plan, scan_id, &["a", "b"]).unwrap();
- let proj_id = plan.nodes.push(Node::Relational(proj));
+ let proj_id = plan.add_proj(scan_id, &["a", "b"]).unwrap();
plan.top = Some(proj_id);
diff --git a/src/ir/operator.rs b/src/ir/operator.rs
index b473095dde..58366a7be2 100644
--- a/src/ir/operator.rs
+++ b/src/ir/operator.rs
@@ -191,31 +191,6 @@ impl Relational {
}
}
- // TODO: we need a more flexible projection constructor (constants, etc)
-
- /// New `Projection` constructor.
- ///
- /// # 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
- pub fn new_proj(
- plan: &mut Plan,
- child: usize,
- col_names: &[&str],
- ) -> Result<Self, QueryPlannerError> {
- let id = plan.nodes.next_id();
- let children: Vec<usize> = vec![child];
- let output = plan.add_output_row(id, &children, &[0], col_names)?;
-
- Ok(Relational::Projection {
- children,
- id,
- output,
- })
- }
-
/// New `Selection` constructor
///
/// # Errors
@@ -347,6 +322,32 @@ impl Plan {
}
Err(QueryPlannerError::InvalidRelation)
}
+
+ // TODO: we need a more flexible projection constructor (constants, etc)
+
+ /// New `Projection` constructor.
+ ///
+ /// # 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
+ pub fn add_proj(
+ &mut self,
+ child: usize,
+ 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, &[0], col_names)?;
+
+ let proj = Relational::Projection {
+ children,
+ id,
+ output,
+ };
+ Ok(self.nodes.push(Node::Relational(proj)))
+ }
}
#[cfg(test)]
diff --git a/src/ir/operator/tests.rs b/src/ir/operator/tests.rs
index 437d4d6ff9..a8d318423e 100644
--- a/src/ir/operator/tests.rs
+++ b/src/ir/operator/tests.rs
@@ -1,7 +1,6 @@
use super::*;
use crate::errors::QueryPlannerError;
use crate::ir::distribution::*;
-use crate::ir::expression::*;
use crate::ir::relation::*;
use crate::ir::value::*;
use crate::ir::*;
@@ -103,19 +102,19 @@ fn projection() {
// Invalid alias names in the output
assert_eq!(
QueryPlannerError::InvalidRow,
- Relational::new_proj(&mut plan, scan_id, &["a", "e"]).unwrap_err()
+ plan.add_proj(scan_id, &["a", "e"]).unwrap_err()
);
// Expression node instead of relational one
assert_eq!(
QueryPlannerError::InvalidNode,
- Relational::new_proj(&mut plan, 1, &["a"]).unwrap_err()
+ plan.add_proj(1, &["a"]).unwrap_err()
);
- // Try to build projection from the invalid node
+ // Try to build projection from the non-existing node
assert_eq!(
QueryPlannerError::ValueOutOfRange,
- Relational::new_proj(&mut plan, 42, &["a"]).unwrap_err()
+ plan.add_proj(42, &["a"]).unwrap_err()
);
}
@@ -153,7 +152,7 @@ fn selection() {
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.add_bool(a_id, Bool::Gt, const_id)?;
+ 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();
--
GitLab