Rename column_position ~> column variables, and introduce runtime error AttemptToIndexAlreadyIndexedColumn

minisql/src/error.rs

@@ -10,6 +10,8 @@ pub enum RuntimeError {
     AttemptingToInsertAlreadyPresentId(TableName, Uuid),
     #[error("table {0} cannot be indexed on column {1}")]
     AttemptToIndexNonIndexableColumn(TableName, ColumnName),
+    #[error("table {0} already indexes column {1}")]
+    AttemptToIndexAlreadyIndexedColumn(TableName, ColumnName),
 }
 
 #[derive(Debug, Error)]

minisql/src/internals/row.rs

@@ -60,16 +60,16 @@ impl Row {
         self.0.len()
     }
 
-    pub fn get(&self, column_position: ColumnPosition) -> Option<&Value> {
+    pub fn get(&self, column: ColumnPosition) -> Option<&Value> {
-        self.0.get(column_position)
+        self.0.get(column)
     }
 
     pub fn restrict_columns(&self, columns: &Vec<ColumnPosition>) -> RestrictedRow {
         // If the index from `columns` is non-existant in `row`, it will just ignore it.
         let mut subrow: Vec<(ColumnPosition, Value)> = vec![];
-        for column_position in columns {
+        for column in columns {
-            if let Some(value) = self.get(*column_position) {
+            if let Some(value) = self.get(*column) {
-                subrow.push((*column_position, value.clone()));
+                subrow.push((*column, value.clone()));
             }
         }
 

minisql/src/internals/table.rs

@@ -54,12 +54,12 @@ impl Table {
             .collect()
     }
 
-    fn get_rows_by_value(&self, column_position: ColumnPosition, value: &Value) -> Vec<Row> {
+    fn get_rows_by_value(&self, column: ColumnPosition, value: &Value) -> Vec<Row> {
         // brute-force search
         self.rows
             .values()
             .filter_map(|row| {
-                if row.get(column_position) == Some(value) {
+                if row.get(column) == Some(value) {
                     Some(row.clone())
                 } else {
                     None
@@ -68,21 +68,21 @@ impl Table {
             .collect()
     }
 
-    pub fn select_all_rows<'a>(&'a self, selected_column_positions: Vec<ColumnPosition>) -> impl Iterator<Item=RestrictedRow> + 'a {
+    pub fn select_all_rows<'a>(&'a self, selected_columns: Vec<ColumnPosition>) -> impl Iterator<Item=RestrictedRow> + 'a {
         self.rows
             .values()
-            .map(move |row| row.restrict_columns(&selected_column_positions))
+            .map(move |row| row.restrict_columns(&selected_columns))
     }
 
     pub fn select_rows_where_eq<'a>(
         &'a self,
-        selected_column_positions: Vec<ColumnPosition>,
+        selected_columns: Vec<ColumnPosition>,
-        column_position: ColumnPosition,
+        column: ColumnPosition,
         value: Value,
     ) -> DbResult<impl Iterator<Item=RestrictedRow> + 'a> {
-        let restrict_columns_of_row = move |row: Row| row.restrict_columns(&selected_column_positions);
+        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_position, &value)? {
+            Value::Indexable(value) => match self.fetch_ids_from_index(column, &value)? {
                 Some(ids) =>
                     Ok(self
                         .get_rows_by_ids(ids)
@@ -91,14 +91,14 @@ impl Table {
                     ),
                 None =>
                     Ok(self
-                        .get_rows_by_value(column_position, &Value::Indexable(value))
+                        .get_rows_by_value(column, &Value::Indexable(value))
                         .into_iter()
                         .map(restrict_columns_of_row)
                     ),
             },
             _ =>
                 Ok(self
-                    .get_rows_by_value(column_position, &value)
+                    .get_rows_by_value(column, &value)
                     .into_iter()
                     .map(restrict_columns_of_row)
                 ),
@@ -129,8 +129,8 @@ impl Table {
     fn delete_row_by_id(&mut self, id: Uuid) -> usize {
         match self.rows.remove(&id) {
             Some(row) => {
-                for (column_position, column_index) in &mut self.indexes {
+                for (column, column_index) in &mut self.indexes {
-                    if let Value::Indexable(value) = &row[*column_position] {
+                    if let Value::Indexable(value) = &row[*column] {
                         let _ = column_index.remove(value, id);
                     };
                 }
@@ -148,12 +148,12 @@ impl Table {
         total_count
     }
 
-    fn delete_rows_by_value(&mut self, column_position: ColumnPosition, value: &Value) -> usize {
+    fn delete_rows_by_value(&mut self, column: ColumnPosition, value: &Value) -> usize {
         let matched_ids: HashSet<Uuid> = self
             .rows
             .iter()
             .filter_map(|(id, row)| {
-                if row.get(column_position) == Some(value) {
+                if row.get(column) == Some(value) {
                     Some(*id)
                 } else {
                     None
@@ -172,38 +172,43 @@ impl Table {
 
     pub fn delete_rows_where_eq(
         &mut self,
-        column_position: ColumnPosition,
+        column: ColumnPosition,
         value: Value,
     ) -> DbResult<usize> {
         match value {
-            Value::Indexable(value) => match self.fetch_ids_from_index(column_position, &value)? {
+            Value::Indexable(value) => match self.fetch_ids_from_index(column, &value)? {
                 Some(ids) => Ok(self.delete_rows_by_ids(ids)),
-                None => Ok(self.delete_rows_by_value(column_position, &Value::Indexable(value))),
+                None => Ok(self.delete_rows_by_value(column, &Value::Indexable(value))),
             },
-            _ => Ok(self.delete_rows_by_value(column_position, &value)),
+            _ => Ok(self.delete_rows_by_value(column, &value)),
         }
     }
 
     // ======Indexing======
-    pub fn attach_index(&mut self, column_position: ColumnPosition) -> DbResult<()> {
+    pub fn attach_index(&mut self, column: ColumnPosition) -> DbResult<()> {
+        if self.indexes.get(&column).is_some() {
+            let column_name = self.schema.column_name_from_column(column).clone();
+            let table_name = self.schema.table_name().clone();
+            return Err(RuntimeError::AttemptToIndexAlreadyIndexedColumn(table_name, column_name))
+        }
         let mut column_index: ColumnIndex = ColumnIndex::new();
-        update_index_from_table(&mut column_index, self, column_position)?;
+        update_index_from_table(&mut column_index, self, column)?;
-        self.indexes.insert(column_position, column_index);
+        self.indexes.insert(column, column_index);
         Ok(())
     }
 
     fn fetch_ids_from_index(
         &self,
-        column_position: ColumnPosition,
+        column: ColumnPosition,
         value: &IndexableValue,
     ) -> DbResult<Option<HashSet<Uuid>>> {
-        if self.schema.is_primary(column_position) {
+        if self.schema.is_primary(column) {
             match value {
                 IndexableValue::Uuid(id) => Ok(Some(HashSet::from([*id]))),
                 _ => unreachable!() // SAFETY: Validation guarantees primary column has correct Uuid type.
             }
         } else {
-            match self.indexes.get(&column_position) {
+            match self.indexes.get(&column) {
                 Some(index) => {
                     // Note that we are cloning the ids here! This can be very wasteful in some cases.
                     // Theoretically it would be possible to return a reference,

minisql/src/schema.rs

@@ -20,8 +20,8 @@ pub type ColumnName = String;
 impl TableSchema {
     pub fn new(table_name: TableName, primary_key: ColumnPosition, column_name_position_map: Vec<(ColumnName, ColumnPosition)>, types: Vec<DbType>) -> Self {
         let mut column_name_position_mapping: BiMap<ColumnName, ColumnPosition> = BiMap::new();
-        for (column_name, column_position) in column_name_position_map {
+        for (column_name, column) in column_name_position_map {
-            column_name_position_mapping.insert(column_name, column_position);
+            column_name_position_mapping.insert(column_name, column);
         }
         Self { table_name, primary_key, column_name_position_mapping, types }
     }
@@ -30,8 +30,8 @@ impl TableSchema {
         &self.table_name
     }
 
-    pub fn column_type(&self, column_position: ColumnPosition) -> DbType {
+    pub fn column_type(&self, column: ColumnPosition) -> DbType {
-        self.types[column_position]
+        self.types[column]
     }
 
     pub fn get_columns(&self) -> Vec<&ColumnName> {
@@ -62,8 +62,8 @@ impl TableSchema {
         self.types.get(position).copied()
     }
 
-    pub fn is_primary(&self, column_position: ColumnPosition) -> bool {
+    pub fn is_primary(&self, column: ColumnPosition) -> bool {
-        self.primary_key == column_position
+        self.primary_key == column
     }
 
     // Assumes `column` comes from a validated source.

parser/src/validation.rs

@@ -479,13 +479,39 @@ mod tests {
         let result = validate_operation(syntax, &db_schema);
         assert!(matches!(result, Ok(Operation::Delete(_, Some(operation::Condition::Eq(_, _))))));
 
-        let Ok(Operation::Delete(table_position, Some(operation::Condition::Eq(column_position, value)))) = result else { panic!() };
+        let Ok(Operation::Delete(table_position, Some(operation::Condition::Eq(column, value)))) = result else { panic!() };
 
         assert!(table_position == users_position);
-        assert!(column_position == age);
+        assert!(column == age);
         assert!(value == Indexable(Int(25)));
-        // assert!(condition == None);
     }
 
     // ====CreateIndex====
+    #[test]
+    fn test_create_index() {
+        let users_schema: TableSchema = users_schema();
+        let db_schema: DbSchema = db_schema(&users_schema);
+
+        let users_position = 0;
+        let age = 2;
+
+        let syntax: RawQuerySyntax = CreateIndex("users".to_string(), "age".to_string());
+        let result = validate_operation(syntax, &db_schema);
+        assert!(matches!(result, Ok(Operation::CreateIndex(_, _))));
+
+        let Ok(Operation::CreateIndex(table_position, column)) = result else { panic!() };
+
+        assert!(table_position == users_position);
+        assert!(column == age);
+    }
+
+    #[test]
+    fn test_create_index_nonexistent_column() {
+        let users_schema: TableSchema = users_schema();
+        let db_schema: DbSchema = db_schema(&users_schema);
+
+        let syntax: RawQuerySyntax = CreateIndex("users".to_string(), "does_not_exist".to_string());
+        let result = validate_operation(syntax, &db_schema);
+        assert!(matches!(result, Err(ValidationError::ColumnsDoNotExist(_))));
+    }
 }