diff --git a/executor/src/lib.rs b/executor/src/lib.rs index abc11aa..2f655be 100644 --- a/executor/src/lib.rs +++ b/executor/src/lib.rs @@ -2816,6 +2816,83 @@ mod tests { assert!(!rows.iter().any(|r| *r.fields[0].deref() == Value::Int32(2))); } + #[test] + fn test_update_primary_key_column_fails() { + let (executor, _temp_dir) = create_test_executor(); + + // Create and populate table + execute_single( + &executor, + "CREATE TABLE users (id INT32 PRIMARY_KEY, name STRING, age INT32);", + ); + execute_single( + &executor, + "INSERT INTO users (id, name, age) VALUES (1, 'Alice', 25);", + ); + execute_single( + &executor, + "INSERT INTO users (id, name, age) VALUES (2, 'Bob', 30);", + ); + + // Try to update primary key column + let result = execute_single(&executor, "UPDATE users SET id = 999 WHERE id = 1;"); + + assert_parse_error_contains(result, "primary key column 'id' cannot be updated"); + + // Verify no records were updated + let (select_plan, select_ast) = + create_single_statement("SELECT id, name, age FROM users;", &executor); + let result = executor.execute_statement(&select_plan, &select_ast); + + let (_, rows) = expect_select_successful(result); + assert_eq!(rows.len(), 2); + + // Alice should still have id = 1 + let alice = rows + .iter() + .find(|r| *r.fields[0].deref() == Value::Int32(1)) + .unwrap(); + assert_eq!(*alice.fields[1].deref(), Value::String("Alice".into())); + assert_eq!(*alice.fields[2].deref(), Value::Int32(25)); + } + + #[test] + fn test_update_multiple_columns_including_primary_key_fails() { + let (executor, _temp_dir) = create_test_executor(); + + // Create and populate table + execute_single( + &executor, + "CREATE TABLE users (id INT32 PRIMARY_KEY, name STRING, age INT32);", + ); + execute_single( + &executor, + "INSERT INTO users (id, name, age) VALUES (1, 'Alice', 25);", + ); + + // Try to update both primary key and other columns + let result = execute_single( + &executor, + "UPDATE users SET id = 999, name = 'Alicia', age = 26 WHERE id = 1;", + ); + + assert_parse_error_contains(result, "primary key column 'id' cannot be updated"); + + // Verify no records were updated + let (select_plan, select_ast) = + create_single_statement("SELECT id, name, age FROM users;", &executor); + let result = executor.execute_statement(&select_plan, &select_ast); + + let (_, rows) = expect_select_successful(result); + assert_eq!(rows.len(), 1); + + // Alice should remain unchanged + let alice = &rows[0]; + assert_eq!(*alice.fields[0].deref(), Value::Int32(1)); + assert_eq!(*alice.fields[1].deref(), Value::String("Alice".into())); + assert_eq!(*alice.fields[2].deref(), Value::Int32(25)); + } + #[test] fn test_drop_table_empty_table() { let (executor, _temp_dir) = create_test_executor(); diff --git a/executor/src/statement_executor.rs b/executor/src/statement_executor.rs index c0bf856..97c60c6 100644 --- a/executor/src/statement_executor.rs +++ b/executor/src/statement_executor.rs @@ -519,6 +519,7 @@ impl<'e, 'q> StatementExecutor<'e, 'q> { Ok(cm) => cm, Err(e) => return StatementResult::from(&e), }; + let e = ExpressionExecutor::with_single_record(&record_handle.record, self.ast); match e.execute_expression(*expression) { Ok(value) => { diff --git a/planner/src/analyzer.rs b/planner/src/analyzer.rs index 55741a2..613ae07 100644 --- a/planner/src/analyzer.rs +++ b/planner/src/analyzer.rs @@ -65,6 +65,8 @@ pub enum AnalyzerError { ColumnAlreadyExists { column: String }, #[error("column '{column}' cannot be dropped")] ColumnCannotBeDropped { column: String }, + #[error("primary key column '{column}' cannot be updated")] + PrimaryKeyCannotBeUpdated { column: String }, #[error("primary key missing in create table statement")] PrimaryKeyMissing, #[error("unexpected type: expected {expected}, got {got}")] @@ -366,6 +368,17 @@ impl<'a> Analyzer<'a> { .collect::, AnalyzerError>>()?; let (resolved_columns, resolved_values): (Vec<_>, Vec<_>) = resolved_column_setters.into_iter().unzip(); + + // Check if any column being updated is the primary key + for &column in &resolved_columns { + let column_name = self.get_column_name(column)?; + if column_name == primary_key { + return Err(AnalyzerError::PrimaryKeyCannotBeUpdated { + column: column_name, + }); + } + } + let resolved_where_clause = update .where_clause .map(|node_id| self.resolve_expression(node_id)) @@ -3163,6 +3176,112 @@ mod tests { } } + #[test] + fn analyze_update_primary_key_errors() { + let catalog = catalog_with_users(); + let mut ast = Ast::default(); + + // table identifier + let table_ident = ast.add_node(Expression::Identifier(IdentifierNode { + value: "users".into(), + })); + let table_name = ast.add_node(Expression::TableIdentifier(TableIdentifierNode { + identifier: table_ident, + alias: None, + })); + + // column (id - primary key) + let id_ident = ast.add_node(Expression::Identifier(IdentifierNode { + value: "id".into(), + })); + let col_id = ast.add_node(Expression::ColumnIdentifier(ColumnIdentifierNode { + identifier: id_ident, + table_alias: None, + })); + + // value + let val_new = ast.add_node(Expression::Literal(LiteralNode { + value: Literal::Int(999), + })); + + let update = UpdateStatement { + table_name, + column_setters: vec![(col_id, val_new)], + where_clause: None, + }; + ast.add_statement(Statement::Update(update)); + + let analyzer = Analyzer::new(&ast, catalog); + let errs = analyzer.analyze().unwrap_err(); + assert_eq!(errs.len(), 1); + let err = &errs[0]; + match err { + AnalyzerError::PrimaryKeyCannotBeUpdated { column } => { + assert_eq!(column, "id"); + } + other => panic!("expected PrimaryKeyCannotBeUpdated, got: {:?}", other), + } + } + + #[test] + fn analyze_update_multiple_columns_including_primary_key_errors() { + let catalog = catalog_with_users(); + let mut ast = Ast::default(); + + // table identifier + let table_ident = ast.add_node(Expression::Identifier(IdentifierNode { + value: "users".into(), + })); + let table_name = ast.add_node(Expression::TableIdentifier(TableIdentifierNode { + identifier: table_ident, + alias: None, + })); + + // column name (non-primary key) + let name_ident = ast.add_node(Expression::Identifier(IdentifierNode { + value: "name".into(), + })); + let col_name = ast.add_node(Expression::ColumnIdentifier(ColumnIdentifierNode { + identifier: name_ident, + table_alias: None, + })); + + // column id (primary key) + let id_ident = ast.add_node(Expression::Identifier(IdentifierNode { + value: "id".into(), + })); + let col_id = ast.add_node(Expression::ColumnIdentifier(ColumnIdentifierNode { + identifier: id_ident, + table_alias: None, + })); + + // values + let val_name = ast.add_node(Expression::Literal(LiteralNode { + value: Literal::String("John".into()), + })); + let val_id = ast.add_node(Expression::Literal(LiteralNode { + value: Literal::Int(999), + })); + + let update = UpdateStatement { + table_name, + column_setters: vec![(col_name, val_name), (col_id, val_id)], + where_clause: None, + }; + ast.add_statement(Statement::Update(update)); + + let analyzer = Analyzer::new(&ast, catalog); + let errs = analyzer.analyze().unwrap_err(); + assert_eq!(errs.len(), 1); + let err = &errs[0]; + match err { + AnalyzerError::PrimaryKeyCannotBeUpdated { column } => { + assert_eq!(column, "id"); + } + other => panic!("expected PrimaryKeyCannotBeUpdated, got: {:?}", other), + } + } + #[test] fn analyze_delete_with_where_clause() { let catalog = catalog_with_users();