Skip to content

Commit 8e4c296

Browse files
feat: add set_nullable support for ALTER TABLE (delta-io#2388)
## What changes are proposed in this pull request? Adds the `SetNullable` operation to the ALTER TABLE framework, allowing columns to be changed from NOT NULL to nullable. Supports nested columns via `ColumnName` paths. `modify_field_at_path` function to recurse through struct and rebuild bottom-up to update nullable field. ## How was this change tested? - Unit tests for `modify_field_at_path`: top-level, nested, sibling preservation, nonexistent field, non-struct traversal, case-insensitive lookup - Unit tests for `SetNullable` via `apply_schema_operations`: required->nullable, already nullable noop, deeply nested (3 levels), chained with add_column - Integration tests in `kernel/tests/alter_table.rs`: set_nullable with schema reload, already-nullable noop, nonexistent column error, chained add_column + set_nullable
1 parent 7ced14e commit 8e4c296

4 files changed

Lines changed: 564 additions & 15 deletions

File tree

kernel/src/schema/mod.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,56 @@ impl StructType {
817817
self.fields.into_values()
818818
}
819819

820+
/// Gets a mutable reference to the underlying field map.
821+
pub(crate) fn field_map_mut(&mut self) -> &mut IndexMap<String, StructField> {
822+
&mut self.fields
823+
}
824+
825+
/// Walk a pre-segmented column path through this schema and return the leaf field.
826+
///
827+
/// `path` is the path's individual name segments (one per nesting level), already split by
828+
/// the caller. Lookup at each level is case-insensitive. The Delta protocol uses `.` as the
829+
/// dotted-path separator at the API surface, but `field_at_path` itself does not split --
830+
/// callers typically pass [`ColumnName::path()`](crate::expressions::ColumnName::path),
831+
/// which yields the segments directly.
832+
///
833+
/// Panics if any segment is missing or an intermediate field is not a struct. Intended for
834+
/// use in test assertions.
835+
///
836+
/// # Example
837+
///
838+
/// ```ignore
839+
/// // Schema:
840+
/// // id: INTEGER not null
841+
/// // address: STRUCT { city: STRING not null, zip: STRING }
842+
/// let path = vec!["address".to_string(), "city".to_string()];
843+
/// let city = schema.field_at_path(&path);
844+
/// assert_eq!(city.name(), "city");
845+
/// assert!(!city.is_nullable());
846+
/// ```
847+
#[cfg(any(test, feature = "test-utils"))]
848+
#[allow(clippy::panic, clippy::expect_used)]
849+
pub fn field_at_path<'a>(&'a self, path: &[String]) -> &'a StructField {
850+
fn find_ci<'a>(
851+
mut fields: impl Iterator<Item = &'a StructField>,
852+
name: &str,
853+
) -> &'a StructField {
854+
let lowered = name.to_lowercase();
855+
fields
856+
.find(|f| f.name().to_lowercase() == lowered)
857+
.unwrap_or_else(|| panic!("field '{name}' not found"))
858+
}
859+
let (first, rest) = path.split_first().expect("non-empty path");
860+
let mut field = find_ci(self.fields(), first);
861+
for seg in rest {
862+
let DataType::Struct(s) = field.data_type() else {
863+
panic!("expected struct at intermediate segment '{seg}'");
864+
};
865+
field = find_ci(s.fields(), seg);
866+
}
867+
field
868+
}
869+
820870
/// Gets all the field names in this struct type in the order they are defined.
821871
pub fn field_names(&self) -> impl ExactSizeIterator<Item = &String> {
822872
self.fields.keys()

kernel/src/transaction/builder/alter_table.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use std::marker::PhantomData;
2828
use std::sync::Arc;
2929

3030
use crate::committer::Committer;
31+
use crate::expressions::ColumnName;
3132
use crate::schema::StructField;
3233
use crate::snapshot::SnapshotRef;
3334
use crate::table_configuration::TableConfiguration;
@@ -112,6 +113,16 @@ impl<S: Chainable> AlterTableTransactionBuilder<S> {
112113
self.operations.push(SchemaOperation::AddColumn { field });
113114
self.transition()
114115
}
116+
117+
/// Change a column's nullability from NOT NULL to nullable. If the column is already
118+
/// nullable, the op is a no-op but still generates a commit.
119+
///
120+
/// Note: this matches Spark's behavior.
121+
pub fn set_nullable(mut self, column: ColumnName) -> AlterTableTransactionBuilder<Modifying> {
122+
self.operations
123+
.push(SchemaOperation::SetNullable { column });
124+
self.transition()
125+
}
115126
}
116127

117128
impl AlterTableTransactionBuilder<Modifying> {

0 commit comments

Comments
 (0)