diff --git a/minisql/src/main.rs b/minisql/src/main.rs index c2c6cb6..f3344bd 100644 --- a/minisql/src/main.rs +++ b/minisql/src/main.rs @@ -12,7 +12,7 @@ enum Operation { Delete(TableName, Option), // Update(...), CreateTable(TableName, TableSchema), - CreateIndex(TableName, ColumnName), // TODO: Is this sufficient? + CreateIndex(TableName, ColumnName), // DropTable(TableName), } @@ -44,7 +44,7 @@ enum Condition { // ==============Values and Types================ type UUID = u64; -// TODO: What about nulls? I would rather not have that as in SQL, it sucks. +// TODO: What about nulls? I would rather not have that in SQL, it sucks. // I would rather have non-nullable values by default, // and something like an explicit Option type for nulls. #[derive(Debug, Clone, PartialEq)] @@ -62,7 +62,6 @@ enum IndexableDbValue { // TODO: what bout null? } -// TODO: Can this be autogenerated from the values? #[derive(Debug, Clone, Copy)] enum DbType { String, @@ -72,7 +71,6 @@ enum DbType { } impl DbValue { - // TODO: Can this be autogenerated? fn to_type(self) -> DbType { match self { Self::Number(_) => DbType::Number, @@ -88,8 +86,6 @@ impl DbValue { // ==============Tables================ -// table-metadata and data - type TableName = String; type TablePosition = usize; @@ -102,10 +98,6 @@ struct Table { HashMap // TODO: Consider generalizing `ColumnPosition` to something that would also apply to a pair of `ColumnNames` etc } -// TODO: Is this really indexed by DbValues? -// Maybe we should have a separate index type for each type of value we're indexing over -// TODO: I should have a set of UUID, not just a single UUID, e.g. -// a user table can have multiple different users with the same name. #[derive(Debug)] struct ColumnIndex { index: BTreeMap> @@ -139,7 +131,7 @@ type Rows = // interface // insert(id, value) -fn select_columns(row: &Row, columns: &Vec) -> Row { +fn restrict_columns(row: &Row, columns: &Vec) -> Row { // If the index from `columns` is non-existant in `row`, it will just ignore it. let mut subrow: Row = vec![]; for column_position in columns { @@ -199,7 +191,7 @@ impl State { // Alternative is to pass a row-consumer to the functionas that knows how to communicate with // the client, but the details of communication are hidden behind an interface // - // writer: impl SqlConsumer + // writer: impl SqlResponseConsumer fn interpret(&mut self, operation: Operation) -> DbResult { // TODO: lock stuff use Operation::*; @@ -224,13 +216,10 @@ impl State { CreateTable(table_name, table_schema) => { let table = Table::new(table_schema); self.attach_table(table_name, table); - // TODO: What about attaching index on the primary column? + Ok(Response::TableCreated) }, CreateIndex(table_name, column_name) => { - // TODO: This is incomplete. It can happen that an index is created - // after the table has some rows for a while. - // In such a case the index needs to be built over all those existing rows. let table: &mut Table = self.table_from_name_mut(&table_name)?; let column_position: ColumnPosition = table.schema.column_position_from_column_name(&column_name)?; @@ -245,14 +234,14 @@ impl State { } // TODO: Give a better name to something that you can respond to with rows -trait SqlConsumer { +trait SqlResponseConsumer { // TODO: } impl TableSchema { fn get_column(&self, column_name: &ColumnName) -> DbResult<(DbType, ColumnPosition)> { - match self.column_name_position_mapping.get_by_left(column_name) { + match self.column_name_position_mapping.get_by_left(column_name) { Some(column_position) => { match self.types.get(*column_position) { Some(type_) => { @@ -389,15 +378,16 @@ impl Table { } fn delete_row_by_id(&mut self, id: UUID) -> usize { - if let Some(row) = self.rows.remove(&id) { - for (column_position, column_index) in &mut self.indexes { - if let DbValue::Indexable(value) = &row[*column_position] { - let _ = column_index.remove(value, id); - }; - } - 1 - } else { - 0 + match self.rows.remove(&id) { + Some(row) => { + for (column_position, column_index) in &mut self.indexes { + if let DbValue::Indexable(value) = &row[*column_position] { + let _ = column_index.remove(value, id); + }; + } + 1 + }, + None => 0 } } @@ -416,11 +406,12 @@ impl Table { self.delete_rows_by_ids(matched_ids) } - fn select_where(&self, column_selection: ColumnSelection, maybe_condition: Option) -> DbResult> { + fn select_where(&self, column_selection: ColumnSelection, condition: Option) -> DbResult> { let selected_column_positions = self.schema.column_positions_from_column_selection(&column_selection)?; - match maybe_condition { + match condition { None => - Ok(self.rows.values().map(|row| select_columns(row, &selected_column_positions)).collect()), + // select all + Ok(self.rows.values().map(|row| restrict_columns(row, &selected_column_positions)).collect()), Some(Condition::Eq(eq_column_name, value)) => { let (type_, eq_column_position) = self.schema.get_column(&eq_column_name)?; @@ -428,7 +419,7 @@ impl Table { match value { DbValue::Indexable(IndexableDbValue::UUID(uuid)) => { match self.get_row_by_id(uuid) { - Some(row) => Ok(vec![select_columns(&row, &selected_column_positions)]), + Some(row) => Ok(vec![restrict_columns(&row, &selected_column_positions)]), None => Ok(vec![]), } }, @@ -440,15 +431,15 @@ impl Table { match self.indexes.get(&eq_column_position) { Some(column_index) => { let ids = column_index.get(value); - Ok(self.get_rows_by_ids(ids).iter().map(|row| select_columns(row, &selected_column_positions)).collect()) + Ok(self.get_rows_by_ids(ids).iter().map(|row| restrict_columns(row, &selected_column_positions)).collect()) }, None => { - Ok(self.get_rows_by_value(eq_column_position, &DbValue::Indexable(value)).iter().map(|row| select_columns(row, &selected_column_positions)).collect()) + Ok(self.get_rows_by_value(eq_column_position, &DbValue::Indexable(value)).iter().map(|row| restrict_columns(row, &selected_column_positions)).collect()) } } }, _ => { - Ok(self.get_rows_by_value(eq_column_position, &value).iter().map(|row| select_columns(row, &selected_column_positions)).collect()) + Ok(self.get_rows_by_value(eq_column_position, &value).iter().map(|row| restrict_columns(row, &selected_column_positions)).collect()) } } } @@ -478,8 +469,6 @@ impl Table { } fn delete_where(&mut self, maybe_condition: Option) -> DbResult { - // kinda similar to select with respect to the conditions - // update index match maybe_condition { None => { // delete all @@ -491,8 +480,6 @@ impl Table { Some(Condition::Eq(eq_column_name, value)) => { let (type_, eq_column_position) = self.schema.get_column(&eq_column_name)?; - // TODO: This shouldn't be necessary - we should index the primary column by - // default. The first case shouldn't be a special case! if self.schema.is_primary(eq_column_position) { match value { DbValue::Indexable(IndexableDbValue::UUID(uuid)) => { @@ -610,7 +597,6 @@ fn main() { } - #[cfg(test)] mod tests { use super::*; @@ -622,7 +608,7 @@ mod tests { TableSchema { table_name: "users".to_string(), - primary_key: 0, + primary_key: id, column_name_position_mapping: { let mut mapping: BiMap = BiMap::new(); mapping.insert("id".to_string(), id); @@ -826,10 +812,7 @@ mod tests { let delete_response: Response = state.interpret(Delete(users.clone(), Some(Eq("id".to_string(), id0.clone())))).unwrap(); - // TODO: WTF, why is it 0? Should be 1 assert!(matches!(delete_response, Response::Deleted(1))); - // assert!(matches!(delete_response, Response::Deleted(_))); - let response: Response = state.interpret(Select(users.clone(), All, None)).unwrap();