From f480b88b4818dcf5b1930fb879dfc8f7d8b7332f Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 3 Jun 2026 00:20:29 +0800 Subject: [PATCH] Support alter table change column options --- src/binder/alter_table.rs | 59 +++++++++++++++++----- src/execution/ddl/add_column.rs | 1 - src/execution/ddl/change_column.rs | 2 - src/execution/ddl/drop_column.rs | 1 - src/execution/ddl/mod.rs | 7 +-- tests/slt/alter_table.slt | 56 +++++++++++++++++++++ tests/slt/change_column.slt | 80 ++++++++++++++++++++++++++++++ 7 files changed, 183 insertions(+), 23 deletions(-) diff --git a/src/binder/alter_table.rs b/src/binder/alter_table.rs index b94cfd0b..93b9bc2e 100644 --- a/src/binder/alter_table.rs +++ b/src/binder/alter_table.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sqlparser::ast::{AlterColumnOperation, AlterTableOperation, ObjectName}; +use sqlparser::ast::{AlterColumnOperation, AlterTableOperation, ColumnOption, ObjectName}; use std::borrow::Cow; use std::sync::Arc; @@ -27,7 +27,8 @@ use crate::planner::operator::alter_table::change_column::{ }; use crate::planner::operator::alter_table::drop_column::DropColumnOperator; use crate::planner::operator::Operator; -use crate::planner::{Childrens, LogicalPlan}; +use crate::planner::Childrens; +use crate::planner::LogicalPlan; use crate::storage::Transaction; use crate::types::value::DataValue; use crate::types::LogicalType; @@ -50,6 +51,33 @@ impl> Binder<'_, '_, T, A> Ok(expr) } + fn bind_change_column_options( + &mut self, + options: &[ColumnOption], + data_type: &LogicalType, + ) -> Result<(DefaultChange, NotNullChange), DatabaseError> { + let mut default_change = DefaultChange::NoChange; + let mut not_null_change = NotNullChange::NoChange; + + for option in options { + match option { + ColumnOption::Null => not_null_change = NotNullChange::Drop, + ColumnOption::NotNull => not_null_change = NotNullChange::Set, + ColumnOption::Default(expr) => { + default_change = + DefaultChange::Set(self.bind_alter_default_expr(expr, data_type)?); + } + option => { + return Err(DatabaseError::UnsupportedStmt(format!( + "CHANGE/MODIFY COLUMN does not currently support this option: {option:?}" + ))) + } + } + } + + Ok((default_change, not_null_change)) + } + pub(crate) fn bind_alter_table( &mut self, name: &ObjectName, @@ -200,24 +228,27 @@ impl> Binder<'_, '_, T, A> options, column_position, } => { - if !options.is_empty() || column_position.is_some() { + if column_position.is_some() { return Err(DatabaseError::UnsupportedStmt( - "MODIFY COLUMN currently only supports changing the data type".to_string(), + "MODIFY COLUMN does not currently support column positions".to_string(), )); } let old_column_name = col_name.value.to_lowercase(); let _ = table .get_column_by_name(&old_column_name) .ok_or_else(|| DatabaseError::column_not_found(old_column_name.clone()))?; + let data_type = LogicalType::try_from(data_type.clone())?; + let (default_change, not_null_change) = + self.bind_change_column_options(options, &data_type)?; LogicalPlan::new( Operator::ChangeColumn(ChangeColumnOperator { table_name, new_column_name: old_column_name.clone(), old_column_name, - data_type: LogicalType::try_from(data_type.clone())?, - default_change: DefaultChange::NoChange, - not_null_change: NotNullChange::NoChange, + data_type, + default_change, + not_null_change, }), Childrens::None, ) @@ -229,10 +260,9 @@ impl> Binder<'_, '_, T, A> options, column_position, } => { - if !options.is_empty() || column_position.is_some() { + if column_position.is_some() { return Err(DatabaseError::UnsupportedStmt( - "CHANGE COLUMN currently only supports renaming and changing the data type" - .to_string(), + "CHANGE COLUMN does not currently support column positions".to_string(), )); } let old_column_name = old_name.value.to_lowercase(); @@ -246,15 +276,18 @@ impl> Binder<'_, '_, T, A> "illegal column naming".to_string(), )); } + let data_type = LogicalType::try_from(data_type.clone())?; + let (default_change, not_null_change) = + self.bind_change_column_options(options, &data_type)?; LogicalPlan::new( Operator::ChangeColumn(ChangeColumnOperator { table_name, old_column_name, new_column_name, - data_type: LogicalType::try_from(data_type.clone())?, - default_change: DefaultChange::NoChange, - not_null_change: NotNullChange::NoChange, + data_type, + default_change, + not_null_change, }), Childrens::None, ) diff --git a/src/execution/ddl/add_column.rs b/src/execution/ddl/add_column.rs index 0d40bc7f..59276d4f 100644 --- a/src/execution/ddl/add_column.rs +++ b/src/execution/ddl/add_column.rs @@ -107,7 +107,6 @@ impl AddColumn { &pk_ty, &old_deserializers, schema.len(), - schema.len(), &serializers, |tuple| { if let Some(value) = &default_value { diff --git a/src/execution/ddl/change_column.rs b/src/execution/ddl/change_column.rs index bc5301eb..66f43ced 100644 --- a/src/execution/ddl/change_column.rs +++ b/src/execution/ddl/change_column.rs @@ -116,7 +116,6 @@ impl ChangeColumn { &pk_ty, &old_deserializers, schema.len(), - schema.len(), &serializers, |tuple| { tuple.values[column_index] = @@ -136,7 +135,6 @@ impl ChangeColumn { &pk_ty, &old_deserializers, schema.len(), - schema.len(), |tuple| { if tuple.values[column_index].is_null() { return Err(DatabaseError::not_null_column(target_column_name.clone())); diff --git a/src/execution/ddl/drop_column.rs b/src/execution/ddl/drop_column.rs index 42a45762..b03aa858 100644 --- a/src/execution/ddl/drop_column.rs +++ b/src/execution/ddl/drop_column.rs @@ -92,7 +92,6 @@ impl DropColumn { &pk_ty, &old_deserializers, tuple_columns.len(), - tuple_columns.len(), &serializers, |tuple| { let _ = tuple.values.remove(column_index); diff --git a/src/execution/ddl/mod.rs b/src/execution/ddl/mod.rs index 9f209b01..f2423d8b 100644 --- a/src/execution/ddl/mod.rs +++ b/src/execution/ddl/mod.rs @@ -41,7 +41,6 @@ fn read_tuple_batch( pk_ty: &LogicalType, old_deserializers: &[TupleValueSerializableImpl], old_values_len: usize, - old_total_len: usize, start_after: Option<&TupleId>, batch: &mut Vec, batch_size: usize, @@ -85,7 +84,7 @@ fn read_tuple_batch( old_deserializers, Some(tuple_id), value, - old_total_len, + old_values_len, )?; len += 1; } @@ -100,7 +99,6 @@ pub(crate) fn visit_table_in_batches( pk_ty: &LogicalType, old_deserializers: &[TupleValueSerializableImpl], old_values_len: usize, - old_total_len: usize, mut visit: F, ) -> Result<(), DatabaseError> where @@ -117,7 +115,6 @@ where pk_ty, old_deserializers, old_values_len, - old_total_len, last_pk.as_ref(), &mut batch, REWRITE_BATCH_SIZE, @@ -146,7 +143,6 @@ pub(crate) fn rewrite_table_in_batches( pk_ty: &LogicalType, old_deserializers: &[TupleValueSerializableImpl], old_values_len: usize, - old_total_len: usize, new_serializers: &[TupleValueSerializableImpl], mut rewrite: F, mut after_write: G, @@ -166,7 +162,6 @@ where pk_ty, old_deserializers, old_values_len, - old_total_len, last_pk.as_ref(), &mut batch, REWRITE_BATCH_SIZE, diff --git a/tests/slt/alter_table.slt b/tests/slt/alter_table.slt index 898755b0..62f03c8e 100644 --- a/tests/slt/alter_table.slt +++ b/tests/slt/alter_table.slt @@ -62,3 +62,59 @@ drop table t1 statement ok drop table t2 + +statement ok +create table t3(id int primary key, v int) + +statement ok +insert into t3 values (1, 10), (2, 20) + +query TTTTI +describe t3 +---- +id Integer 4 false PRIMARY null +v Integer 4 true EMPTY null + +statement ok +insert into t3 values (3, 10) + +statement error +alter table t3 alter column v type bigint using v + 1 + +statement error +alter table t3 rename to t3_new + +statement error +alter table t3 drop column id, v + +statement ok +insert into t3 values (4, 10) + +statement error +alter table t3 modify column v int unique + +statement error +alter table t3 change column v v2 int unique + +query III rowsort +select * from t3 +---- +1 10 +2 20 +3 10 +4 10 + +query TTTTI +describe t3 +---- +id Integer 4 false PRIMARY null +v Integer 4 true EMPTY null + +statement error +alter table t3 modify column v int first + +statement error +alter table t3 change column v v int after id + +statement ok +drop table t3 diff --git a/tests/slt/change_column.slt b/tests/slt/change_column.slt index 7c82cbe4..7b7c8269 100644 --- a/tests/slt/change_column.slt +++ b/tests/slt/change_column.slt @@ -115,5 +115,85 @@ select value2 from alter_users where value2 is not null order by id 999 404 +statement ok +update alter_users set value2 = 606 where id = 6 + +statement ok +alter table alter_users change column value2 value2 int not null + +query TTTTI +describe alter_users +---- +id Integer 4 false PRIMARY null +value1 Integer 4 true UNIQUE null +value2 Integer 4 false EMPTY null + +statement error +insert into alter_users (id, value1) values (7, 77) + +statement ok +alter table alter_users change column value2 value2 int default 707 + +statement ok +insert into alter_users (id, value1) values (7, 77) + +query I +select value2 from alter_users where id = 7 +---- +707 + +query TTTTI +describe alter_users +---- +id Integer 4 false PRIMARY null +value1 Integer 4 true UNIQUE null +value2 Integer 4 false EMPTY 707 + +statement ok +alter table alter_users modify column value2 int null + +statement ok +insert into alter_users (id, value1) values (8, 88) + +query I +select value2 from alter_users where id = 8 +---- +707 + +query TTTTI +describe alter_users +---- +id Integer 4 false PRIMARY null +value1 Integer 4 true UNIQUE null +value2 Integer 4 true EMPTY 707 + +statement ok +alter table alter_users modify column value2 int not null + +query TTTTI +describe alter_users +---- +id Integer 4 false PRIMARY null +value1 Integer 4 true UNIQUE null +value2 Integer 4 false EMPTY 707 + +statement ok +alter table alter_users modify column value2 int default 808 + +statement ok +insert into alter_users (id, value1) values (9, 99) + +query I +select value2 from alter_users where id = 9 +---- +808 + +query TTTTI +describe alter_users +---- +id Integer 4 false PRIMARY null +value1 Integer 4 true UNIQUE null +value2 Integer 4 false EMPTY 808 + statement ok drop table alter_users