From 6000b1f2423bfab31dd47478fc7dee6a6c349bca Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Fri, 26 Jan 2024 19:45:15 +0100 Subject: [PATCH 1/7] Put parsing details into one module --- parser/src/core.rs | 4 ++-- parser/src/lib.rs | 10 ++-------- parser/src/{ => parsing}/common.rs | 6 +++--- parser/src/{ => parsing}/create.rs | 6 +++--- parser/src/{ => parsing}/delete.rs | 6 +++--- parser/src/{ => parsing}/index.rs | 6 +++--- parser/src/{ => parsing}/insert.rs | 4 ++-- parser/src/{ => parsing}/literal.rs | 4 ++-- parser/src/parsing/mod.rs | 7 +++++++ parser/src/{ => parsing}/select.rs | 6 +++--- 10 files changed, 30 insertions(+), 29 deletions(-) rename parser/src/{ => parsing}/common.rs (96%) rename parser/src/{ => parsing}/create.rs (97%) rename parser/src/{ => parsing}/delete.rs (90%) rename parser/src/{ => parsing}/index.rs (95%) rename parser/src/{ => parsing}/insert.rs (98%) rename parser/src/{ => parsing}/literal.rs (98%) create mode 100644 parser/src/parsing/mod.rs rename parser/src/{ => parsing}/select.rs (96%) diff --git a/parser/src/core.rs b/parser/src/core.rs index 778973d..1f97797 100644 --- a/parser/src/core.rs +++ b/parser/src/core.rs @@ -1,7 +1,7 @@ use minisql::{operation::Operation, schema::TableSchema}; use nom::{branch::alt, multi::many0, IResult}; -use crate::{create::parse_create, delete::parse_delete, index::parse_create_index, insert::parse_insert, select::parse_select, validation::{validate_operation, ValidationError}}; +use crate::{parsing::{create::parse_create, delete::parse_delete, index::parse_create_index, insert::parse_insert, select::parse_select}, validation::{validate_operation, ValidationError}}; #[derive(Debug)] pub enum Error { @@ -38,4 +38,4 @@ pub fn parse_and_validate(query: String, db_metadata: &Vec<(String, &TableSchema // #[test] // fn test_select() { // parse_and_validate("SELECT * FROM users;".to_string(), &Vec::new()).unwrap(); -// } \ No newline at end of file +// } diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 1a4cb72..94b121b 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -1,11 +1,5 @@ -mod literal; -mod select; -mod common; -mod create; -mod insert; -mod delete; -mod index; +mod parsing; mod validation; mod core; @@ -13,4 +7,4 @@ pub use core::parse_and_validate; pub use core::Error; pub use validation::validate_operation; -pub use minisql; \ No newline at end of file +pub use minisql; diff --git a/parser/src/common.rs b/parser/src/parsing/common.rs similarity index 96% rename from parser/src/common.rs rename to parser/src/parsing/common.rs index 0cd90c1..23fb866 100644 --- a/parser/src/common.rs +++ b/parser/src/parsing/common.rs @@ -8,7 +8,7 @@ use nom::{ }; use minisql::{operation::Condition, type_system::DbType}; -use crate::literal::parse_db_value; +use super::literal::parse_db_value; pub fn parse_table_name(input: &str) -> IResult<&str, &str> { alt(( @@ -68,7 +68,7 @@ fn parse_equality(input: &str) -> IResult<&str, Condition> { #[cfg(test)] mod tests { use minisql::{operation::Condition, type_system::DbType}; - use crate::common::{parse_db_type, parse_equality}; + use crate::parsing::common::{parse_db_type, parse_equality}; #[test] fn test_parse_equality() { @@ -92,4 +92,4 @@ mod tests { assert!(matches!(parse_db_type("NUMBER").expect("should parse").1, DbType::Number)); assert!(matches!(parse_db_type("Unknown"), Err(_))); } -} \ No newline at end of file +} diff --git a/parser/src/create.rs b/parser/src/parsing/create.rs similarity index 97% rename from parser/src/create.rs rename to parser/src/parsing/create.rs index 4cda32d..94e3538 100644 --- a/parser/src/create.rs +++ b/parser/src/parsing/create.rs @@ -7,7 +7,7 @@ use nom::{ IResult, combinator::opt, }; -use crate::common::{parse_table_name, parse_identifier, parse_db_type}; +use super::common::{parse_table_name, parse_identifier, parse_db_type}; pub fn parse_create(input: &str) -> IResult<&str, Operation> { let (input, _) = tag("CREATE")(input)?; @@ -69,7 +69,7 @@ pub fn parse_column_definition(input: &str) -> IResult<&str, (ColumnName, DbType #[cfg(test)] mod tests { use minisql::operation::Operation; - use crate::create::parse_create; + use crate::parsing::create::parse_create; #[test] fn test_parse_create_no_spaces() { @@ -106,4 +106,4 @@ mod tests { } } -} \ No newline at end of file +} diff --git a/parser/src/delete.rs b/parser/src/parsing/delete.rs similarity index 90% rename from parser/src/delete.rs rename to parser/src/parsing/delete.rs index e1f5a4c..af71cec 100644 --- a/parser/src/delete.rs +++ b/parser/src/parsing/delete.rs @@ -5,7 +5,7 @@ use nom::{ IResult, }; -use crate::common::{parse_table_name, parse_condition}; +use super::common::{parse_table_name, parse_condition}; pub fn parse_delete(input: &str) -> IResult<&str, Operation> { let (input, _) = tag("DELETE")(input)?; @@ -26,7 +26,7 @@ pub fn parse_delete(input: &str) -> IResult<&str, Operation> { #[cfg(test)] mod tests { use minisql::operation::Operation; - use crate::delete::parse_delete; + use crate::parsing::delete::parse_delete; #[test] fn test_parse_delete() { @@ -35,4 +35,4 @@ mod tests { } // TODO: add test with condition -} \ No newline at end of file +} diff --git a/parser/src/index.rs b/parser/src/parsing/index.rs similarity index 95% rename from parser/src/index.rs rename to parser/src/parsing/index.rs index 80f6a96..3130a6b 100644 --- a/parser/src/index.rs +++ b/parser/src/parsing/index.rs @@ -5,7 +5,7 @@ use nom::{ IResult, combinator::opt, }; -use crate::common::{parse_identifier, parse_table_name}; +use super::common::{parse_identifier, parse_table_name}; pub fn parse_create_index(input: &str) -> IResult<&str, Operation> { let (input, _) = tag("CREATE")(input)?; @@ -39,7 +39,7 @@ pub fn parse_create_index(input: &str) -> IResult<&str, Operation> { #[cfg(test)] mod tests { use minisql::operation::Operation; - use crate::index::parse_create_index; + use crate::parsing::index::parse_create_index; #[test] @@ -67,4 +67,4 @@ mod tests { _ => {} } } -} \ No newline at end of file +} diff --git a/parser/src/insert.rs b/parser/src/parsing/insert.rs similarity index 98% rename from parser/src/insert.rs rename to parser/src/parsing/insert.rs index 340e6bf..365be68 100644 --- a/parser/src/insert.rs +++ b/parser/src/parsing/insert.rs @@ -1,4 +1,4 @@ -use crate::{literal::parse_db_value, common::{parse_table_name, parse_identifier}}; +use super::{literal::parse_db_value, common::{parse_table_name, parse_identifier}}; use minisql::{operation::Operation, type_system::Value}; use nom::{ bytes::complete::tag, @@ -91,4 +91,4 @@ mod tests { } } } -} \ No newline at end of file +} diff --git a/parser/src/literal.rs b/parser/src/parsing/literal.rs similarity index 98% rename from parser/src/literal.rs rename to parser/src/parsing/literal.rs index 2552e1e..44e7910 100644 --- a/parser/src/literal.rs +++ b/parser/src/parsing/literal.rs @@ -110,7 +110,7 @@ fn parse_uuid(input: &str) -> IResult<&str, Value> { #[cfg(test)] mod tests { use minisql::type_system::{IndexableValue, Value}; - use crate::literal::{parse_db_value, parse_string}; + use crate::parsing::literal::{parse_db_value, parse_string}; #[test] @@ -161,4 +161,4 @@ mod tests { fn test_parse_int() { assert_eq!(parse_db_value("5134616"), Ok(("", Value::Indexable(IndexableValue::Int(5134616))))); } -} \ No newline at end of file +} diff --git a/parser/src/parsing/mod.rs b/parser/src/parsing/mod.rs new file mode 100644 index 0000000..482deb4 --- /dev/null +++ b/parser/src/parsing/mod.rs @@ -0,0 +1,7 @@ +pub(crate) mod literal; +pub(crate) mod select; +pub(crate) mod common; +pub(crate) mod create; +pub(crate) mod insert; +pub(crate) mod delete; +pub(crate) mod index; diff --git a/parser/src/select.rs b/parser/src/parsing/select.rs similarity index 96% rename from parser/src/select.rs rename to parser/src/parsing/select.rs index 2c6ab71..c3c4292 100644 --- a/parser/src/select.rs +++ b/parser/src/parsing/select.rs @@ -1,4 +1,4 @@ -use crate::common::{parse_table_name, parse_column_name, parse_condition}; +use super::common::{parse_table_name, parse_column_name, parse_condition}; use minisql::operation::{ColumnSelection, Operation}; use nom::{ branch::alt, @@ -45,7 +45,7 @@ pub fn try_parse_column_selection(input: &str) -> IResult<&str, ColumnSelection> #[cfg(test)] mod tests { use minisql::operation::{ColumnSelection, Operation}; - use crate::{common::{parse_column_name, parse_table_name}, select::parse_select}; + use crate::parsing::{common::{parse_column_name, parse_table_name}, select::parse_select}; #[test] @@ -119,4 +119,4 @@ mod tests { } // TODO: a test with multiple statements // TODO: allow underscores in identifiers -} \ No newline at end of file +} From 677fd19bece270ad89513d7862c3aa8501880ffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Moravec?= Date: Sat, 27 Jan 2024 16:34:23 +0100 Subject: [PATCH 2/7] fix: NUMBER type name --- parser/src/parsing/common.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parser/src/parsing/common.rs b/parser/src/parsing/common.rs index 23fb866..f71d587 100644 --- a/parser/src/parsing/common.rs +++ b/parser/src/parsing/common.rs @@ -32,12 +32,12 @@ pub fn parse_column_name(input: &str) -> IResult<&str, String> { } pub fn parse_db_type(input: &str) -> IResult<&str, DbType> { - let (input, type_name) = alt((tag("STRING"), tag("INT"), tag("Float"), tag("UUID")))(input)?; + let (input, type_name) = alt((tag("STRING"), tag("INT"), tag("NUMBER"), tag("UUID")))(input)?; let db_type = match type_name { "STRING" => DbType::String, "INT" => DbType::Int, "UUID" => DbType::Uuid, - "Float" => DbType::Number, + "NUMBER" => DbType::Number, _ => return Err(nom::Err::Failure(make_error(input, nom::error::ErrorKind::IsNot))) }; Ok((input, db_type)) From 5ced11c40d0fab7f952a163fb5303ec34cf48e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Moravec?= Date: Sat, 27 Jan 2024 16:34:44 +0100 Subject: [PATCH 3/7] feat: integrate thiserror --- parser/src/core.rs | 9 ++++++--- parser/src/validation.rs | 10 +++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/parser/src/core.rs b/parser/src/core.rs index 1f97797..248c5c6 100644 --- a/parser/src/core.rs +++ b/parser/src/core.rs @@ -1,12 +1,15 @@ use minisql::{operation::Operation, schema::TableSchema}; use nom::{branch::alt, multi::many0, IResult}; +use thiserror::Error; use crate::{parsing::{create::parse_create, delete::parse_delete, index::parse_create_index, insert::parse_insert, select::parse_select}, validation::{validate_operation, ValidationError}}; -#[derive(Debug)] +#[derive(Debug, Error)] pub enum Error { + #[error("parsing error: {0}")] ParsingError(String), - ValidationError(ValidationError) + #[error("validation error: {0}")] + ValidationError(#[from] ValidationError) } pub fn parse_statement<'a>(input: &'a str) -> IResult<&str, Operation> { @@ -31,7 +34,7 @@ pub fn parse_and_validate(query: String, db_metadata: &Vec<(String, &TableSchema Error::ParsingError(err.to_string()) })?; - validate_operation(&op, db_metadata).map_err(|err| Error::ValidationError(err))?; + validate_operation(&op, db_metadata)?; Ok(op) } diff --git a/parser/src/validation.rs b/parser/src/validation.rs index 2be6dce..795e04f 100644 --- a/parser/src/validation.rs +++ b/parser/src/validation.rs @@ -1,17 +1,25 @@ use std::collections::HashSet; +use thiserror::Error; use minisql::{operation::{ColumnSelection, Condition, InsertionValues, Operation}, schema::TableSchema, type_system::{DbType, IndexableValue, Value}}; -#[derive(Debug)] +#[derive(Debug, Error)] pub enum ValidationError { + #[error("table {0} does not exist")] TableDoesNotExist(String), + #[error("table {0} already exists")] TableExists(String), + #[error("column {0} does not exist")] ColumnDoesNotExist(String), + #[error("bad column position {0}")] BadColumnPosition(usize), + #[error("duplicate column {0}")] DuplicateColumn(String), + #[error("type mismatch")] TypeMismatch, + #[error("value for required column {0} is missing")] ValueForRequiredColumnIsMissing(String) } From 9999d67b8f98240af2f193d0967a86aa62db3398 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sat, 27 Jan 2024 17:14:11 +0100 Subject: [PATCH 4/7] Cleanup validation.rs --- minisql/src/schema.rs | 19 ++++ parser/src/validation.rs | 210 +++++++++++++++++++++------------------ 2 files changed, 134 insertions(+), 95 deletions(-) diff --git a/minisql/src/schema.rs b/minisql/src/schema.rs index ba9759a..c56b4da 100644 --- a/minisql/src/schema.rs +++ b/minisql/src/schema.rs @@ -36,6 +36,24 @@ impl TableSchema { self.types[column_position] } + pub fn get_columns(&self) -> Vec<&ColumnName> { + self.column_name_position_mapping.iter().map(|(name, _)| name).collect() + } + + pub fn does_column_exist(&self, column_name: &ColumnName) -> bool { + self.column_name_position_mapping.contains_left(column_name) + } + + pub fn get_column_position(&self, column_name: &ColumnName) -> Option { + self.column_name_position_mapping.get_by_left(column_name).copied() + } + + pub fn get_type_at(&self, column_name: &ColumnName) -> Option { + let position = self.get_column_position(column_name)?; + self.types.get(position).copied() + } + + // TODO: Get rid of this after validation is merged fn get_column(&self, column_name: &ColumnName) -> DbResult<(DbType, ColumnPosition)> { match self.column_name_position_mapping.get_by_left(column_name) { Some(column_position) => match self.types.get(*column_position) { @@ -52,6 +70,7 @@ impl TableSchema { } } + // TODO: Get rid of this after validation is merged pub fn column_position_from_column_name( &self, column_name: &ColumnName, diff --git a/parser/src/validation.rs b/parser/src/validation.rs index 2be6dce..d978c0c 100644 --- a/parser/src/validation.rs +++ b/parser/src/validation.rs @@ -1,98 +1,107 @@ use std::collections::HashSet; -use minisql::{operation::{ColumnSelection, Condition, InsertionValues, Operation}, schema::TableSchema, type_system::{DbType, IndexableValue, Value}}; +use minisql::{operation::{ColumnSelection, Condition, InsertionValues, Operation}, schema::{TableSchema, ColumnName, TableName}, type_system::DbType}; #[derive(Debug)] pub enum ValidationError { - TableDoesNotExist(String), - TableExists(String), - ColumnDoesNotExist(String), - BadColumnPosition(usize), + TableDoesNotExist(TableName), + TableAlreadyExists(TableName), + ColumnsDoNotExist(Vec), DuplicateColumn(String), - TypeMismatch, - ValueForRequiredColumnIsMissing(String) + TypeMismatch(TypeMismatch), + RequiredColumnsAreMissing(Vec) } -pub fn type_of(value: &Value) -> DbType { - match value { - Value::Indexable(IndexableValue::Int(_)) => DbType::Int, - Value::Indexable(IndexableValue::String(_)) => DbType::String, - Value::Number(_) => DbType::Number, - Value::Indexable(IndexableValue::Uuid(_)) => DbType::Uuid - } +#[derive(Debug)] +pub struct TypeMismatch { + pub column_name: ColumnName, + pub received_type: DbType, + pub expected_type: DbType, } +pub type DbSchema<'a> = Vec<(TableName, &'a TableSchema)>; + /// Validates the operation based on db_metadata -pub fn validate_operation(operation: &Operation, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { +pub fn validate_operation(operation: &Operation, db_schema: &DbSchema) -> Result<(), ValidationError> { match operation { Operation::Select(table_name, column_selection, condition) => { - validate_select(table_name, column_selection, condition, db_metadata)?; + validate_select(table_name, column_selection, condition, db_schema)?; }, Operation::Insert(table_name, insertion_values) => { - validate_insert(&table_name, insertion_values, db_metadata)?; + validate_insert(&table_name, insertion_values, db_schema)?; }, Operation::Delete(table_name, condition) => { - validate_delete(table_name, condition, db_metadata)?; + validate_delete(table_name, condition, db_schema)?; }, // Operation::Update(table_name, insertion_values, condition) => { // validate_update(table_name, insertion_values, db_metadata)?; // }, Operation::CreateTable(table_name, schema) => { - validate_create(table_name, schema, db_metadata)?; + validate_create(table_name, schema, db_schema)?; }, Operation::CreateIndex(table_name, column_name) => { - validate_create_index(table_name, column_name, db_metadata)?; + validate_create_index(table_name, column_name, db_schema)?; }, // Operation::DropTable(table_name) => { - // validate_drop(table_name, db_metadata)?; + // validate_drop(table_name, db_schema)?; // } } Ok(()) } +fn validate_table_exists<'a>(db_schema: &DbSchema<'a>, table_name: &'a TableName) -> Result<&'a TableSchema, ValidationError> { + db_schema.iter().find(|(tname, _)| table_name.eq(tname)) + .ok_or(ValidationError::TableDoesNotExist(table_name.to_string())) + .map(|(_, table_schema)| table_schema).copied() +} + + // pub fn validate_drop(table_name: &str, db_metadata: &Vec<(String, TableSchema)>) -> Result<(), ValidationError> { // db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) // .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; // Ok(()) // } -pub fn validate_create(table_name: &str, schema: &TableSchema, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { - if db_metadata.iter().find(|(tname, _)| table_name.eq(tname)).is_some() { - return Err(ValidationError::TableExists(table_name.to_string())); - } - let mut column_names = HashSet::new(); - for (name, _) in &schema.column_name_position_mapping { - if column_names.contains(name) { - return Err(ValidationError::DuplicateColumn(name.clone())); - } else { - column_names.insert(name.clone()); - } +pub fn validate_create(table_name: &TableName, schema: &TableSchema, db_schema: &DbSchema) -> Result<(), ValidationError> { + if let Some(_) = get_table_schema(db_schema, table_name) { + return Err(ValidationError::TableAlreadyExists(table_name.to_string())); } + find_first_duplicate(&schema.get_columns()) + .map_or_else( + || Ok(()), + |duplicate_column| Err(ValidationError::DuplicateColumn(duplicate_column.to_string())) + )?; + // TODO: Ensure it has a primary key?? Ok(()) } -pub fn validate_select(table_name: &str, column_selection: &ColumnSelection, condition: &Option, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { - let (_, schema) = db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) - .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; +pub fn validate_select(table_name: &TableName, column_selection: &ColumnSelection, condition: &Option, db_schema: &Vec<(TableName, &TableSchema)>) -> Result<(), ValidationError> { + let schema = validate_table_exists(db_schema, table_name)?; match column_selection { ColumnSelection::Columns(columns) => { - columns.iter().find(|c| { - !schema.column_name_position_mapping.contains_left(*c) - }).map_or_else(||Ok(()), |c| Err(ValidationError::ColumnDoesNotExist(c.to_string())))?; + let non_existant_columns: Vec = + columns.iter().filter_map(|column| + if schema.does_column_exist(&column) { + Some(column.clone()) + } else { + None + }).collect(); + if non_existant_columns.len() > 0 { + Err(ValidationError::ColumnsDoNotExist(non_existant_columns)) + } else { + validate_condition(condition, schema) + } } - _ => {} + ColumnSelection::All => Ok(()) } - validate_condition(condition, schema)?; - Ok(()) } // pub fn validate_update(table_name: &str, insertion_values: &InsertionValues, db_metadata: &Vec<(String, TableSchema)>) -> Result<(), ValidationError> { -// let (_, schema) = db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) -// .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; +// let schema = validate_table_exists(db_schema, table_name)?; // let mut column_names = HashSet::new(); // // Find duplicate columns // for (name, _) in insertion_values { @@ -111,7 +120,7 @@ pub fn validate_select(table_name: &str, column_selection: &ColumnSelection, con // if let Some((name, _, _)) = column_value_type.iter().find(|(_, _, t)| { // t.is_none() // }) { -// return Err(ValidationError::ColumnDoesNotExist((*name).clone())); +// return Err(ValidationError::ColumnsDoNotExist(vec![(*name).clone())]); // } // // Check types @@ -128,63 +137,56 @@ pub fn validate_select(table_name: &str, column_selection: &ColumnSelection, con // Ok(()) // } -pub fn validate_insert(table_name: &str, insertion_values: &InsertionValues, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { - let (_, schema) = db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) - .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; - let inserted_columns: HashSet = HashSet::from_iter(insertion_values.iter().map(|(name, _)| name.clone())); - // TODO: primary key is not required - for (column_name, _) in &schema.column_name_position_mapping { - if !inserted_columns.contains(column_name) { - return Err(ValidationError::ValueForRequiredColumnIsMissing(column_name.clone())) - } +pub fn validate_insert(table_name: &TableName, insertion_values: &InsertionValues, db_schema: &DbSchema) -> Result<(), ValidationError> { + let schema = validate_table_exists(db_schema, table_name)?; + + // Check for duplicate columns in insertion_values. + let columns_in_query_vec: Vec<&ColumnName> = insertion_values.iter().map(|(column_name, _)| column_name).collect(); + find_first_duplicate(&columns_in_query_vec) + .map_or_else( + || Ok(()), + |duplicate_column| Err(ValidationError::DuplicateColumn(duplicate_column.to_string())) + )?; + + // Check that the set of columns in the insertion_values is the same as the set of required columns of the table. + let columns_in_query: HashSet<&ColumnName> = HashSet::from_iter(columns_in_query_vec); + let columns_in_schema: HashSet<&ColumnName> = HashSet::from_iter(schema.get_columns()); + let non_existant_columns = Vec::from_iter(columns_in_query.difference(&columns_in_schema)); + if non_existant_columns.len() > 0 { + return Err(ValidationError::ColumnsDoNotExist(non_existant_columns.iter().map(|str| str.to_string()).collect())); } - // Ensure columns exist in schema - let column_value_type: Vec<_> = insertion_values.iter().map(|(column, value)| { - (column, value, schema.column_name_position_mapping.iter().find(|(name, _) | { - (*name).eq(column) - }).map(|(_, t)| schema.types.get(*t as usize))) - }).collect(); - if let Some((name, _, _)) = column_value_type.iter().find(|(_, _, t)| { - match t { - Some(Some(_)) => false, - _ => true - } - }) { - return Err(ValidationError::ColumnDoesNotExist((*name).clone())); + let missing_required_columns = Vec::from_iter(columns_in_schema.difference(&columns_in_query)); + if missing_required_columns.len() > 0 { + return Err(ValidationError::RequiredColumnsAreMissing(missing_required_columns.iter().map(|str| str.to_string()).collect())); } // Check types - if let Some((_, _, _)) = column_value_type.iter().find(|(_, value, t)| { - if let Some(Some(t)) = t { - !type_of(value).eq(t) - } else { - false + for (column_name, value) in insertion_values { + let expected_type = schema.get_type_at(column_name).ok_or(ValidationError::ColumnsDoNotExist(vec![column_name.to_string()]))?; // By the previous validation steps this is never gonna trigger an error. + let value_type = value.to_type(); + if value_type != expected_type { + return Err(ValidationError::TypeMismatch(TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type })); } - }) { - // TODO: Add column name information - return Err(ValidationError::TypeMismatch); } + Ok(()) } -pub fn validate_delete(table_name: &str, condition: &Option, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { - let (_, schema) = db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) - .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; +pub fn validate_delete(table_name: &TableName, condition: &Option, db_schema: &DbSchema) -> Result<(), ValidationError> { + let schema = validate_table_exists(db_schema, table_name)?; validate_condition(condition, schema)?; Ok(()) } fn validate_condition(condition: &Option, schema: &TableSchema) -> Result<(), ValidationError> { match condition { - Some(c) => { - match c { - Condition::Eq(left, right) => { - let position = schema.column_name_position_mapping.get_by_left(left) - .ok_or(ValidationError::ColumnDoesNotExist(left.clone()))?; - let column_type = schema.types.get(*position as usize) - .ok_or(ValidationError::BadColumnPosition(*position))?; - if !column_type.eq(&type_of(right)) { - return Err(ValidationError::TypeMismatch); + Some(condition) => { + match condition { + Condition::Eq(column_name, value) => { + let expected_type: DbType = schema.get_type_at(column_name).ok_or(ValidationError::ColumnsDoNotExist(vec![column_name.to_string()]))?; + let value_type: DbType = value.to_type(); + if !expected_type.eq(&value_type) { + return Err(ValidationError::TypeMismatch(TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type })); } } } @@ -194,13 +196,31 @@ fn validate_condition(condition: &Option, schema: &TableSchema) -> Re Ok(()) } -fn validate_create_index(table_name: &str, column_name: &str, db_metadata: &Vec<(String, &TableSchema)>) -> Result<(), ValidationError> { - // Ensure table exists - let (_, schema) = db_metadata.iter().find(|(tname, _)| table_name.eq(tname)) - .ok_or(ValidationError::TableDoesNotExist(table_name.to_string()))?; - // Ensure column exists - if !schema.column_name_position_mapping.contains_left(column_name) { - return Err(ValidationError::ColumnDoesNotExist(column_name.to_string())); +fn validate_create_index(table_name: &TableName, column_name: &ColumnName, db_schema: &DbSchema) -> Result<(), ValidationError> { + let schema = validate_table_exists(db_schema, table_name)?; + if schema.does_column_exist(column_name) { + Ok(()) + } else { + Err(ValidationError::ColumnsDoNotExist(vec![column_name.to_string()])) } - Ok(()) -} \ No newline at end of file +} + +// ===Helpers=== +fn find_first_duplicate(xs: &[A]) -> Option<&A> +where A: Eq + std::hash::Hash +{ + let mut already_seen_elements: HashSet<&A> = HashSet::new(); + for x in xs { + if already_seen_elements.contains(x) { + return Some(x); + } else { + already_seen_elements.insert(&x); + } + } + None +} + +fn get_table_schema<'a>(db_schema: &DbSchema<'a>, table_name: &'a TableName) -> Option<&'a TableSchema> { + let (_, table_schema) = db_schema.iter().find(|(tname, _)| table_name.eq(tname))?; + Some(table_schema) +} From 4e5959a53a5c1852c50a52a799de51e0ba120955 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sat, 27 Jan 2024 17:26:00 +0100 Subject: [PATCH 5/7] Fix formatting of validation errors --- parser/src/validation.rs | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/parser/src/validation.rs b/parser/src/validation.rs index 870b11a..2c20c24 100644 --- a/parser/src/validation.rs +++ b/parser/src/validation.rs @@ -11,29 +11,20 @@ pub enum ValidationError { TableDoesNotExist(TableName), #[error("table {0} already exists")] TableAlreadyExists(TableName), - // TODO - // #[error("columns {0} do not exist")] - #[error("columns do not exist")] + #[error("columns {0:?} do not exist")] ColumnsDoNotExist(Vec), #[error("duplicate column {0}")] DuplicateColumn(ColumnName), - // TODO: You need to actually print the error message - #[error("type mismatch")] - TypeMismatch(TypeMismatch), - // TODO - // #[error("values for required columns {0} are missing")] - #[error("values for required columns are missing")] + #[error("type mismatch at column `{column_name:?}` (expected {expected_type:?}, found {received_type:?})")] + TypeMismatch { + column_name: ColumnName, + received_type: DbType, + expected_type: DbType, + }, + #[error("values for required columns {0:?} are missing")] RequiredColumnsAreMissing(Vec) } -// TODO: Add derive(Error) -#[derive(Debug)] -pub struct TypeMismatch { - pub column_name: ColumnName, - pub received_type: DbType, - pub expected_type: DbType, -} - pub type DbSchema<'a> = Vec<(TableName, &'a TableSchema)>; /// Validates the operation based on db_metadata @@ -178,7 +169,7 @@ pub fn validate_insert(table_name: &TableName, insertion_values: &InsertionValue let expected_type = schema.get_type_at(column_name).ok_or(ValidationError::ColumnsDoNotExist(vec![column_name.to_string()]))?; // By the previous validation steps this is never gonna trigger an error. let value_type = value.to_type(); if value_type != expected_type { - return Err(ValidationError::TypeMismatch(TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type })); + return Err(ValidationError::TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type }); } } @@ -199,7 +190,7 @@ fn validate_condition(condition: &Option, schema: &TableSchema) -> Re let expected_type: DbType = schema.get_type_at(column_name).ok_or(ValidationError::ColumnsDoNotExist(vec![column_name.to_string()]))?; let value_type: DbType = value.to_type(); if !expected_type.eq(&value_type) { - return Err(ValidationError::TypeMismatch(TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type })); + return Err(ValidationError::TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type }); } } } From cf76cc4d10e269408b79f141976e70e6d0ec4e58 Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sat, 27 Jan 2024 18:11:12 +0100 Subject: [PATCH 6/7] Restore schema fields to private --- minisql/src/schema.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minisql/src/schema.rs b/minisql/src/schema.rs index c56b4da..e029606 100644 --- a/minisql/src/schema.rs +++ b/minisql/src/schema.rs @@ -12,8 +12,8 @@ use std::collections::HashMap; pub struct TableSchema { table_name: TableName, // used for descriptive errors primary_key: ColumnPosition, - pub column_name_position_mapping: BiMap, - pub types: Vec, + column_name_position_mapping: BiMap, + types: Vec, } pub type TableName = String; From 464c0b6698215b0c56bd4682fc82ef7eedffca4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20Moravec?= Date: Sat, 27 Jan 2024 18:47:43 +0100 Subject: [PATCH 7/7] fix: parse uuid with 'u' prefix --- parser/src/parsing/literal.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/parser/src/parsing/literal.rs b/parser/src/parsing/literal.rs index 44e7910..13d2b83 100644 --- a/parser/src/parsing/literal.rs +++ b/parser/src/parsing/literal.rs @@ -100,17 +100,17 @@ pub fn parse_string(input: &str) -> IResult<&str, Value> { Ok((input, Value::Indexable(IndexableValue::String(value)))) } -fn parse_uuid(input: &str) -> IResult<&str, Value> { - // TODO: make it actually uuid - u64(input).map(|(input, v)| { +pub fn parse_uuid(input: &str) -> IResult<&str, Value> { + let (input, value) = pair(char('u'), u64)(input).map(|(input, (_, v))| { (input, Value::Indexable(IndexableValue::Uuid(v))) - }) + })?; + Ok((input, value)) } #[cfg(test)] mod tests { use minisql::type_system::{IndexableValue, Value}; - use crate::parsing::literal::{parse_db_value, parse_string}; + use crate::parsing::literal::{parse_db_value, parse_string, parse_uuid}; #[test] @@ -161,4 +161,9 @@ mod tests { fn test_parse_int() { assert_eq!(parse_db_value("5134616"), Ok(("", Value::Indexable(IndexableValue::Int(5134616))))); } + + #[test] + fn test_parse_uuid() { + assert_eq!(parse_uuid("u131515"), Ok(("", Value::Indexable(IndexableValue::Uuid(131515))))) + } }