From 293865c2d674166dab44b5eb1f4bb655f5aa34b7 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Fri, 29 Aug 2025 14:53:44 -0300 Subject: [PATCH] feat+fix: add tests and restrict altering some constraints --- Makefile | 6 +++++- core/translate/alter.rs | 20 ++++++++++++++++++++ core/vdbe/execute.rs | 36 +++++++++++++++++++++--------------- testing/alter_column.test | 24 ++++++++++++++++++++++++ testing/alter_table.test | 4 ++-- 5 files changed, 72 insertions(+), 18 deletions(-) create mode 100755 testing/alter_column.test diff --git a/Makefile b/Makefile index d2abf3376..3d31faf1d 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,7 @@ uv-sync-test: uv sync --all-extras --dev --package turso_test .PHONE: uv-sync -test: limbo uv-sync-test test-compat test-vector test-sqlite3 test-shell test-memory test-write test-update test-constraint test-collate test-extensions test-mvcc test-matviews +test: limbo uv-sync-test test-compat test-alter-column test-vector test-sqlite3 test-shell test-memory test-write test-update test-constraint test-collate test-extensions test-mvcc test-matviews .PHONY: test test-extensions: limbo uv-sync-test @@ -82,6 +82,10 @@ test-matviews: RUST_LOG=$(RUST_LOG) SQLITE_EXEC=$(SQLITE_EXEC) ./testing/materialized_views.test .PHONY: test-matviews +test-alter-column: + RUST_LOG=$(RUST_LOG) SQLITE_EXEC=$(SQLITE_EXEC) ./testing/alter_column.test +.PHONY: test-alter-column + reset-db: ./scripts/clone_test_db.sh .PHONY: reset-db diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 409941502..3c74ad242 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -365,6 +365,26 @@ pub fn translate_alter_table( ))); }; + if definition + .constraints + .iter() + .any(|c| matches!(c.constraint, ast::ColumnConstraint::PrimaryKey { .. })) + { + return Err(LimboError::ParseError( + "PRIMARY KEY constraint cannot be altered".to_string(), + )); + } + + if definition + .constraints + .iter() + .any(|c| matches!(c.constraint, ast::ColumnConstraint::Unique { .. })) + { + return Err(LimboError::ParseError( + "UNIQUE constraint cannot be altered".to_string(), + )); + } + let sqlite_schema = schema .get_btree_table(SQLITE_TABLEID) .expect("sqlite_schema should be on schema"); diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 3b60b1cf7..5de6f0fb0 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -4851,10 +4851,10 @@ pub fn op_function( match stmt { ast::Stmt::CreateIndex { + tbl_name, unique, if_not_exists, idx_name, - tbl_name, columns, where_clause, } => { @@ -4866,10 +4866,10 @@ pub fn op_function( Some( ast::Stmt::CreateIndex { + tbl_name: ast::Name::new(&rename_to), unique, if_not_exists, idx_name, - tbl_name: ast::Name::new(&rename_to), columns, where_clause, } @@ -4878,9 +4878,9 @@ pub fn op_function( ) } ast::Stmt::CreateTable { + tbl_name, temporary, if_not_exists, - tbl_name, body, } => { let table_name = normalize_ident(tbl_name.name.as_str()); @@ -4891,13 +4891,13 @@ pub fn op_function( Some( ast::Stmt::CreateTable { - temporary, - if_not_exists, tbl_name: ast::QualifiedName { db_name: None, name: ast::Name::new(&rename_to), alias: None, }, + temporary, + if_not_exists, body, } .format() @@ -4927,7 +4927,7 @@ pub fn op_function( let column_def = { match &state.registers[*start_reg + 7].get_value() { - Value::Text(column_def) => normalize_ident(column_def.as_str()), + Value::Text(column_def) => column_def.as_str(), _ => panic!("rename_to parameter should be TEXT"), } }; @@ -4952,11 +4952,11 @@ pub fn op_function( match stmt { ast::Stmt::CreateIndex { + tbl_name, + mut columns, unique, if_not_exists, idx_name, - tbl_name, - mut columns, where_clause, } => { if table != normalize_ident(tbl_name.as_str()) { @@ -4976,11 +4976,11 @@ pub fn op_function( Some( ast::Stmt::CreateIndex { + tbl_name, + columns, unique, if_not_exists, idx_name, - tbl_name, - columns, where_clause, } .format() @@ -4988,10 +4988,10 @@ pub fn op_function( ) } ast::Stmt::CreateTable { - temporary, - if_not_exists, tbl_name, body, + temporary, + if_not_exists, } => { if table != normalize_ident(tbl_name.name.as_str()) { break 'sql None; @@ -5011,18 +5011,24 @@ pub fn op_function( .find(|column| column.col_name == ast::Name::new(&rename_from)) .expect("column being renamed should be present"); - *column = column_def; + match alter_func { + AlterTableFunc::AlterColumn => *column = column_def, + AlterTableFunc::RenameColumn => { + column.col_name = column_def.col_name + } + _ => unreachable!(), + } Some( ast::Stmt::CreateTable { - temporary, - if_not_exists, tbl_name, body: ast::CreateTableBody::ColumnsAndConstraints { columns, constraints, options, }, + temporary, + if_not_exists, } .format() .unwrap(), diff --git a/testing/alter_column.test b/testing/alter_column.test new file mode 100755 index 000000000..3672497ab --- /dev/null +++ b/testing/alter_column.test @@ -0,0 +1,24 @@ +#!/usr/bin/env tclsh + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +do_execsql_test_on_specific_db {:memory:} alter-column-rename-and-type { + CREATE TABLE t (a INTEGER); + CREATE INDEX i ON t (a); + ALTER TABLE t ALTER COLUMN a TO b BLOB; + SELECT sql FROM sqlite_schema; +} { + "CREATE TABLE t (b BLOB)" + "CREATE INDEX i ON t (b)" +} + +do_execsql_test_in_memory_any_error fail-alter-column-primary-key { + CREATE TABLE t (a); + ALTER TABLE t ALTER COLUMN a TO a PRIMARY KEY; +} + +do_execsql_test_in_memory_any_error fail-alter-column-unique { + CREATE TABLE t (a); + ALTER TABLE t ALTER COLUMN a TO a UNIQUE; +} diff --git a/testing/alter_table.test b/testing/alter_table.test index 6f44b9ad1..6c18dff41 100755 --- a/testing/alter_table.test +++ b/testing/alter_table.test @@ -10,12 +10,12 @@ do_execsql_test_on_specific_db {:memory:} alter-table-rename-table { } { "t2" } do_execsql_test_on_specific_db {:memory:} alter-table-rename-column { - CREATE TABLE t (a); + CREATE TABLE t (a INTEGER); CREATE INDEX i ON t (a); ALTER TABLE t RENAME a TO b; SELECT sql FROM sqlite_schema; } { - "CREATE TABLE t (b)" + "CREATE TABLE t (b INTEGER)" "CREATE INDEX i ON t (b)" }