Remove AlterTable (#129)

* New

* Done, just need to implement

* Rudamentary impl

* Rename table for sled

* fix

* `cargo fix` & `cargo fmt`

* Nearly ready

* IndexAdd doesn't need action

* fmt

* ColumnAdd

* Sled ColumnRemove

* Fixes & Sled IndexRemove

Author: KyGost

src/data/mod.rs

@@ -11,7 +11,7 @@ pub use {
 	index::{Index, IndexFilter},
 	join::{join_iters, JoinType},
 	row::{Row, RowError},
-	schema::Schema,
+	schema::*,
 	table::{get_name, Table, TableError},
 	value::*,
 };

src/data/schema.rs

@@ -1,6 +1,7 @@
 use {
 	crate::{Column, Index},
 	serde::{Deserialize, Serialize},
+	std::collections::HashMap,
 };
 
 #[derive(Clone, Serialize, Deserialize)]
@@ -9,3 +10,175 @@ pub struct Schema {
 	pub column_defs: Vec<Column>,
 	pub indexes: Vec<Index>,
 }
+
+#[derive(Clone, Default)]
+pub struct SchemaDiff {
+	pub table_name: Option<String>,
+	pub column_defs: Option<HashMap<Option<usize>, Option<Column>>>,
+	pub indexes: Option<HashMap<Option<usize>, Option<Index>>>,
+}
+impl SchemaDiff {
+	pub fn new_rename(new_name: String) -> Self {
+		Self {
+			table_name: Some(new_name),
+			column_defs: None,
+			indexes: None,
+		}
+	}
+	pub fn new_add_column(new_column: Column) -> Self {
+		Self {
+			table_name: None,
+			column_defs: Some([(None, Some(new_column))].into()),
+			indexes: None,
+		}
+	}
+	pub fn new_remove_column(column_index: usize) -> Self {
+		Self {
+			table_name: None,
+			column_defs: Some([(Some(column_index), None)].into()),
+			indexes: None,
+		}
+	}
+	pub fn new_rename_column(
+		column_index: usize,
+		mut column: Column,
+		new_column_name: String,
+	) -> Self {
+		column.name = new_column_name;
+		Self {
+			table_name: None,
+			column_defs: Some([(Some(column_index), Some(column))].into()),
+			indexes: None,
+		}
+	}
+	pub fn new_add_index(new_index: Index) -> Self {
+		Self {
+			table_name: None,
+			column_defs: None,
+			indexes: Some([(None, Some(new_index))].into()),
+		}
+	}
+}
+
+impl SchemaDiff {
+	pub fn merge(self, mut schema: Schema) -> Schema {
+		if let Some(table_name) = self.table_name {
+			schema.table_name = table_name
+		}
+		if let Some(column_defs) = self.column_defs {
+			for (index, column_def) in column_defs.into_iter() {
+				match (index, column_def) {
+					(None, None) => (),
+					(Some(index), None) => {
+						schema.column_defs.remove(index);
+					} // TODO: WARN: Will be an issue if multiple change
+					(Some(index), Some(column_def)) => {
+						schema
+							.column_defs
+							.get_mut(index)
+							.map(|old_column_def| *old_column_def = column_def);
+					}
+					(None, Some(column_def)) => {
+						schema.column_defs.push(column_def);
+					}
+				}
+			}
+		}
+		if let Some(indexes) = self.indexes {
+			for (index, index_def) in indexes.into_iter() {
+				match (index, index_def) {
+					(None, None) => (),
+					(Some(index), None) => {
+						schema.indexes.remove(index);
+					} // TODO: WARN: Will be an issue if multiple change
+					(Some(index), Some(index_def)) => {
+						schema
+							.indexes
+							.get_mut(index)
+							.map(|old_index_def| *old_index_def = index_def);
+					}
+					(None, Some(index_def)) => {
+						schema.indexes.push(index_def);
+					}
+				}
+			}
+		}
+		schema
+	}
+}
+
+impl From<Schema> for SchemaDiff {
+	fn from(from: Schema) -> Self {
+		let column_defs = from
+			.column_defs
+			.into_iter()
+			.enumerate()
+			.map(|(key, col)| (Some(key), Some(col)))
+			.collect::<HashMap<Option<usize>, Option<Column>>>();
+		let indexes = from
+			.indexes
+			.into_iter()
+			.enumerate()
+			.map(|(key, idx)| (Some(key), Some(idx)))
+			.collect::<HashMap<Option<usize>, Option<Index>>>();
+		Self {
+			table_name: Some(from.table_name),
+			column_defs: Some(column_defs),
+			indexes: Some(indexes),
+		}
+	}
+}
+
+pub enum SchemaChange {
+	RenameTable(String),
+
+	ColumnUpdate(usize, Column),
+	ColumnAdd(Column),
+	ColumnRemove(usize),
+
+	IndexUpdate(usize, Index),
+	IndexAdd(Index),
+	IndexRemove(usize),
+}
+impl SchemaDiff {
+	pub fn get_changes(&self) -> Vec<SchemaChange> {
+		use SchemaChange::*;
+		let mut changes = Vec::new();
+		if let Some(table_name) = &self.table_name {
+			changes.push(RenameTable(table_name.clone()))
+		}
+		if let Some(column_defs) = &self.column_defs {
+			for (index, column_def) in column_defs.into_iter() {
+				match (index, column_def) {
+					(None, None) => (),
+					(Some(index), Some(column_def)) => {
+						changes.push(ColumnUpdate(*index, column_def.clone()));
+					}
+					(None, Some(column_def)) => {
+						changes.push(ColumnAdd(column_def.clone()));
+					}
+					(Some(index), None) => {
+						changes.push(ColumnRemove(*index));
+					}
+				}
+			}
+		}
+		if let Some(indexes) = &self.indexes {
+			for (index, index_def) in indexes.into_iter() {
+				match (index, index_def) {
+					(None, None) => (),
+					(Some(index), Some(index_def)) => {
+						changes.push(IndexUpdate(*index, index_def.clone()));
+					}
+					(None, Some(index_def)) => {
+						changes.push(IndexAdd(index_def.clone()));
+					}
+					(Some(index), None) => {
+						changes.push(IndexRemove(*index));
+					}
+				}
+			}
+		}
+		changes
+	}
+}

src/executor/alter_table/alter_table.rs

@@ -1,6 +1,6 @@
 use {
 	super::{validate, AlterError},
-	crate::{data::get_name, Error, Glue, Result},
+	crate::{data::get_name, Error, Glue, Result, SchemaDiff},
 	sqlparser::ast::{AlterTableOperation, ObjectName},
 };
 
@@ -13,37 +13,65 @@ impl Glue {
 		let table_name = get_name(name).map_err(Error::from)?;
 		let database = &mut **self.get_mut_database(&None)?;
 
-		match operation {
+		let diff = match operation {
 			AlterTableOperation::RenameTable {
 				table_name: new_table_name,
 			} => {
 				let new_table_name = get_name(new_table_name).map_err(Error::from)?;
 
-				database.rename_schema(table_name, new_table_name).await
+				SchemaDiff::new_rename(new_table_name.clone())
 			}
 			AlterTableOperation::RenameColumn {
 				old_column_name,
 				new_column_name,
 			} => {
-				database
-					.rename_column(table_name, &old_column_name.value, &new_column_name.value)
-					.await
+				let schema = database
+					.fetch_schema(table_name)
+					.await?
+					.ok_or(AlterError::TableNotFound(table_name.clone()))?;
+				let (column_index, column) = schema
+					.column_defs
+					.into_iter()
+					.enumerate()
+					.find(|(_, column)| column.name == old_column_name.value)
+					.ok_or(AlterError::ColumnNotFound(
+						table_name.clone(),
+						old_column_name.value.clone(),
+					))?;
+				SchemaDiff::new_rename_column(column_index, column, new_column_name.value.clone())
 			}
 			AlterTableOperation::AddColumn { column_def } => {
 				validate(column_def).map_err(Error::from)?;
 
-				database.add_column(table_name, &column_def.into()).await
+				SchemaDiff::new_add_column(column_def.into())
 			}
 			AlterTableOperation::DropColumn {
 				column_name,
-				if_exists,
+				if_exists: _,
 				..
 			} => {
-				database
-					.drop_column(table_name, &column_name.value, *if_exists)
-					.await
+				let schema = database
+					.fetch_schema(table_name)
+					.await?
+					.ok_or(AlterError::TableNotFound(table_name.clone()))?;
+				let (column_index, _) = schema
+					.column_defs
+					.into_iter()
+					.enumerate()
+					.find(|(_, column)| column.name == column_name.value)
+					.ok_or(AlterError::ColumnNotFound(
+						table_name.clone(),
+						column_name.value.clone(),
+					))?;
+
+				SchemaDiff::new_remove_column(column_index)
+			}
+			_ => {
+				return Err(
+					AlterError::UnsupportedAlterTableOperation(operation.to_string()).into(),
+				)
 			}
-			_ => Err(AlterError::UnsupportedAlterTableOperation(operation.to_string()).into()),
-		}
+		};
+		database.alter_table(table_name, diff).await
 	}
 }

src/executor/alter_table/create_index.rs

@@ -1,5 +1,5 @@
 use {
-	crate::{data::get_name, AlterError, ExecuteError, Glue, Index, Result},
+	crate::{data::get_name, AlterError, ExecuteError, Glue, Index, Result, SchemaDiff},
 	sqlparser::ast::{Expr, ObjectName, OrderByExpr},
 };
 
@@ -57,13 +57,14 @@ impl Glue {
 				)
 				.into())
 			} else if let Some(column) = column {
-				let mut schema = schema.clone();
+				let schema = schema.clone();
 				let index = Index::new(name, column, unique);
 				index
 					.reset(database, table_name, &schema.column_defs)
 					.await?;
-				schema.indexes.push(index);
-				database.replace_schema(table_name, schema).await
+				database
+					.alter_table(table_name, SchemaDiff::new_add_index(index))
+					.await
 			} else {
 				unreachable!()
 			}

src/result.rs

@@ -9,9 +9,6 @@ use {
 	thiserror::Error as ThisError,
 };
 
-#[cfg(feature = "alter-table")]
-use crate::store::AlterTableError;
-
 #[derive(ThisError, Serialize, Debug, PartialEq)]
 pub enum WIPError {
 	#[error("TODO")]
@@ -22,10 +19,6 @@ pub enum WIPError {
 
 #[derive(ThisError, Serialize, Debug)]
 pub enum Error {
-	#[cfg(feature = "alter-table")]
-	#[error(transparent)]
-	AlterTable(#[from] AlterTableError),
-
 	#[error(transparent)]
 	#[serde(with = "stringify")]
 	Storage(#[from] Box<dyn std::error::Error>),
@@ -81,8 +74,6 @@ impl PartialEq for Error {
 		use Error::*;
 
 		match (self, other) {
-			#[cfg(feature = "alter-table")]
-			(AlterTable(l), AlterTable(r)) => l == r,
 			(Execute(l), Execute(r)) => l == r,
 			(Alter(l), Alter(r)) => l == r,
 			(Fetch(l), Fetch(r)) => l == r,

src/storages/csv_storage/mod.rs

@@ -2,15 +2,9 @@ mod auto_increment;
 mod store;
 mod store_mut;
 mod utils;
-/*mod alter_table;
-mod error;
-#[cfg(not(feature = "alter-table"))]
-impl crate::AlterTable for CSVStorage {}
-#[cfg(not(feature = "auto-increment"))]
-impl crate::AutoIncrement for CSVStorage {}*/
 
 use {
-	crate::{data::Schema, store::*, Column, FullStorage, Result, Storage, ValueType, WIPError},
+	crate::{data::Schema, Column, FullStorage, Result, Storage, ValueType, WIPError},
 	csv::ReaderBuilder,
 	serde::{Deserialize, Serialize},
 	std::{
@@ -46,7 +40,6 @@ impl Default for CSVSettings {
 	}
 }
 
-impl AlterTable for CSVStorage {}
 impl FullStorage for CSVStorage {}
 
 impl Storage {

src/storages/memory/alter_table.rs

@@ -1,4 +1,3 @@
-mod alter_table;
 mod auto_increment;
 mod store;
 mod store_mut;