Skip to content

Commit d596cb0

Browse files
committed
fix: Address CodeRabbit feedback
- Add value assertions in test_parse_update_multiple_columns - Add bounds check for row.values access with proper error handling - Add error path tests for type mismatch, duplicate column, and unknown column - Prevent potential panic from corrupted row data
1 parent 19543f2 commit d596cb0

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

nexum_core/src/executor/mod.rs

Lines changed: 133 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,17 @@ impl Executor {
315315
};
316316

317317
if should_update {
318-
// Apply assignments to the row
318+
// Apply assignments to the row with bounds checking
319319
for (col_idx, new_value) in &assignment_indices {
320-
row.values[*col_idx] = new_value.clone();
320+
if let Some(value) = row.values.get_mut(*col_idx) {
321+
*value = new_value.clone();
322+
} else {
323+
return Err(StorageError::ReadError(format!(
324+
"Row data corrupted: column index {} out of bounds (row has {} values)",
325+
col_idx,
326+
row.values.len()
327+
)));
328+
}
321329
}
322330
updates.push((key.clone(), row));
323331
}
@@ -878,4 +886,127 @@ mod tests {
878886
_ => panic!("Expected Selected result"),
879887
}
880888
}
889+
890+
#[test]
891+
fn test_update_type_mismatch_error() {
892+
let storage = StorageEngine::memory().unwrap();
893+
let executor = Executor::new(storage);
894+
895+
// Create table with integer column
896+
let create = Statement::CreateTable {
897+
name: "test_type_error".to_string(),
898+
columns: vec![
899+
Column {
900+
name: "id".to_string(),
901+
data_type: DataType::Integer,
902+
},
903+
Column {
904+
name: "count".to_string(),
905+
data_type: DataType::Integer,
906+
},
907+
],
908+
};
909+
executor.execute(create).unwrap();
910+
911+
// Insert a row
912+
let insert = Statement::Insert {
913+
table: "test_type_error".to_string(),
914+
columns: vec!["id".to_string(), "count".to_string()],
915+
values: vec![vec![Value::Integer(1), Value::Integer(10)]],
916+
};
917+
executor.execute(insert).unwrap();
918+
919+
// Try to update integer column with text value - should fail
920+
let update = Statement::Update {
921+
table: "test_type_error".to_string(),
922+
assignments: vec![("count".to_string(), Value::Text("not a number".to_string()))],
923+
where_clause: None,
924+
};
925+
926+
let result = executor.execute(update);
927+
assert!(result.is_err());
928+
let err_msg = result.unwrap_err().to_string();
929+
assert!(err_msg.contains("Type mismatch"));
930+
}
931+
932+
#[test]
933+
fn test_update_duplicate_column_error() {
934+
let storage = StorageEngine::memory().unwrap();
935+
let executor = Executor::new(storage);
936+
937+
// Create table
938+
let create = Statement::CreateTable {
939+
name: "test_dup_col".to_string(),
940+
columns: vec![
941+
Column {
942+
name: "id".to_string(),
943+
data_type: DataType::Integer,
944+
},
945+
Column {
946+
name: "name".to_string(),
947+
data_type: DataType::Text,
948+
},
949+
],
950+
};
951+
executor.execute(create).unwrap();
952+
953+
// Insert a row
954+
let insert = Statement::Insert {
955+
table: "test_dup_col".to_string(),
956+
columns: vec!["id".to_string(), "name".to_string()],
957+
values: vec![vec![Value::Integer(1), Value::Text("Alice".to_string())]],
958+
};
959+
executor.execute(insert).unwrap();
960+
961+
// Try to update same column twice - should fail
962+
let update = Statement::Update {
963+
table: "test_dup_col".to_string(),
964+
assignments: vec![
965+
("name".to_string(), Value::Text("Bob".to_string())),
966+
("name".to_string(), Value::Text("Charlie".to_string())),
967+
],
968+
where_clause: None,
969+
};
970+
971+
let result = executor.execute(update);
972+
assert!(result.is_err());
973+
let err_msg = result.unwrap_err().to_string();
974+
assert!(err_msg.contains("Duplicate column assignment"));
975+
}
976+
977+
#[test]
978+
fn test_update_unknown_column_error() {
979+
let storage = StorageEngine::memory().unwrap();
980+
let executor = Executor::new(storage);
981+
982+
// Create table
983+
let create = Statement::CreateTable {
984+
name: "test_unknown_col".to_string(),
985+
columns: vec![Column {
986+
name: "id".to_string(),
987+
data_type: DataType::Integer,
988+
}],
989+
};
990+
executor.execute(create).unwrap();
991+
992+
// Insert a row
993+
let insert = Statement::Insert {
994+
table: "test_unknown_col".to_string(),
995+
columns: vec!["id".to_string()],
996+
values: vec![vec![Value::Integer(1)]],
997+
};
998+
executor.execute(insert).unwrap();
999+
1000+
// Try to update non-existent column - should fail
1001+
let update = Statement::Update {
1002+
table: "test_unknown_col".to_string(),
1003+
assignments: vec![("nonexistent".to_string(), Value::Integer(42))],
1004+
where_clause: None,
1005+
};
1006+
1007+
let result = executor.execute(update);
1008+
assert!(result.is_err());
1009+
let err_msg = result.unwrap_err().to_string();
1010+
assert!(err_msg.contains("not found"));
1011+
}
8811012
}

nexum_core/src/sql/parser.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,9 @@ mod tests {
316316
assert_eq!(table, "users");
317317
assert_eq!(assignments.len(), 2);
318318
assert_eq!(assignments[0].0, "name");
319+
assert_eq!(assignments[0].1, Value::Text("Bob".to_string()));
319320
assert_eq!(assignments[1].0, "age");
321+
assert_eq!(assignments[1].1, Value::Integer(30));
320322
assert!(where_clause.is_some());
321323
}
322324
_ => panic!("Expected Update statement"),

0 commit comments

Comments
 (0)