From 8eec9c6759539b78c409b7ae9cff44263c1dbf3d Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sun, 28 Jan 2024 21:40:43 +0100 Subject: [PATCH 1/3] Fix some of the clippy errors --- minisql/src/internals/table.rs | 15 ++++++--------- minisql/src/interpreter.rs | 6 +++--- minisql/src/restricted_row.rs | 4 ++++ parser/src/core.rs | 8 ++------ proto/src/reader/frontend.rs | 11 ++++++----- proto/src/writer/oneway.rs | 4 ++-- 6 files changed, 23 insertions(+), 25 deletions(-) diff --git a/minisql/src/internals/table.rs b/minisql/src/internals/table.rs index 9dc05de..aff4b37 100644 --- a/minisql/src/internals/table.rs +++ b/minisql/src/internals/table.rs @@ -41,7 +41,7 @@ impl Table { } pub fn table_name(&self) -> &TableName { - &self.schema.table_name() + self.schema.table_name() } // ======Selection====== @@ -69,18 +69,18 @@ impl Table { .collect() } - pub fn select_all_rows<'a>(&'a self, selected_columns: Vec) -> impl Iterator + 'a { + pub fn select_all_rows(&self, selected_columns: Vec) -> impl Iterator + '_ { self.rows .values() .map(move |row| row.restrict_columns(&selected_columns)) } - pub fn select_rows_where_eq<'a>( - &'a self, + pub fn select_rows_where_eq( + &self, selected_columns: Vec, column: Column, value: Value, - ) -> DbResult + 'a> { + ) -> DbResult + '_> { let restrict_columns_of_row = move |row: Row| row.restrict_columns(&selected_columns); match value { Value::Indexable(value) => match self.fetch_ids_from_index(column, &value)? { @@ -116,10 +116,7 @@ impl Table { } for (column, column_index) in &mut self.indexes { - match &row[*column] { - Value::Indexable(val) => column_index.add(val.clone(), id), - _ => {}, - } + if let Value::Indexable(val) = &row[*column] { column_index.add(val.clone(), id) } } let _ = self.rows.insert(id, row); diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index 27f697e..b3d3471 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -55,7 +55,7 @@ impl State { } } - pub fn db_schema<'a>(&'a self) -> DbSchema { + pub fn db_schema(&self) -> DbSchema { let mut schema: DbSchema = Vec::new(); for (table_name, &table_position) in &self.table_name_position_mapping { let table_schema = self.tables[table_position].schema(); @@ -64,11 +64,11 @@ impl State { schema } - fn table_at<'a>(&'a self, table_position: TablePosition) -> &'a Table { + fn table_at(&self, table_position: TablePosition) -> &Table { &self.tables[table_position] } - fn table_at_mut<'a>(&'a mut self, table_position: TablePosition) -> &'a mut Table { + fn table_at_mut(&mut self, table_position: TablePosition) -> &mut Table { &mut self.tables[table_position] } diff --git a/minisql/src/restricted_row.rs b/minisql/src/restricted_row.rs index 8ea9aca..77cafcb 100644 --- a/minisql/src/restricted_row.rs +++ b/minisql/src/restricted_row.rs @@ -28,6 +28,10 @@ impl RestrictedRow { self.0.len() } + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + pub fn iter(&self) -> impl Iterator { self.0.iter() } diff --git a/parser/src/core.rs b/parser/src/core.rs index ec0c140..9001e8f 100644 --- a/parser/src/core.rs +++ b/parser/src/core.rs @@ -1,6 +1,6 @@ use minisql::{operation::Operation, interpreter::DbSchema}; use crate::syntax::RawQuerySyntax; -use nom::{branch::alt, multi::many0, IResult}; +use nom::{branch::alt, 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}}; @@ -13,7 +13,7 @@ pub enum Error { ValidationError(#[from] ValidationError) } -pub fn parse_statement<'a>(input: &'a str) -> IResult<&str, RawQuerySyntax> { +fn parse_statement<'a>(input: &'a str) -> IResult<&str, RawQuerySyntax> { alt(( parse_insert, parse_create, @@ -25,10 +25,6 @@ pub fn parse_statement<'a>(input: &'a str) -> IResult<&str, RawQuerySyntax> { ))(input) } -pub fn parse_statements<'a>(input: &'a str) -> IResult<&str, Vec> { - many0(parse_statement)(input) -} - pub fn parse_and_validate(str_query: String, db_schema: &DbSchema) -> Result { let (_, op) = parse_statement(str_query.as_str()) .map_err(|err| { diff --git a/proto/src/reader/frontend.rs b/proto/src/reader/frontend.rs index bb2ffc8..4d0cf52 100644 --- a/proto/src/reader/frontend.rs +++ b/proto/src/reader/frontend.rs @@ -24,15 +24,15 @@ where R: AsyncBufRead + Unpin + Send, { async fn peek_special_message(&mut self) -> Result, ProtoPeekError> { - if let Some(cancel) = try_get_cancel_request(&mut self).await? { + if let Some(cancel) = try_get_cancel_request(self).await? { return Ok(Some(cancel)); } - if let Some(ssl) = try_get_ssl_request(&mut self).await? { + if let Some(ssl) = try_get_ssl_request(self).await? { return Ok(Some(ssl)); } - if let Some(startup) = try_get_startup_message(&mut self).await? { + if let Some(startup) = try_get_startup_message(self).await? { return Ok(Some(startup)); } @@ -43,11 +43,12 @@ where &mut self, msg: &SpecialMessage, ) -> Result<(), ProtoConsumeError> { - Ok(match msg { + match msg { SpecialMessage::CancelRequest(_) => consume_cancel_request(self), SpecialMessage::SSLRequest => consume_ssl_request(self), SpecialMessage::StartupMessage(_) => consume_startup_message(self).await?, - }) + }; + Ok(()) } } diff --git a/proto/src/writer/oneway.rs b/proto/src/writer/oneway.rs index 30d2665..f4782f2 100644 --- a/proto/src/writer/oneway.rs +++ b/proto/src/writer/oneway.rs @@ -20,12 +20,12 @@ where { async fn write_proto(&mut self, message: T) -> Result<(), ProtoWriteError> { let variant = message.variant(); - let mut data = message.serialize()?; + let data = message.serialize()?; let length = data.len() as i32 + 4; self.inner.write_u8(variant).await?; self.inner.write_i32(length).await?; - self.inner.write_all(&mut data).await?; + self.inner.write_all(&data).await?; Ok(()) } From 2ba158a0d440f7485158b394d5cb8848b79c3a9a Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sun, 28 Jan 2024 22:08:46 +0100 Subject: [PATCH 2/3] Clippy --- minisql/src/internals/column_index.rs | 7 +++++++ minisql/src/internals/row.rs | 7 +++++++ minisql/src/interpreter.rs | 7 +++++++ parser/src/core.rs | 2 +- parser/src/parsing/literal.rs | 7 ++++--- parser/src/parsing/select.rs | 2 +- parser/src/validation.rs | 27 ++++++++++++++------------- 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/minisql/src/internals/column_index.rs b/minisql/src/internals/column_index.rs index c5a5ebb..6d7e2e6 100644 --- a/minisql/src/internals/column_index.rs +++ b/minisql/src/internals/column_index.rs @@ -7,6 +7,13 @@ pub struct ColumnIndex { index: BTreeMap>, } +// To satisfy clippy. +impl Default for ColumnIndex { + fn default() -> Self { + Self::new() + } +} + impl ColumnIndex { pub fn new() -> Self { let index = BTreeMap::new(); diff --git a/minisql/src/internals/row.rs b/minisql/src/internals/row.rs index 504828a..201ace6 100644 --- a/minisql/src/internals/row.rs +++ b/minisql/src/internals/row.rs @@ -39,6 +39,13 @@ impl FromIterator for Row { } } +// To satisfy clippy. +impl Default for Row { + fn default() -> Self { + Self::new() + } +} + impl Row { pub fn new() -> Self { Self(vec![]) diff --git a/minisql/src/interpreter.rs b/minisql/src/interpreter.rs index b3d3471..93045fd 100644 --- a/minisql/src/interpreter.rs +++ b/minisql/src/interpreter.rs @@ -47,6 +47,13 @@ impl std::fmt::Debug for Response<'_> { } } +// To satisfy clippy. +impl Default for State { + fn default() -> Self { + Self::new() + } +} + impl State { pub fn new() -> Self { Self { diff --git a/parser/src/core.rs b/parser/src/core.rs index 9001e8f..d063a07 100644 --- a/parser/src/core.rs +++ b/parser/src/core.rs @@ -13,7 +13,7 @@ pub enum Error { ValidationError(#[from] ValidationError) } -fn parse_statement<'a>(input: &'a str) -> IResult<&str, RawQuerySyntax> { +fn parse_statement(input: &str) -> IResult<&str, RawQuerySyntax> { alt(( parse_insert, parse_create, diff --git a/parser/src/parsing/literal.rs b/parser/src/parsing/literal.rs index 13d2b83..921a8bf 100644 --- a/parser/src/parsing/literal.rs +++ b/parser/src/parsing/literal.rs @@ -28,9 +28,10 @@ pub fn parse_number(input: &str) -> IResult<&str, Value> { Some((_fsign, fdigits)) => { // Combine integer and fractional parts let combined_parts = format!( - "{}{}", - format!("{}{}", sign.unwrap_or('+'), digits), - format!(".{}", fdigits) + "{}{}.{}", + sign.unwrap_or('+'), + digits, + fdigits ); // Parse the combined parts as a floating-point number let value = combined_parts.parse::() diff --git a/parser/src/parsing/select.rs b/parser/src/parsing/select.rs index e7a09a8..e45aa3c 100644 --- a/parser/src/parsing/select.rs +++ b/parser/src/parsing/select.rs @@ -37,7 +37,7 @@ pub fn try_parse_column_selection(input: &str) -> IResult<&str, ColumnSelection> }); let columns_parser = map( separated_list0(terminated(char(','), multispace0), parse_column_name), - |names| ColumnSelection::Columns(names), + ColumnSelection::Columns, ); alt((all_parser, columns_parser))(input) } diff --git a/parser/src/validation.rs b/parser/src/validation.rs index a52ff27..31cae4b 100644 --- a/parser/src/validation.rs +++ b/parser/src/validation.rs @@ -61,7 +61,7 @@ fn validate_table_exists<'a>(db_schema: &DbSchema<'a>, table_name: &'a TableName fn validate_create_table(raw_table_schema: RawTableSchema, db_schema: &DbSchema) -> Result { let table_name: &TableName = &raw_table_schema.table_name; - if let Some(_) = get_table_schema(db_schema, &table_name) { + if get_table_schema(db_schema, table_name).is_some() { return Err(ValidationError::TableAlreadyExists(table_name.to_string())); } @@ -89,10 +89,11 @@ fn validate_table_schema(raw_table_schema: RawTableSchema) -> Result 1 { - return Err(ValidationError::MultiplePrimaryKeysFound(raw_table_schema.table_name.clone())) + let number_of_primary_keys = primary_keys.len(); + if number_of_primary_keys == 0 { + Err(ValidationError::PrimaryKeyMissing(raw_table_schema.table_name.clone())) + } else if number_of_primary_keys > 1 { + Err(ValidationError::MultiplePrimaryKeysFound(raw_table_schema.table_name.clone())) } else { let (primary_column_name, primary_key_type) = primary_keys[0].clone(); if primary_key_type == DbType::Uuid { @@ -113,18 +114,18 @@ fn validate_select(table_name: TableName, column_selection: syntax::ColumnSelect syntax::ColumnSelection::Columns(columns) => { let non_existant_columns: Vec = columns.iter().filter_map(|column| - if schema.does_column_exist(&column) { + if schema.does_column_exist(column) { None } else { Some(column.clone()) }).collect(); - if non_existant_columns.len() > 0 { - Err(ValidationError::ColumnsDoNotExist(non_existant_columns)) - } else { + if non_existant_columns.is_empty() { let selection: operation::ColumnSelection = columns.iter().filter_map(|column_name| schema.get_column(column_name)).collect(); let validated_condition = validate_condition(condition, schema)?; Ok(Operation::Select(table_position, selection, validated_condition)) + } else { + Err(ValidationError::ColumnsDoNotExist(non_existant_columns)) } } syntax::ColumnSelection::All => { @@ -149,11 +150,11 @@ fn validate_insert(table_name: TableName, insertion_values: syntax::InsertionVal 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 { + if !non_existant_columns.is_empty() { return Err(ValidationError::ColumnsDoNotExist(non_existant_columns.iter().map(|column_name| column_name.to_string()).collect())); } let missing_required_columns = Vec::from_iter(columns_in_schema.difference(&columns_in_query)); - if missing_required_columns.len() > 0 { + if !missing_required_columns.is_empty() { return Err(ValidationError::RequiredColumnsAreMissing(missing_required_columns.iter().map(|str| str.to_string()).collect())); } @@ -194,7 +195,7 @@ fn validate_condition(condition: Option, schema: &TableSchema if expected_type.eq(&value_type) { Ok(Some(operation::Condition::Eq(column, value))) } else { - return Err(ValidationError::TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type }); + Err(ValidationError::TypeMismatch { column_name: column_name.to_string(), received_type: value_type, expected_type }) } } } @@ -228,7 +229,7 @@ where T: Eq + std::hash::Hash if already_seen_elements.contains(t) { return Some(t); } else { - already_seen_elements.insert(&t); + already_seen_elements.insert(t); } } None From 0cac6a0094e6bd3b602e44e29ece39863c3bce6d Mon Sep 17 00:00:00 2001 From: Yuriy Dupyn <2153100+omedusyo@users.noreply.github.com> Date: Sun, 28 Jan 2024 22:25:38 +0100 Subject: [PATCH 3/3] Clippy --- client/src/main.rs | 2 +- server/src/main.rs | 10 ++++++---- server/src/proto_wrapper.rs | 17 +++++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/client/src/main.rs b/client/src/main.rs index 66b281d..c8e34f8 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -61,7 +61,7 @@ async fn main() -> anyhow::Result<()> { println!("Ready for query: {:?}", data); line.clear(); let res = std::io::stdin().read_line(&mut line); - if let Ok(_) = res { + if res.is_ok() { if line.eq("exit") { break; } diff --git a/server/src/main.rs b/server/src/main.rs index da7a08a..d5648e5 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -110,8 +110,10 @@ async fn create_token(tokens: &TokenStore) -> anyhow::Result<(i32, i32, ResetCan let mut tokens = tokens.lock().await; loop { let pid_key = random_pid_key(); - if !tokens.contains_key(&pid_key) { - tokens.insert(pid_key, token.clone()); + + use std::collections::hash_map; + if let hash_map::Entry::Vacant(token_entry) = tokens.entry(pid_key) { + token_entry.insert(token.clone()); let (pid, key) = pid_key; return Ok((pid, key, token)); @@ -187,7 +189,7 @@ async fn handle_query(writer: &mut W, state: &SharedDbState, query: String, t true } Response::Selected(schema, columns, mut rows) => { - writer.write_table_header(&schema, &columns).await?; + writer.write_table_header(schema, &columns).await?; match rows.next() { Some(row) => { writer.write_table_row(&row).await?; @@ -223,7 +225,7 @@ async fn handle_query(writer: &mut W, state: &SharedDbState, query: String, t if need_write { let state = state.read().await; - state_to_file(&state, &config.get_file_path()).await?; + state_to_file(&state, config.get_file_path()).await?; } Ok(()) diff --git a/server/src/proto_wrapper.rs b/server/src/proto_wrapper.rs index dfcb7ed..6462847 100644 --- a/server/src/proto_wrapper.rs +++ b/server/src/proto_wrapper.rs @@ -2,6 +2,7 @@ use async_trait::async_trait; use rand::Rng; use rand_pcg::Pcg64; use rand_seeder::Seeder; +use std::fmt; use minisql::operation::ColumnSelection; use minisql::restricted_row::RestrictedRow; use minisql::schema::{Column, TableSchema}; @@ -20,14 +21,14 @@ pub enum CompleteStatus { CreateIndex, } -impl CompleteStatus { - fn to_string(&self) -> String { +impl fmt::Display for CompleteStatus { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - CompleteStatus::Insert { oid, rows } => format!("INSERT {} {}", oid, rows), - CompleteStatus::Delete(rows) => format!("DELETE {}", rows), - CompleteStatus::Select(rows) => format!("SELECT {}", rows), - CompleteStatus::CreateTable => "CREATE TABLE".to_string(), - CompleteStatus::CreateIndex => "CREATE INDEX".to_string(), + CompleteStatus::Insert { oid, rows } => write!(f, "INSERT {} {}", oid, rows), + CompleteStatus::Delete(rows) => write!(f, "DELETE {}", rows), + CompleteStatus::Select(rows) => write!(f, "SELECT {}", rows), + CompleteStatus::CreateTable => write!(f, "CREATE TABLE"), + CompleteStatus::CreateIndex => write!(f, "CREATE INDEX"), } } } @@ -118,4 +119,4 @@ fn column_to_description(schema: &TableSchema, column: Column) -> anyhow::Result fn table_name_to_oid(table_name: &str) -> i32 { let mut rng: Pcg64 = Seeder::from(table_name).make_rng(); rng.gen::() -} \ No newline at end of file +}