refactor: flatten the error hierarchy (#125)

* perf: remove the redundant true or false when in is rewritten as multiple equivalent expressions * refactor: flatten the error hierarchy
KipData · Feb 2, 2024 · 280b360 · 280b360
1 parent 75fada6
commit 280b360
Show file tree

Hide file tree

Showing 101 changed files with 703 additions and 814 deletions.
diff --git a/benchmarks/query_benchmark.rs b/benchmarks/query_benchmark.rs
@@ -1,5 +1,6 @@
 use criterion::{criterion_group, criterion_main, Criterion};
-use fnck_sql::db::{Database, DatabaseError};
+use fnck_sql::db::Database;
+use fnck_sql::errors::DatabaseError;
 use fnck_sql::execution::volcano;
 use fnck_sql::storage::kip::KipStorage;
 use fnck_sql::storage::Storage;

diff --git a/examples/hello_world.rs b/examples/hello_world.rs
@@ -1,4 +1,5 @@
-use fnck_sql::db::{Database, DatabaseError};
+use fnck_sql::db::Database;
+use fnck_sql::errors::DatabaseError;
 use fnck_sql::implement_from_tuple;
 use fnck_sql::types::tuple::Tuple;
 use fnck_sql::types::value::DataValue;

diff --git a/examples/transaction.rs b/examples/transaction.rs
@@ -1,4 +1,5 @@
-use fnck_sql::db::{Database, DatabaseError};
+use fnck_sql::db::Database;
+use fnck_sql::errors::DatabaseError;
 
 #[tokio::main]
 async fn main() -> Result<(), DatabaseError> {

diff --git a/src/binder/aggregate.rs b/src/binder/aggregate.rs
@@ -3,7 +3,7 @@ use itertools::Itertools;
 use sqlparser::ast::{Expr, OrderByExpr};
 use std::collections::HashSet;
 
-use crate::binder::BindError;
+use crate::errors::DatabaseError;
 use crate::planner::LogicalPlan;
 use crate::storage::Transaction;
 use crate::{
@@ -26,7 +26,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
     pub fn extract_select_aggregate(
         &mut self,
         select_items: &mut [ScalarExpression],
-    ) -> Result<(), BindError> {
+    ) -> Result<(), DatabaseError> {
         for column in select_items {
             self.visit_column_agg_expr(column)?;
         }
@@ -37,7 +37,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         select_list: &mut [ScalarExpression],
         groupby: &[Expr],
-    ) -> Result<(), BindError> {
+    ) -> Result<(), DatabaseError> {
         self.validate_groupby_illegal_column(select_list, groupby)?;
 
         for gb in groupby {
@@ -51,7 +51,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         having: &Option<Expr>,
         orderbys: &[OrderByExpr],
-    ) -> Result<(Option<ScalarExpression>, Option<Vec<SortField>>), BindError> {
+    ) -> Result<(Option<ScalarExpression>, Option<Vec<SortField>>), DatabaseError> {
         // Extract having expression.
         let return_having = if let Some(having) = having {
             let mut having = self.bind_expr(having)?;
@@ -87,7 +87,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         Ok((return_having, return_orderby))
     }
 
-    fn visit_column_agg_expr(&mut self, expr: &mut ScalarExpression) -> Result<(), BindError> {
+    fn visit_column_agg_expr(&mut self, expr: &mut ScalarExpression) -> Result<(), DatabaseError> {
         match expr {
             ScalarExpression::AggCall { .. } => {
                 self.context.agg_calls.push(expr.clone());
@@ -125,7 +125,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         select_items: &[ScalarExpression],
         groupby: &[Expr],
-    ) -> Result<(), BindError> {
+    ) -> Result<(), DatabaseError> {
         let mut group_raw_exprs = vec![];
         for expr in groupby {
             let expr = self.bind_expr(expr)?;
@@ -159,15 +159,15 @@ impl<'a, T: Transaction> Binder<'a, T> {
             group_raw_set.remove(expr);
 
             if !group_raw_exprs.iter().contains(expr) {
-                return Err(BindError::AggMiss(format!(
+                return Err(DatabaseError::AggMiss(format!(
                     "{:?} must appear in the GROUP BY clause or be used in an aggregate function",
                     expr
                 )));
             }
         }
 
         if !group_raw_set.is_empty() {
-            return Err(BindError::AggMiss(
+            return Err(DatabaseError::AggMiss(
                 "In the GROUP BY clause the field must be in the select clause".to_string(),
             ));
         }
@@ -202,7 +202,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
     }
 
     /// Validate having or orderby clause is valid, if SQL has group by clause.
-    pub fn validate_having_orderby(&self, expr: &ScalarExpression) -> Result<(), BindError> {
+    pub fn validate_having_orderby(&self, expr: &ScalarExpression) -> Result<(), DatabaseError> {
         if self.context.group_by_exprs.is_empty() {
             return Ok(());
         }
@@ -215,7 +215,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
                     return Ok(());
                 }
 
-                Err(BindError::AggMiss(
+                Err(DatabaseError::AggMiss(
                     format!(
                         "column {:?} must appear in the GROUP BY clause or be used in an aggregate function",
                         expr
@@ -230,7 +230,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
                     return self.validate_having_orderby(expr.unpack_alias());
                 }
 
-                Err(BindError::AggMiss(
+                Err(DatabaseError::AggMiss(
                     format!(
                         "column {:?} must appear in the GROUP BY clause or be used in an aggregate function",
                         expr

diff --git a/src/binder/alter_table.rs b/src/binder/alter_table.rs
@@ -3,7 +3,8 @@ use sqlparser::ast::{AlterTableOperation, ObjectName};
 use std::sync::Arc;
 
 use super::{is_valid_identifier, Binder};
-use crate::binder::{lower_case_name, split_name, BindError};
+use crate::binder::{lower_case_name, split_name};
+use crate::errors::DatabaseError;
 use crate::planner::operator::alter_table::add_column::AddColumnOperator;
 use crate::planner::operator::alter_table::drop_column::DropColumnOperator;
 use crate::planner::operator::scan::ScanOperator;
@@ -16,7 +17,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         name: &ObjectName,
         operation: &AlterTableOperation,
-    ) -> Result<LogicalPlan, BindError> {
+    ) -> Result<LogicalPlan, DatabaseError> {
         let table_name: Arc<String> = Arc::new(split_name(&lower_case_name(name))?.to_string());
 
         if let Some(table) = self.context.table(table_name.clone()) {
@@ -30,7 +31,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
                     let column = self.bind_column(column_def)?;
 
                     if !is_valid_identifier(column.name()) {
-                        return Err(BindError::InvalidColumn(
+                        return Err(DatabaseError::InvalidColumn(
                             "illegal column naming".to_string(),
                         ));
                     }
@@ -83,7 +84,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
 
             Ok(plan)
         } else {
-            Err(BindError::InvalidTable(format!(
+            Err(DatabaseError::InvalidTable(format!(
                 "not found table {}",
                 table_name
             )))

diff --git a/src/binder/analyze.rs b/src/binder/analyze.rs
@@ -1,4 +1,5 @@
-use crate::binder::{lower_case_name, split_name, BindError, Binder};
+use crate::binder::{lower_case_name, split_name, Binder};
+use crate::errors::DatabaseError;
 use crate::planner::operator::analyze::AnalyzeOperator;
 use crate::planner::operator::scan::ScanOperator;
 use crate::planner::operator::Operator;
@@ -9,7 +10,7 @@ use sqlparser::ast::ObjectName;
 use std::sync::Arc;
 
 impl<'a, T: Transaction> Binder<'a, T> {
-    pub(crate) fn bind_analyze(&mut self, name: &ObjectName) -> Result<LogicalPlan, BindError> {
+    pub(crate) fn bind_analyze(&mut self, name: &ObjectName) -> Result<LogicalPlan, DatabaseError> {
         let name = lower_case_name(name);
         let name = split_name(&name)?;
         let table_name = Arc::new(name.to_string());
@@ -18,7 +19,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
             .context
             .table(table_name.clone())
             .cloned()
-            .ok_or_else(|| BindError::InvalidTable(format!("bind table {}", name)))?;
+            .ok_or_else(|| DatabaseError::InvalidTable(format!("bind table {}", name)))?;
         let columns = table_catalog
             .all_columns()
             .into_iter()

diff --git a/src/binder/copy.rs b/src/binder/copy.rs
@@ -2,6 +2,7 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::Arc;
 
+use crate::errors::DatabaseError;
 use crate::planner::operator::copy_from_file::CopyFromFileOperator;
 use crate::planner::operator::copy_to_file::CopyToFileOperator;
 use crate::planner::operator::Operator;
@@ -57,14 +58,14 @@ impl<'a, T: Transaction> Binder<'a, T> {
         to: bool,
         target: CopyTarget,
         options: &[CopyOption],
-    ) -> Result<LogicalPlan, BindError> {
+    ) -> Result<LogicalPlan, DatabaseError> {
         let (table_name, ..) = match source {
             CopySource::Table {
                 table_name,
                 columns,
             } => (table_name, columns),
             CopySource::Query(_) => {
-                return Err(BindError::UnsupportedCopySource(
+                return Err(DatabaseError::UnsupportedCopySource(
                     "bad copy source".to_string(),
                 ));
             }
@@ -100,7 +101,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
                 })
             }
         } else {
-            Err(BindError::InvalidTable(format!(
+            Err(DatabaseError::InvalidTable(format!(
                 "not found table {}",
                 table_name
             )))

diff --git a/src/binder/create_table.rs b/src/binder/create_table.rs
@@ -4,8 +4,9 @@ use std::collections::HashSet;
 use std::sync::Arc;
 
 use super::{is_valid_identifier, Binder};
-use crate::binder::{lower_case_name, split_name, BindError};
+use crate::binder::{lower_case_name, split_name};
 use crate::catalog::{ColumnCatalog, ColumnDesc};
+use crate::errors::DatabaseError;
 use crate::expression::ScalarExpression;
 use crate::planner::operator::create_table::CreateTableOperator;
 use crate::planner::operator::Operator;
@@ -22,24 +23,26 @@ impl<'a, T: Transaction> Binder<'a, T> {
         columns: &[ColumnDef],
         constraints: &[TableConstraint],
         if_not_exists: bool,
-    ) -> Result<LogicalPlan, BindError> {
+    ) -> Result<LogicalPlan, DatabaseError> {
         let name = lower_case_name(name);
         let name = split_name(&name)?;
         let table_name = Arc::new(name.to_string());
 
         if !is_valid_identifier(&table_name) {
-            return Err(BindError::InvalidTable("illegal table naming".to_string()));
+            return Err(DatabaseError::InvalidTable(
+                "illegal table naming".to_string(),
+            ));
         }
         {
             // check duplicated column names
             let mut set = HashSet::new();
             for col in columns.iter() {
                 let col_name = &col.name.value;
                 if !set.insert(col_name.clone()) {
-                    return Err(BindError::AmbiguousColumn(col_name.to_string()));
+                    return Err(DatabaseError::AmbiguousColumn(col_name.to_string()));
                 }
                 if !is_valid_identifier(col_name) {
-                    return Err(BindError::InvalidColumn(
+                    return Err(DatabaseError::InvalidColumn(
                         "illegal column naming".to_string(),
                     ));
                 }
@@ -74,7 +77,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         }
 
         if columns.iter().filter(|col| col.desc.is_primary).count() != 1 {
-            return Err(BindError::InvalidTable(
+            return Err(DatabaseError::InvalidTable(
                 "The primary key field must exist and have at least one".to_string(),
             ));
         }
@@ -91,7 +94,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         Ok(plan)
     }
 
-    pub fn bind_column(&mut self, column_def: &ColumnDef) -> Result<ColumnCatalog, BindError> {
+    pub fn bind_column(&mut self, column_def: &ColumnDef) -> Result<ColumnCatalog, DatabaseError> {
         let column_name = column_def.name.to_string();
         let mut column_desc = ColumnDesc::new(
             LogicalType::try_from(column_def.data_type.clone())?,
@@ -138,14 +141,13 @@ mod tests {
     use super::*;
     use crate::binder::BinderContext;
     use crate::catalog::ColumnDesc;
-    use crate::execution::ExecutorError;
     use crate::storage::kip::KipStorage;
     use crate::storage::Storage;
     use crate::types::LogicalType;
     use tempfile::TempDir;
 
     #[tokio::test]
-    async fn test_create_bind() -> Result<(), ExecutorError> {
+    async fn test_create_bind() -> Result<(), DatabaseError> {
         let temp_dir = TempDir::new().expect("unable to create temporary working directory");
         let storage = KipStorage::new(temp_dir.path()).await?;
         let transaction = storage.transaction().await?;

diff --git a/src/binder/delete.rs b/src/binder/delete.rs
@@ -1,4 +1,5 @@
-use crate::binder::{lower_case_name, split_name, BindError, Binder};
+use crate::binder::{lower_case_name, split_name, Binder};
+use crate::errors::DatabaseError;
 use crate::planner::operator::delete::DeleteOperator;
 use crate::planner::operator::scan::ScanOperator;
 use crate::planner::operator::Operator;
@@ -12,7 +13,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         from: &TableWithJoins,
         selection: &Option<Expr>,
-    ) -> Result<LogicalPlan, BindError> {
+    ) -> Result<LogicalPlan, DatabaseError> {
         if let TableFactor::Table { name, alias, .. } = &from.relation {
             let name = lower_case_name(name);
             let name = split_name(&name)?;
@@ -22,7 +23,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
                 .context
                 .table(table_name.clone())
                 .cloned()
-                .ok_or_else(|| BindError::InvalidTable(format!("bind table {}", name)))?;
+                .ok_or_else(|| DatabaseError::InvalidTable(format!("bind table {}", name)))?;
             let primary_key_column = table_catalog
                 .all_columns_with_id()
                 .iter()

diff --git a/src/binder/drop_table.rs b/src/binder/drop_table.rs
@@ -1,4 +1,5 @@
-use crate::binder::{lower_case_name, split_name, BindError, Binder};
+use crate::binder::{lower_case_name, split_name, Binder};
+use crate::errors::DatabaseError;
 use crate::planner::operator::drop_table::DropTableOperator;
 use crate::planner::operator::Operator;
 use crate::planner::LogicalPlan;
@@ -11,7 +12,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
         &mut self,
         name: &ObjectName,
         if_exists: &bool,
-    ) -> Result<LogicalPlan, BindError> {
+    ) -> Result<LogicalPlan, DatabaseError> {
         let name = lower_case_name(name);
         let name = split_name(&name)?;
         let table_name = Arc::new(name.to_string());

diff --git a/src/binder/explain.rs b/src/binder/explain.rs
@@ -1,10 +1,11 @@
-use crate::binder::{BindError, Binder};
+use crate::binder::Binder;
+use crate::errors::DatabaseError;
 use crate::planner::operator::Operator;
 use crate::planner::LogicalPlan;
 use crate::storage::Transaction;
 
 impl<'a, T: Transaction> Binder<'a, T> {
-    pub(crate) fn bind_explain(&mut self, plan: LogicalPlan) -> Result<LogicalPlan, BindError> {
+    pub(crate) fn bind_explain(&mut self, plan: LogicalPlan) -> Result<LogicalPlan, DatabaseError> {
         Ok(LogicalPlan {
             operator: Operator::Explain,
             childrens: vec![plan],