Add validation to prevent ALTER COLUMN NOT NULL with NULL values

Co-authored-by: BohuTANG <[email protected]>

Author: Copilot

src/query/service/src/interpreters/interpreter_table_modify_column.rs

@@ -162,6 +162,33 @@ impl ModifyTableColumnInterpreter {
         let schema = table.schema().as_ref().clone();
         let table_info = table.get_table_info();
         let mut new_schema = schema.clone();
+        
+        // Validate that columns being changed to NOT NULL don't contain NULL values
+        let fuse_table = FuseTable::try_from_table(table.as_ref())?;
+        let base_snapshot = fuse_table.read_table_snapshot().await?;
+        if let Some(snapshot) = base_snapshot.as_ref() {
+            for (field, _comment) in field_and_comments {
+                if let Some((_, old_field)) = schema.column_with_name(&field.name) {
+                    // Check if we're changing from nullable to NOT NULL
+                    let is_removing_nullable = 
+                        old_field.data_type.is_nullable() && !field.data_type.is_nullable();
+                    
+                    if is_removing_nullable {
+                        // Check if column contains NULL values by examining statistics
+                        let column_id = old_field.column_id;
+                        if let Some(col_stats) = snapshot.summary.col_stats.get(&column_id) {
+                            if col_stats.null_count > 0 {
+                                return Err(ErrorCode::BadArguments(format!(
+                                    "Cannot change column '{}' to NOT NULL: column contains {} NULL values",
+                                    field.name, col_stats.null_count
+                                )));
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        
         // first check default expr before lock table
         for (field, _comment) in field_and_comments {
             if let Some((i, old_field)) = schema.column_with_name(&field.name) {
@@ -455,12 +482,10 @@ impl ModifyTableColumnInterpreter {
                         (_, _) => {
                             if need_remove_nullable {
                                 // If the column is being changed from NULLABLE to NOT NULL,
-                                // wrap it with `coalesce()` to replace NULL values with the default,
-                                // and `remove_nullable()` to mark the resulting expression as non-nullable.
-                                format!(
-                                    "remove_nullable(coalesce(`{}`, {}))",
-                                    field.name, default_scalar
-                                )
+                                // use `remove_nullable()` to mark the resulting expression as non-nullable.
+                                // Note: We validate earlier that the column doesn't contain NULL values,
+                                // so we don't need to use coalesce() to replace NULLs.
+                                format!("remove_nullable(`{}`)", field.name)
                             } else {
                                 format!("`{}`", field.name)
                             }

tests/sqllogictests/suites/base/05_ddl/05_0060_ddl_alter_column_not_null.test

@@ -0,0 +1,83 @@
+statement ok
+USE default
+
+statement ok
+DROP TABLE IF EXISTS test_alter_not_null
+
+# Test 1: Cannot alter column to NOT NULL when it contains NULL values
+statement ok
+CREATE OR REPLACE TABLE test_alter_not_null(a STRING NULL, b INT NULL);
+
+statement ok
+INSERT INTO test_alter_not_null VALUES ('a',1), ('b',NULL), (NULL,3), ('d',4);
+
+query IT
+SELECT a, b FROM test_alter_not_null ORDER BY b;
+----
+a 1
+b NULL
+NULL 3
+d 4
+
+# Should fail: column 'a' contains NULL values
+statement error 1065
+ALTER TABLE test_alter_not_null MODIFY COLUMN a STRING NOT NULL;
+
+# Should fail: column 'b' contains NULL values
+statement error 1065
+ALTER TABLE test_alter_not_null MODIFY COLUMN b INT NOT NULL;
+
+statement ok
+DROP TABLE test_alter_not_null
+
+# Test 2: Can alter column to NOT NULL when it doesn't contain NULL values
+statement ok
+CREATE OR REPLACE TABLE test_alter_not_null(a STRING NULL, b INT NULL);
+
+statement ok
+INSERT INTO test_alter_not_null VALUES ('a',1), ('b',2), ('c',3), ('d',4);
+
+query IT
+SELECT a, b FROM test_alter_not_null ORDER BY b;
+----
+a 1
+b 2
+c 3
+d 4
+
+# Should succeed: column 'a' doesn't contain NULL values
+statement ok
+ALTER TABLE test_alter_not_null MODIFY COLUMN a STRING NOT NULL;
+
+# Should succeed: column 'b' doesn't contain NULL values
+statement ok
+ALTER TABLE test_alter_not_null MODIFY COLUMN b INT NOT NULL;
+
+query TT
+DESC test_alter_not_null;
+----
+a VARCHAR NO NULL (empty)
+b INT NO NULL (empty)
+
+statement ok
+DROP TABLE test_alter_not_null
+
+# Test 3: Empty table should allow NOT NULL
+statement ok
+CREATE OR REPLACE TABLE test_alter_not_null(a STRING NULL, b INT NULL);
+
+# Should succeed: table is empty
+statement ok
+ALTER TABLE test_alter_not_null MODIFY COLUMN a STRING NOT NULL;
+
+statement ok
+ALTER TABLE test_alter_not_null MODIFY COLUMN b INT NOT NULL;
+
+query TT
+DESC test_alter_not_null;
+----
+a VARCHAR NO NULL (empty)
+b INT NO NULL (empty)
+
+statement ok
+DROP TABLE test_alter_not_null