Support CHANGE/MODIFY COLUMN options

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<T: Transaction, A: AsRef<[(&'static str, DataValue)]>> 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<T: Transaction, A: AsRef<[(&'static str, DataValue)]>> 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<T: Transaction, A: AsRef<[(&'static str, DataValue)]>> 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<T: Transaction, A: AsRef<[(&'static str, DataValue)]>> 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,
                 )

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 {

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()));

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);

src/execution/ddl/mod.rs

@@ -41,7 +41,6 @@ fn read_tuple_batch<T: Transaction>(
     pk_ty: &LogicalType,
     old_deserializers: &[TupleValueSerializableImpl],
     old_values_len: usize,
-    old_total_len: usize,
     start_after: Option<&TupleId>,
     batch: &mut Vec<Tuple>,
     batch_size: usize,
@@ -85,7 +84,7 @@ fn read_tuple_batch<T: Transaction>(
                 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<T, F>(
     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<T, F, G>(
     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,

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

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