From 164cd169013204550c3cb65ee69e1f6acd455557 Mon Sep 17 00:00:00 2001 From: TwinklerG Date: Fri, 24 Jul 2026 16:02:19 +0800 Subject: [PATCH] refactor: UpdateSchemaAction --- .../loader/tests/schema_update_suite.rs | 77 +- crates/iceberg/public-api.txt | 77 +- crates/iceberg/src/error.rs | 10 + crates/iceberg/src/spec/datatypes.rs | 40 +- crates/iceberg/src/spec/schema/index.rs | 1 + crates/iceberg/src/spec/schema/mod.rs | 2 +- crates/iceberg/src/spec/schema/update.rs | 1968 +++++ crates/iceberg/src/spec/values/primitive.rs | 8 + crates/iceberg/src/test_utils.rs | 84 + crates/iceberg/src/transaction/mod.rs | 15 +- .../iceberg/src/transaction/update_schema.rs | 6946 ++++++++++++++--- 11 files changed, 8216 insertions(+), 1012 deletions(-) create mode 100644 crates/iceberg/src/spec/schema/update.rs diff --git a/crates/catalog/loader/tests/schema_update_suite.rs b/crates/catalog/loader/tests/schema_update_suite.rs index 9421bbf0ee..26e45e04a1 100644 --- a/crates/catalog/loader/tests/schema_update_suite.rs +++ b/crates/catalog/loader/tests/schema_update_suite.rs @@ -25,7 +25,7 @@ use std::collections::HashMap; use common::{CatalogKind, cleanup_namespace_dyn, load_catalog}; use iceberg::spec::{NestedField, PrimitiveType, Schema, StructType, Type}; -use iceberg::transaction::{AddColumn, ApplyTransactionAction, Transaction}; +use iceberg::transaction::{AddColumn, DeleteColumn, Transaction}; use iceberg::{ErrorKind, NamespaceIdent, Result, TableCreation, TableIdent}; use iceberg_test_utils::normalize_test_name_with_parts; use rstest::rstest; @@ -52,6 +52,8 @@ fn base_schema() -> Schema { #[case::memory_catalog(CatalogKind::Memory)] #[tokio::test] async fn test_catalog_schema_add_column(#[case] kind: CatalogKind) -> Result<()> { + use iceberg::transaction::ApplyTransactionAction; + let Some(harness) = load_catalog(kind).await else { return Ok(()); }; @@ -78,11 +80,13 @@ async fn test_catalog_schema_add_column(#[case] kind: CatalogKind) -> Result<()> let tx = Transaction::new(&table); let tx = tx - .update_schema() - .add_column(AddColumn::optional( - "a", - Type::Primitive(PrimitiveType::Int), - )) + .update_schema()? + .add( + AddColumn::builder() + .name("a") + .r#type(PrimitiveType::Int.into()) + .build(), + )? .apply(tx)?; let updated = tx.commit(catalog.as_ref()).await?; @@ -105,6 +109,8 @@ async fn test_catalog_schema_add_column(#[case] kind: CatalogKind) -> Result<()> #[case::memory_catalog(CatalogKind::Memory)] #[tokio::test] async fn test_catalog_schema_add_nested_and_delete_column(#[case] kind: CatalogKind) -> Result<()> { + use iceberg::transaction::ApplyTransactionAction; + let Some(harness) = load_catalog(kind).await else { return Ok(()); }; @@ -135,28 +141,33 @@ async fn test_catalog_schema_add_nested_and_delete_column(#[case] kind: CatalogK // First transaction: add a nested struct column. let tx = Transaction::new(&table); let tx = tx - .update_schema() - .add_column(AddColumn::optional( - "info", - Type::Struct(StructType::new(vec![ - NestedField::optional(0, "city", Type::Primitive(PrimitiveType::String)).into(), - ])), - )) + .update_schema()? + .add( + AddColumn::builder() + .name("info") + .r#type( + StructType::new(vec![ + NestedField::optional(0, "city", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .build(), + )? .apply(tx)?; let table = tx.commit(catalog.as_ref()).await?; // Second transaction: add a sub-field to the nested struct and delete a top-level column. let tx = Transaction::new(&table); let tx = tx - .update_schema() - .add_column( + .update_schema()? + .add( AddColumn::builder() .name("zip") - .field_type(Type::Primitive(PrimitiveType::String)) - .parent("info") + .r#type(PrimitiveType::String.into()) + .parent(Some("info".into())) .build(), - ) - .delete_column("baz") + )? + .delete(DeleteColumn::new("baz"))? .apply(tx)?; let table = tx.commit(catalog.as_ref()).await?; @@ -179,6 +190,8 @@ async fn test_catalog_schema_add_nested_and_delete_column(#[case] kind: CatalogK #[case::memory_catalog(CatalogKind::Memory)] #[tokio::test] async fn test_catalog_schema_delete_invalid_column_errors(#[case] kind: CatalogKind) -> Result<()> { + use iceberg::transaction::ApplyTransactionAction; + let Some(harness) = load_catalog(kind).await else { return Ok(()); }; @@ -208,14 +221,20 @@ async fn test_catalog_schema_delete_invalid_column_errors(#[case] kind: CatalogK // Deleting an identifier field must fail. let tx = Transaction::new(&table); - let tx = tx.update_schema().delete_column("bar").apply(tx)?; + let tx = tx + .update_schema()? + .delete(DeleteColumn::new("bar"))? + .apply(tx)?; let err = tx.commit(catalog.as_ref()).await.unwrap_err(); assert_eq!(err.kind(), ErrorKind::PreconditionFailed); // Deleting a nonexistent field must fail. let tx = Transaction::new(&table); - let tx = tx.update_schema().delete_column("nonexistent").apply(tx)?; - let err = tx.commit(catalog.as_ref()).await.unwrap_err(); + let err = tx + .update_schema()? + .delete(DeleteColumn::new("nonexistent")) + .err() + .unwrap(); assert_eq!(err.kind(), ErrorKind::PreconditionFailed); Ok(()) @@ -233,6 +252,8 @@ async fn test_catalog_schema_delete_invalid_column_errors(#[case] kind: CatalogK async fn test_catalog_schema_update_persisted_after_reload( #[case] kind: CatalogKind, ) -> Result<()> { + use iceberg::transaction::ApplyTransactionAction; + let Some(harness) = load_catalog(kind).await else { return Ok(()); }; @@ -263,11 +284,13 @@ async fn test_catalog_schema_update_persisted_after_reload( let tx = Transaction::new(&table); let tx = tx - .update_schema() - .add_column(AddColumn::optional( - "new_field", - Type::Primitive(PrimitiveType::Long), - )) + .update_schema()? + .add( + AddColumn::builder() + .name("new_field") + .r#type(PrimitiveType::Long.into()) + .build(), + )? .apply(tx)?; tx.commit(catalog.as_ref()).await?; diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt index e6de7ab4d8..3cc7a0d31d 100644 --- a/crates/iceberg/public-api.txt +++ b/crates/iceberg/public-api.txt @@ -1468,6 +1468,8 @@ impl core::cmp::PartialEq for iceberg::spec::Literal pub fn iceberg::spec::Literal::eq(&self, other: &iceberg::spec::Literal) -> bool impl core::convert::From for iceberg::spec::Literal pub fn iceberg::spec::Literal::from(value: iceberg::spec::Datum) -> Self +impl core::convert::From for iceberg::spec::Literal +pub fn iceberg::spec::Literal::from(value: iceberg::spec::PrimitiveLiteral) -> Self impl core::fmt::Debug for iceberg::spec::Literal pub fn iceberg::spec::Literal::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::hash::Hash for iceberg::spec::Literal @@ -1575,6 +1577,8 @@ impl core::cmp::PartialOrd for iceberg::spec::PrimitiveLiteral pub fn iceberg::spec::PrimitiveLiteral::partial_cmp(&self, other: &iceberg::spec::PrimitiveLiteral) -> core::option::Option impl core::convert::From for iceberg::spec::PrimitiveLiteral pub fn iceberg::spec::PrimitiveLiteral::from(value: iceberg::spec::Datum) -> Self +impl core::convert::From for iceberg::spec::Literal +pub fn iceberg::spec::Literal::from(value: iceberg::spec::PrimitiveLiteral) -> Self impl core::fmt::Debug for iceberg::spec::PrimitiveLiteral pub fn iceberg::spec::PrimitiveLiteral::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::hash::Hash for iceberg::spec::PrimitiveLiteral @@ -1724,6 +1728,8 @@ pub fn iceberg::spec::Type::decimal(precision: u32, scale: u32) -> iceberg::Resu pub fn iceberg::spec::Type::decimal_max_precision(num_bytes: u32) -> iceberg::Result pub fn iceberg::spec::Type::decimal_required_bytes(precision: u32) -> iceberg::Result pub fn iceberg::spec::Type::is_floating_type(&self) -> bool +pub fn iceberg::spec::Type::is_list(&self) -> bool +pub fn iceberg::spec::Type::is_map(&self) -> bool pub fn iceberg::spec::Type::is_nested(&self) -> bool pub fn iceberg::spec::Type::is_primitive(&self) -> bool pub fn iceberg::spec::Type::is_struct(&self) -> bool @@ -1966,10 +1972,34 @@ impl serde_core::ser::Serialize for iceberg::spec::FieldSummary pub fn iceberg::spec::FieldSummary::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer impl<'de> serde_core::de::Deserialize<'de> for iceberg::spec::FieldSummary pub fn iceberg::spec::FieldSummary::deserialize<__D>(__deserializer: __D) -> core::result::Result::Error> where __D: serde_core::de::Deserializer<'de> +pub struct iceberg::spec::IndexByName +impl iceberg::spec::IndexByName +pub fn iceberg::spec::IndexByName::indexes(self) -> (std::collections::hash::map::HashMap, std::collections::hash::map::HashMap) +impl core::default::Default for iceberg::spec::IndexByName +pub fn iceberg::spec::IndexByName::default() -> iceberg::spec::IndexByName +impl iceberg::spec::SchemaVisitor for iceberg::spec::IndexByName +pub type iceberg::spec::IndexByName::T = () +pub fn iceberg::spec::IndexByName::after_list_element(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_map_key(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_map_value(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_struct_field(&mut self, _field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_list_element(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_map_key(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_map_value(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_struct_field(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::field(&mut self, field: &iceberg::spec::NestedFieldRef, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::list(&mut self, list: &iceberg::spec::ListType, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::map(&mut self, map: &iceberg::spec::MapType, _key_value: Self::T, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::primitive(&mut self, _p: &iceberg::spec::PrimitiveType) -> iceberg::Result +pub fn iceberg::spec::IndexByName::schema(&mut self, _schema: &iceberg::spec::Schema, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::struct(&mut self, _struct: &iceberg::spec::StructType, _results: alloc::vec::Vec) -> iceberg::Result +pub fn iceberg::spec::IndexByName::variant(&mut self, _v: &iceberg::spec::VariantType) -> iceberg::Result pub struct iceberg::spec::ListType pub iceberg::spec::ListType::element_field: iceberg::spec::NestedFieldRef impl iceberg::spec::ListType pub fn iceberg::spec::ListType::new(element_field: iceberg::spec::NestedFieldRef) -> Self +pub fn iceberg::spec::ListType::optional(element_id: i32, element_type: iceberg::spec::Type) -> Self +pub fn iceberg::spec::ListType::required(element_id: i32, element_type: iceberg::spec::Type) -> Self impl core::clone::Clone for iceberg::spec::ListType pub fn iceberg::spec::ListType::clone(&self) -> iceberg::spec::ListType impl core::cmp::Eq for iceberg::spec::ListType @@ -3038,6 +3068,23 @@ pub fn iceberg::spec::SchemaVisitor::primitive(&mut self, p: &iceberg::spec::Pri pub fn iceberg::spec::SchemaVisitor::schema(&mut self, schema: &iceberg::spec::Schema, value: Self::T) -> iceberg::Result pub fn iceberg::spec::SchemaVisitor::struct(&mut self, struct: &iceberg::spec::StructType, results: alloc::vec::Vec) -> iceberg::Result pub fn iceberg::spec::SchemaVisitor::variant(&mut self, v: &iceberg::spec::VariantType) -> iceberg::Result +impl iceberg::spec::SchemaVisitor for iceberg::spec::IndexByName +pub type iceberg::spec::IndexByName::T = () +pub fn iceberg::spec::IndexByName::after_list_element(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_map_key(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_map_value(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::after_struct_field(&mut self, _field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_list_element(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_map_key(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_map_value(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::before_struct_field(&mut self, field: &iceberg::spec::NestedFieldRef) -> iceberg::Result<()> +pub fn iceberg::spec::IndexByName::field(&mut self, field: &iceberg::spec::NestedFieldRef, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::list(&mut self, list: &iceberg::spec::ListType, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::map(&mut self, map: &iceberg::spec::MapType, _key_value: Self::T, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::primitive(&mut self, _p: &iceberg::spec::PrimitiveType) -> iceberg::Result +pub fn iceberg::spec::IndexByName::schema(&mut self, _schema: &iceberg::spec::Schema, _value: Self::T) -> iceberg::Result +pub fn iceberg::spec::IndexByName::struct(&mut self, _struct: &iceberg::spec::StructType, _results: alloc::vec::Vec) -> iceberg::Result +pub fn iceberg::spec::IndexByName::variant(&mut self, _v: &iceberg::spec::VariantType) -> iceberg::Result pub trait iceberg::spec::SchemaWithPartnerVisitor

pub type iceberg::spec::SchemaWithPartnerVisitor::T pub fn iceberg::spec::SchemaWithPartnerVisitor::after_list_element(&mut self, _field: &iceberg::spec::NestedFieldRef, _partner: &P) -> iceberg::Result<()> @@ -3056,6 +3103,8 @@ pub fn iceberg::spec::SchemaWithPartnerVisitor::schema(&mut self, schema: &icebe pub fn iceberg::spec::SchemaWithPartnerVisitor::struct(&mut self, struct: &iceberg::spec::StructType, partner: &P, results: alloc::vec::Vec) -> iceberg::Result pub fn iceberg::spec::SchemaWithPartnerVisitor::variant(&mut self, v: &iceberg::spec::VariantType, partner: &P) -> iceberg::Result pub fn iceberg::spec::deserialize_data_file_from_json(json: &str, partition_spec_id: i32, partition_type: &iceberg::spec::StructType, schema: &iceberg::spec::Schema) -> iceberg::Result +pub fn iceberg::spec::index_by_id(struct: &iceberg::spec::StructType) -> iceberg::Result> +pub fn iceberg::spec::index_parents(struct: &iceberg::spec::StructType) -> iceberg::Result> pub fn iceberg::spec::prune_columns(schema: &iceberg::spec::Schema, selected: impl core::iter::traits::collect::IntoIterator, select_full_types: bool) -> iceberg::Result pub fn iceberg::spec::read_data_files_from_avro(reader: &mut R, schema: &iceberg::spec::Schema, partition_spec_id: i32, partition_type: &iceberg::spec::StructType, version: iceberg::spec::FormatVersion) -> iceberg::Result> pub fn iceberg::spec::serialize_data_file_to_json(data_file: iceberg::spec::DataFile, partition_type: &iceberg::spec::StructType, format_version: iceberg::spec::FormatVersion) -> iceberg::Result @@ -3124,6 +3173,8 @@ pub fn iceberg::table::TableBuilder::readonly(self, readonly: bool) -> Self pub fn iceberg::table::TableBuilder::runtime(self, runtime: iceberg::Runtime) -> Self pub mod iceberg::test_utils pub fn iceberg::test_utils::check_record_batches(record_batches: alloc::vec::Vec, expected_schema: expect_test::Expect, expected_data: expect_test::Expect, ignore_check_columns: &[&str], sort_column: core::option::Option<&str>) +pub fn iceberg::test_utils::get_projected_ids_of_schema(schema: &iceberg::spec::Schema) -> std::collections::hash::set::HashSet +pub fn iceberg::test_utils::get_projected_ids_of_type(type: &iceberg::spec::Type) -> std::collections::hash::set::HashSet pub fn iceberg::test_utils::test_runtime() -> iceberg::Runtime pub mod iceberg::transaction pub struct iceberg::transaction::ActionCommit @@ -3133,10 +3184,24 @@ pub fn iceberg::transaction::ActionCommit::take_requirements(&mut self) -> alloc pub fn iceberg::transaction::ActionCommit::take_updates(&mut self) -> alloc::vec::Vec pub struct iceberg::transaction::AddColumn impl iceberg::transaction::AddColumn -pub fn iceberg::transaction::AddColumn::optional(name: impl alloc::string::ToString, field_type: iceberg::spec::Type) -> Self -pub fn iceberg::transaction::AddColumn::required(name: impl alloc::string::ToString, field_type: iceberg::spec::Type, initial_default: iceberg::spec::Literal) -> Self +pub fn iceberg::transaction::AddColumn::optional(name: impl core::convert::Into, type: iceberg::spec::Type) -> Self +pub fn iceberg::transaction::AddColumn::parent(self, parent: impl core::convert::Into>) -> Self +pub fn iceberg::transaction::AddColumn::required(name: impl core::convert::Into, type: iceberg::spec::Type) -> Self impl iceberg::transaction::AddColumn -pub fn iceberg::transaction::AddColumn::builder() -> AddColumnBuilder<((), (), (), (), (), (), ())> +pub fn iceberg::transaction::AddColumn::builder() -> AddColumnBuilder<((), (), (), (), (), ())> +pub struct iceberg::transaction::DeleteColumn +impl iceberg::transaction::DeleteColumn +pub fn iceberg::transaction::DeleteColumn::new(name: impl core::convert::Into) -> Self +pub struct iceberg::transaction::MoveColumn +impl iceberg::transaction::MoveColumn +pub fn iceberg::transaction::MoveColumn::after(name: impl core::convert::Into, reference: impl core::convert::Into) -> Self +pub fn iceberg::transaction::MoveColumn::before(name: impl core::convert::Into, reference: impl core::convert::Into) -> Self +pub fn iceberg::transaction::MoveColumn::first(name: impl core::convert::Into) -> Self +pub struct iceberg::transaction::RenameColumn +impl iceberg::transaction::RenameColumn +pub fn iceberg::transaction::RenameColumn::new(name: impl core::convert::Into, new_name: impl core::convert::Into) -> Self +impl iceberg::transaction::RenameColumn +pub fn iceberg::transaction::RenameColumn::builder() -> RenameColumnBuilder<((), ())> pub struct iceberg::transaction::Transaction impl iceberg::transaction::Transaction pub async fn iceberg::transaction::Transaction::commit(self, catalog: &dyn iceberg::Catalog) -> iceberg::Result @@ -3145,12 +3210,15 @@ pub fn iceberg::transaction::Transaction::fast_append(&self) -> iceberg::transac pub fn iceberg::transaction::Transaction::new(table: &iceberg::table::Table) -> Self pub fn iceberg::transaction::Transaction::replace_sort_order(&self) -> iceberg::transaction::sort_order::ReplaceSortOrderAction pub fn iceberg::transaction::Transaction::update_location(&self) -> iceberg::transaction::update_location::UpdateLocationAction -pub fn iceberg::transaction::Transaction::update_schema(&self) -> iceberg::transaction::update_schema::UpdateSchemaAction +pub fn iceberg::transaction::Transaction::update_schema(&self) -> iceberg::Result pub fn iceberg::transaction::Transaction::update_statistics(&self) -> iceberg::transaction::update_statistics::UpdateStatisticsAction pub fn iceberg::transaction::Transaction::update_table_properties(&self) -> iceberg::transaction::update_properties::UpdatePropertiesAction pub fn iceberg::transaction::Transaction::upgrade_table_version(&self) -> iceberg::transaction::upgrade_format_version::UpgradeFormatVersionAction impl core::clone::Clone for iceberg::transaction::Transaction pub fn iceberg::transaction::Transaction::clone(&self) -> iceberg::transaction::Transaction +pub struct iceberg::transaction::UpdateColumn +impl iceberg::transaction::UpdateColumn +pub fn iceberg::transaction::UpdateColumn::builder(name: impl core::convert::Into) -> iceberg::transaction::update_schema::UpdateColumnBuilder pub trait iceberg::transaction::ApplyTransactionAction pub fn iceberg::transaction::ApplyTransactionAction::apply(self, tx: iceberg::transaction::Transaction) -> iceberg::Result impl iceberg::transaction::ApplyTransactionAction for T @@ -3355,6 +3423,7 @@ impl iceberg::writer::IcebergWriterBuilder for iceberg::writer::base_wr pub type iceberg::writer::base_writer::equality_delete_writer::EqualityDeleteFileWriterBuilder::R = iceberg::writer::base_writer::equality_delete_writer::EqualityDeleteFileWriter pub fn iceberg::writer::base_writer::equality_delete_writer::EqualityDeleteFileWriterBuilder::build<'life0, 'async_trait>(&'life0 self, partition_key: core::option::Option) -> core::pin::Pin> + core::marker::Send + 'async_trait)>> where Self: 'async_trait, 'life0: 'async_trait pub macro iceberg::ensure_data_valid! +pub macro iceberg::ensure_precondition! #[non_exhaustive] pub enum iceberg::ErrorKind pub iceberg::ErrorKind::CatalogCommitConflicts pub iceberg::ErrorKind::DataInvalid diff --git a/crates/iceberg/src/error.rs b/crates/iceberg/src/error.rs index 02c3eee8fc..7f2217eccf 100644 --- a/crates/iceberg/src/error.rs +++ b/crates/iceberg/src/error.rs @@ -469,6 +469,16 @@ macro_rules! ensure_data_valid { }; } +/// Helper macro to check preconditions. +#[macro_export] +macro_rules! ensure_precondition { + ($cond: expr, $fmt: literal, $($arg:tt)*) => { + if !$cond { + return Err($crate::error::Error::new($crate::error::ErrorKind::PreconditionFailed, format!($fmt, $($arg)*))) + } + }; +} + #[cfg(test)] mod tests { use anyhow::anyhow; diff --git a/crates/iceberg/src/spec/datatypes.rs b/crates/iceberg/src/spec/datatypes.rs index 79c48c1318..65e1f1bce0 100644 --- a/crates/iceberg/src/spec/datatypes.rs +++ b/crates/iceberg/src/spec/datatypes.rs @@ -119,6 +119,18 @@ impl Type { matches!(self, Type::Struct(_)) } + /// Whether the type is list type. + #[inline(always)] + pub fn is_list(&self) -> bool { + matches!(self, Type::List(_)) + } + + /// Whether the type is map type. + #[inline(always)] + pub fn is_map(&self) -> bool { + matches!(self, Type::Map(_)) + } + /// Whether the type is nested type. #[inline(always)] pub fn is_nested(&self) -> bool { @@ -187,7 +199,7 @@ impl Type { Ok(REQUIRED_LENGTH[precision as usize - 1]) } - /// Creates decimal type. + /// Creates decimal type. #[inline(always)] pub fn decimal(precision: u32, scale: u32) -> Result { ensure_data_valid!( @@ -694,6 +706,18 @@ impl NestedField { self.id = id; self } + + /// Set the type of the field + pub(crate) fn with_type(mut self, field_type: Type) -> Self { + *self.field_type = field_type; + self + } + + /// Set the name of the field. + pub(crate) fn with_name(mut self, name: impl ToString) -> Self { + self.name = name.to_string(); + self + } } impl fmt::Display for NestedField { @@ -726,6 +750,20 @@ impl ListType { pub fn new(element_field: NestedFieldRef) -> Self { Self { element_field } } + + /// Construct an optional list type with the given element field. + pub fn optional(element_id: i32, element_type: Type) -> Self { + Self { + element_field: NestedField::list_element(element_id, element_type, false).into(), + } + } + + /// Construct a required list type with the given element field. + pub fn required(element_id: i32, element_type: Type) -> Self { + Self { + element_field: NestedField::list_element(element_id, element_type, true).into(), + } + } } /// Module for type serialization/deserialization. diff --git a/crates/iceberg/src/spec/schema/index.rs b/crates/iceberg/src/spec/schema/index.rs index e4358e9ef9..b750b24a6e 100644 --- a/crates/iceberg/src/spec/schema/index.rs +++ b/crates/iceberg/src/spec/schema/index.rs @@ -164,6 +164,7 @@ pub fn index_parents(r#struct: &StructType) -> Result> { Ok(index.result) } +/// An index of field names to field ids, and short field names to field ids. #[derive(Default)] pub struct IndexByName { // Maybe radix tree is better here? diff --git a/crates/iceberg/src/spec/schema/mod.rs b/crates/iceberg/src/spec/schema/mod.rs index 652f98b649..ac2a8cd22f 100644 --- a/crates/iceberg/src/spec/schema/mod.rs +++ b/crates/iceberg/src/spec/schema/mod.rs @@ -34,7 +34,7 @@ use serde::{Deserialize, Serialize}; use self::_serde::SchemaEnum; use self::id_reassigner::ReassignFieldIds; -use self::index::{IndexByName, index_by_id, index_parents}; +pub use self::index::{IndexByName, index_by_id, index_parents}; pub use self::prune_columns::prune_columns; use super::NestedField; use crate::error::Result; diff --git a/crates/iceberg/src/spec/schema/update.rs b/crates/iceberg/src/spec/schema/update.rs new file mode 100644 index 0000000000..a333a2b587 --- /dev/null +++ b/crates/iceberg/src/spec/schema/update.rs @@ -0,0 +1,1968 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::collections::HashMap; +use std::sync::Arc; + +use typed_builder::TypedBuilder; + +use crate::spec::schema::index::index_parents; +use crate::spec::{ + ListType, Literal, MapType, NestedField, NestedFieldRef, PrimitiveType, Schema, SchemaRef, + SchemaVisitor, StructType, Type, visit_schema, +}; +use crate::{Error, ErrorKind, Result, ensure_data_valid}; + +const TABLE_ROOT_ID: i32 = -1; + +/// Operations that can be applied to a schema to produce a new schema. These are used in `UpdateSchemaAction` and are not intended to be used directly by end users. Instead, end users should use `UpdateSchema` which will be converted into a list of `SchemaOperation`s. +pub enum SchemaOperation { + /// Add a column to the schema + Add(AddColumn), + /// Update a column's type, doc, or default value + Update(UpdateColumn), + /// Rename a column + Rename(RenameColumn), + /// Delete a column + Delete(DeleteColumn), + /// Move a column + Move(MoveColumn), + /// Allow incompatible changes + AllowIncompatibleChanges, +} + +/// A column to be added to the schema. +#[derive(TypedBuilder)] +pub struct AddColumn { + #[builder(default, setter(strip_option))] + parent: Option, + #[builder(setter(into))] + name: String, + #[builder(default = true)] + is_optional: bool, + r#type: Type, + #[builder(default, setter(strip_option))] + doc: Option, + #[builder(default, setter(strip_option))] + default_value: Option, +} + +/// A column to be deleted from the schema. +pub struct DeleteColumn { + name: String, +} + +impl DeleteColumn { + /// Create a new `DeleteColumn` with the given column name. + pub fn new(name: impl Into) -> Self { + Self { name: name.into() } + } +} + +/// A column to be renamed in the schema. +#[derive(TypedBuilder)] +pub struct RenameColumn { + #[builder(setter(into))] + name: String, + #[builder(setter(into))] + new_name: String, +} + +/// A column to be updated in the schema. +pub struct UpdateColumn { + name: String, + op: UpdateColumnOperation, +} + +impl From for SchemaOperation { + fn from(update: UpdateColumn) -> Self { + SchemaOperation::Update(update) + } +} + +impl UpdateColumn { + /// Create a new `UpdateColumn` to update the column's requiredness. + pub fn new_required(name: impl Into, is_required: bool) -> Self { + Self { + name: name.into(), + op: UpdateColumnOperation::Required(is_required), + } + } + + /// Create a new `UpdateColumn` to update the column's type. + pub fn new_type(name: impl Into, new_type: PrimitiveType) -> Self { + Self { + name: name.into(), + op: UpdateColumnOperation::Type(new_type), + } + } + + /// Create a new `UpdateColumn` to update the column's doc. + pub fn new_doc(name: impl Into, new_doc: Option) -> Self { + Self { + name: name.into(), + op: UpdateColumnOperation::Doc(new_doc), + } + } + + /// Create a new `UpdateColumn` to update the column's default value. + pub fn new_default_value(name: impl Into, new_default_value: Option) -> Self { + Self { + name: name.into(), + op: UpdateColumnOperation::DefaultValue(new_default_value), + } + } +} + +enum UpdateColumnOperation { + Required(bool), + Type(PrimitiveType), + Doc(Option), + DefaultValue(Option), +} + +/// A column to be moved in the schema. +pub struct MoveColumn { + name: String, + reference_name: String, + move_type: MoveType, +} + +impl MoveColumn { + /// Move the column to the first position. + pub fn first(name: impl Into) -> Self { + Self { + name: name.into(), + reference_name: String::new(), + move_type: MoveType::First, + } + } + + /// Move the column before the reference column. + pub fn before(name: impl Into, reference: impl Into) -> Self { + Self { + name: name.into(), + reference_name: reference.into(), + move_type: MoveType::Before, + } + } + + /// Move the column after the reference column. + pub fn after(name: impl Into, reference: impl Into) -> Self { + Self { + name: name.into(), + reference_name: reference.into(), + move_type: MoveType::After, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum MoveType { + First, + Before, + After, +} + +#[derive(Clone, Debug)] +struct Move { + field_id: i32, + reference_field_id: i32, + r#type: MoveType, +} + +impl Move { + fn first(field_id: i32) -> Self { + Move::new(field_id, TABLE_ROOT_ID, MoveType::First) + } + + fn before(field_id: i32, reference_field_id: i32) -> Self { + Move::new(field_id, reference_field_id, MoveType::Before) + } + + fn after(field_id: i32, reference_field_id: i32) -> Self { + Move::new(field_id, reference_field_id, MoveType::After) + } + + fn new(field_id: i32, reference_field_id: i32, r#type: MoveType) -> Self { + Move { + field_id, + reference_field_id, + r#type, + } + } + + fn field_id(&self) -> i32 { + self.field_id + } + + fn reference_field_id(&self) -> i32 { + self.reference_field_id + } + + fn r#type(&self) -> MoveType { + self.r#type + } +} + +/// Applies a list of `SchemaOperation`s to a `Schema` to produce a new `Schema`. This is used in `UpdateSchemaAction` to apply schema changes as part of a transaction commit. This function validates that the schema operations are valid (e.g. that added columns do not have duplicate names, that deleted columns exist, etc.) and returns an error if any invalid operations are found. If all operations are valid, it returns the updated schema. +pub fn schema_update(schema: SchemaRef, operations: &[SchemaOperation]) -> Result { + let mut updates: HashMap = HashMap::new(); + let mut deletes = Vec::new(); + let mut moves: HashMap<_, Vec<_>> = HashMap::new(); + let mut parent_to_added_ids: HashMap<_, Vec<_>> = HashMap::new(); + let mut id_to_parent = index_parents(&schema.r#struct).unwrap(); + let mut last_column_id = schema.highest_field_id; + let mut added_name_to_id = HashMap::new(); + let mut identifier_field_ids = schema.identifier_field_ids.clone(); + let mut allow_incompatible_changes = false; + + for operation in operations { + match operation { + SchemaOperation::Add(add) => { + let (parent, name, is_optional, field_type, doc, default_value) = ( + &add.parent, + &add.name, + add.is_optional, + &add.r#type, + &add.doc, + &add.default_value, + ); + let mut parent_id = TABLE_ROOT_ID; + let full_name = if let Some(parent) = parent { + let parent_field = schema.field_by_name(parent).ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot find parent struct: {}", parent), + ))?; + let parent_field = if parent_field.field_type.is_nested() { + let parent_type = parent_field.field_type.as_ref(); + match parent_type { + Type::List(nested) => nested.element_field.as_ref(), // fields are added to the element type + Type::Map(nested) => nested.value_field.as_ref(), // fields are added to the map value type + _ => parent_field, + } + } else { + parent_field + }; + ensure_data_valid!( + parent_field.field_type.is_struct(), + "Cannot add to non-struct column: {}: {}", + &parent, + parent_field.field_type + ); + parent_id = parent_field.id; + let full_name = format!("{}.{}", parent, name); + let current_field = schema.field_by_name(&full_name); + ensure_data_valid!( + !deletes.contains(&parent_id), + "Can not add a column that will be deleted: {}", + name + ); + ensure_data_valid!( + current_field.is_none() || deletes.contains(¤t_field.unwrap().id), + "Cannot add column, name already exists: {}", + &name + ); + full_name + } else { + let current_field = schema.field_by_name(name); + ensure_data_valid!( + current_field.is_none() || deletes.contains(¤t_field.unwrap().id), + "Cannot add column, name already exists: {}", + &name + ); + name.clone() + }; + ensure_precondition!( + default_value.is_some() || is_optional || allow_incompatible_changes, + "Incompatible change: cannot add required column without a default value: {}", + full_name + ); + last_column_id += 1; + let new_id = last_column_id; + added_name_to_id.insert(full_name, new_id); + + if parent_id != TABLE_ROOT_ID { + id_to_parent.insert(new_id, parent_id); + } + // TODO: Maybe we can use `ReassignFieldIds`? + let assigned_type = assign_fresh_ids(field_type.clone(), &mut last_column_id); + let mut new_field = NestedField::new(new_id, name, assigned_type, !is_optional); + new_field.doc = doc.clone(); + new_field.write_default = default_value.clone(); + new_field.initial_default = default_value.clone(); + updates.insert(new_id, new_field.into()); + parent_to_added_ids + .entry(parent_id) + .or_default() + .push(new_id); + } + SchemaOperation::Delete(delete) => { + let field = schema.field_by_name(&delete.name).ok_or_else(|| { + Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete missing column: {}", delete.name), + ) + })?; + ensure_data_valid!( + !parent_to_added_ids.contains_key(&field.id), + "Cannot delete a column that has updates: {}", + delete.name + ); + ensure_data_valid!( + !updates.contains_key(&field.id), + "Cannot delete a column that has updates: {}", + delete.name + ); + deletes.push(field.id); + } + SchemaOperation::Rename(rename) => { + let (name, new_name) = (&rename.name, &rename.new_name); + let field = schema.field_by_name(name).ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot rename missing column: {}", name), + ))?; + ensure_data_valid!( + !deletes.contains(&field.id), + "Cannot rename a column that will be deleted: {}", + name + ); + // merge with an update, if present + let field_id = field.id; + let update = updates.get(&field_id); + let new_field = if let Some(update) = update { + Arc::unwrap_or_clone(update.clone()).with_name(new_name) + } else { + Arc::unwrap_or_clone(field.clone()).with_name(new_name) + }; + updates.insert(field_id, Arc::new(new_field)); + if identifier_field_ids.contains(&field_id) { + identifier_field_ids.remove(&field_id); + identifier_field_ids.insert(field_id); + } + } + SchemaOperation::Update(update) => { + let (name, op) = (&update.name, &update.op); + let field = find_for_update(name, schema.clone(), &updates, &added_name_to_id)? + .ok_or(Error::new( + ErrorKind::DataInvalid, + format!("Cannot update missing column: {}", name), + ))?; + ensure_data_valid!( + !deletes.contains(&field.id), + "Cannot update column that will be deleted: {}", + name, + ); + let mut new_field = Arc::unwrap_or_clone(field.clone()); + match op { + UpdateColumnOperation::Required(new_required) => { + if (*new_required && !field.required) || (!*new_required && field.required) + { + let is_default_add = added_name_to_id.contains_key(name) + && field.initial_default.is_some(); + ensure_precondition!( + !*new_required || is_default_add || allow_incompatible_changes, + "Cannot change column nullability: {}: optional -> required", + name + ); + new_field.required = *new_required; + } + } + UpdateColumnOperation::Type(new_type) => { + ensure_precondition!( + is_promotion_allowed(field.field_type.as_ref(), new_type), + "Cannot promote {} from type {} to type {}", + name, + field.field_type, + new_type + ); + *new_field.field_type = new_type.clone().into(); + } + UpdateColumnOperation::Doc(new_doc) => { + new_field.doc = new_doc.clone(); + } + UpdateColumnOperation::DefaultValue(new_default_value) => { + new_field.write_default = new_default_value.clone(); + } + } + updates.insert(field.id, Arc::new(new_field)); + } + SchemaOperation::Move(r#move) => { + let (name, reference_name, move_type) = + (&r#move.name, &r#move.reference_name, &r#move.move_type); + let field_id = + find_for_move(name, schema.clone(), &added_name_to_id)?.ok_or(Error::new( + ErrorKind::DataInvalid, + format!("Cannot move missing column: {}", name), + ))?; + let r#move = if move_type == &MoveType::First { + Move::first(field_id) + } else { + let reference_field_id = find_for_move( + reference_name, + schema.clone(), + &added_name_to_id, + )? + .ok_or(Error::new( + ErrorKind::DataInvalid, + format!("Cannot move relative to missing column: {}", reference_name), + ))?; + match move_type { + MoveType::Before => Move::before(field_id, reference_field_id), + MoveType::After => Move::after(field_id, reference_field_id), + _ => unreachable!(), + } + }; + let parent_id = id_to_parent.get(&field_id); + if let Some(&parent_id) = parent_id { + let parent = schema.field_by_id(parent_id).unwrap(); + ensure_data_valid!( + parent.field_type.is_struct(), + "Cannot move field in non-struct type: {}", + parent + ); + if r#move.r#type == MoveType::After || r#move.r#type == MoveType::Before { + ensure_data_valid!( + parent_id == *id_to_parent.get(&r#move.reference_field_id).unwrap(), + "Cannot move field {} to a different struct", + name, + ); + } + moves.entry(parent_id).or_default().push(r#move); + } else { + if move_type == &MoveType::After || move_type == &MoveType::Before { + ensure_data_valid!( + !id_to_parent.contains_key(&r#move.reference_field_id), + "Cannot move field {} to a different struct", + name, + ); + } + moves.entry(TABLE_ROOT_ID).or_default().push(r#move); + } + } + SchemaOperation::AllowIncompatibleChanges => { + allow_incompatible_changes = true; + } + } + } + // apply schema changes + let mut visitor = ApplyChangesVisitor { + deletes, + updates, + parent_to_added_ids, + moves, + }; + let struct_type = visit_schema(schema.as_ref(), &mut visitor)? + .unwrap() + .to_struct_type() + .unwrap(); + // validate identifier requirements based on the latest schema + Ok(Schema::builder() + .with_fields(struct_type.fields().to_vec()) + .with_identifier_field_ids(identifier_field_ids) + .build()? + .into()) +} + +fn is_promotion_allowed(from: &Type, to: &PrimitiveType) -> bool { + let from = match from { + Type::Primitive(p) => p, + _ => return false, + }; + if from == to { + return true; + } + match from { + PrimitiveType::Int => { + matches!(to, PrimitiveType::Long) + } + PrimitiveType::Float => matches!(to, PrimitiveType::Double), + PrimitiveType::Decimal { + precision: p, + scale: s, + } => { + matches!( + to, + PrimitiveType::Decimal { + precision: to_p, + scale: to_s + } if to_p >= p && to_s == s + ) + } + _ => false, + } +} + +fn find_for_update( + name: &str, + schema: SchemaRef, + updates: &HashMap, + added_name_to_id: &HashMap, +) -> Result> { + let field = schema.field_by_name(name); + if let Some(field) = field { + let pending_update = updates.get(&field.id); + if let Some(pending_update) = pending_update { + Ok(Some(pending_update.clone())) + } else { + Ok(Some(field.clone())) + } + } else { + let added_id = added_name_to_id.get(name); + if let Some(added_id) = added_id { + Ok(updates.get(added_id).cloned()) + } else { + Ok(None) + } + } +} + +fn find_for_move( + name: &str, + schema: SchemaRef, + added_name_to_id: &HashMap, +) -> Result> { + let added_id = added_name_to_id.get(name); + if let Some(added_id) = added_id { + return Ok(Some(*added_id)); + } + let field = schema.field_by_name(name); + if let Some(field) = field { + return Ok(Some(field.id)); + } + Ok(None) +} + +fn assign_fresh_ids(field_type: Type, next_id: &mut i32) -> Type { + match field_type { + Type::Primitive(_) => field_type, + Type::Struct(s) => { + let new_fields = s + .fields() + .iter() + .map(|field| { + *next_id += 1; + let new_field_id = *next_id; + let new_type = assign_fresh_ids((*field.field_type).clone(), next_id); + Arc::new(NestedField::new( + new_field_id, + &field.name, + new_type, + field.required, + )) + }) + .collect(); + Type::Struct(StructType::new(new_fields)) + } + Type::List(list) => { + *next_id += 1; + let element_id = *next_id; + let element_type = assign_fresh_ids((*list.element_field.field_type).clone(), next_id); + Type::List(ListType::new(Arc::new(NestedField::new( + element_id, + &list.element_field.name, + element_type, + list.element_field.required, + )))) + } + Type::Map(map) => { + *next_id += 1; + let key_id = *next_id; + *next_id += 1; + let value_id = *next_id; + let key_type = assign_fresh_ids((*map.key_field.field_type).clone(), next_id); + let value_type = assign_fresh_ids((*map.value_field.field_type).clone(), next_id); + Type::Map(MapType::new( + Arc::new(NestedField::new( + key_id, + &map.key_field.name, + key_type, + true, + )), + Arc::new(NestedField::new( + value_id, + &map.value_field.name, + value_type, + map.value_field.required, + )), + )) + } + } +} + +struct ApplyChangesVisitor { + deletes: Vec, + updates: HashMap, + parent_to_added_ids: HashMap>, + moves: HashMap>, +} + +impl SchemaVisitor for ApplyChangesVisitor { + type T = Option; + + fn schema(&mut self, _schema: &Schema, value: Self::T) -> Result { + let added_fields: Vec = self + .parent_to_added_ids + .get(&TABLE_ROOT_ID) + .unwrap_or(&vec![]) + .iter() + .map(|id| self.updates.get(id).unwrap().clone()) + .collect(); + let fields = add_and_move_fields( + value.clone().unwrap().to_struct_type().unwrap().fields(), + &added_fields, + self.moves.get(&TABLE_ROOT_ID).unwrap_or(&vec![]), + ); + if !fields.is_empty() { + return Ok(Some(Type::Struct(StructType::new(fields)))); + } + Ok(value) + } + + fn r#struct(&mut self, r#struct: &StructType, results: Vec) -> Result { + let mut has_change = false; + let mut new_fields: Vec = Vec::with_capacity(results.len()); + for (result_type, field) in results.iter().zip(r#struct.fields()) { + if result_type.is_none() { + has_change = true; + continue; + } + let result_type = result_type.clone().unwrap(); + let update = self.updates.get(&field.id); + let updated = if let Some(update) = update { + Arc::unwrap_or_clone(update.clone()).of_type(Box::new(result_type)) + } else { + Arc::unwrap_or_clone(field.clone()).of_type(Box::new(result_type)) + }; + if field.as_ref() == &updated { + new_fields.push(field.clone()); + } else { + has_change = true; + new_fields.push(updated.into()); + } + } + if has_change { + return Ok(Some(Type::Struct(StructType::new(new_fields)))); + } + Ok(Some(Type::Struct(r#struct.clone()))) + } + + fn field(&mut self, field: &NestedFieldRef, value: Self::T) -> Result { + let field_id = field.id; + // handle deletes + if self.deletes.contains(&field_id) { + return Ok(None); + } + // handle updates + let update = self.updates.get(&field_id); + if let Some(update) = update + && update.field_type.as_ref() != field.field_type.as_ref() + { + return Ok(Some(*update.field_type.clone())); + } + // handle adds + let new_fields: Vec<_> = self + .parent_to_added_ids + .get(&field_id) + .unwrap_or(&vec![]) + .iter() + .filter_map(|id| self.updates.get(id)) + .cloned() + .collect(); + let columns_to_move = self.moves.get(&field_id).cloned().unwrap_or(vec![]); + if !new_fields.is_empty() || !columns_to_move.is_empty() { + let fields = add_and_move_fields( + value.clone().unwrap().to_struct_type().unwrap().fields(), + &new_fields, + &columns_to_move, + ); + if !fields.is_empty() { + return Ok(Some(Type::Struct(StructType::new(fields)))); + } + } + Ok(value) + } + + fn list(&mut self, list: &ListType, element_result: Self::T) -> Result { + let element_field = list.element_field.clone(); + let element_type = self + .field(&element_field, element_result)? + .ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete list element type from list: {:?}", list), + ))?; + let element_update = self.updates.get(&element_field.id); + let is_element_optional = if let Some(element_update) = element_update { + !element_update.required + } else { + !element_field.required + }; + let is_element_required = !is_element_optional; + if is_element_required == element_field.required + && &element_type == list.element_field.field_type.as_ref() + { + return Ok(Some(Type::List(list.clone()))); + } + if is_element_optional { + Ok(Some(Type::List(ListType::optional( + list.element_field.id, + element_type, + )))) + } else { + Ok(Some(Type::List(ListType::required( + list.element_field.id, + element_type, + )))) + } + } + + fn map( + &mut self, + map: &MapType, + key_result: Self::T, + value_result: Self::T, + ) -> Result { + let key_id = map.key_field.id; + if self.deletes.contains(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete map keys: {:?}", map), + )); + } else if self.updates.contains_key(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot update map keys: {:?}", map), + )); + } else if self.parent_to_added_ids.contains_key(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot add fields to map keys: {:?}", map), + )); + } else if map.key_field.field_type.as_ref() != &key_result.unwrap() { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot alter map keys: {:?}", map), + )); + } + let value_field = map.value_field.clone(); + let value_type = self.field(&value_field, value_result)?.ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete value type from map: {:?}", map), + ))?; + let value_update = self.updates.get(&value_field.id); + let is_value_required = if let Some(update) = value_update { + update.required + } else { + map.value_field.required + }; + if is_value_required == map.value_field.required + && map.value_field.field_type.as_ref() == &value_type + { + return Ok(Some(Type::Map(map.clone()))); + } + if is_value_required { + Ok(Some(Type::Map(MapType::required( + map.key_field.id, + *map.key_field.field_type.clone(), + map.value_field.id, + value_type, + )))) + } else { + Ok(Some(Type::Map(MapType::optional( + map.key_field.id, + *map.key_field.field_type.clone(), + map.value_field.id, + value_type, + )))) + } + } + + fn primitive(&mut self, p: &PrimitiveType) -> Result { + Ok(Some(Type::Primitive(p.clone()))) + } +} + +fn add_and_move_fields( + fields: &[NestedFieldRef], + adds: &[NestedFieldRef], + moves: &[Move], +) -> Vec { + if !adds.is_empty() { + if !moves.is_empty() { + return move_fields(&add_fields(fields, adds), moves); + } + return add_fields(fields, adds); + } else if !moves.is_empty() { + return move_fields(fields, moves); + } + vec![] +} + +fn add_fields(fields: &[NestedFieldRef], adds: &[NestedFieldRef]) -> Vec { + let mut new_fields = fields.to_owned(); + new_fields.extend(adds.iter().cloned()); + new_fields +} + +fn move_fields(fields: &[NestedFieldRef], moves: &[Move]) -> Vec { + let mut reordered = fields.to_vec(); + for r#move in moves { + let idx = reordered + .iter() + .position(|f| f.id == r#move.field_id()) + .unwrap(); + let to_move = reordered.remove(idx); + match r#move.r#type() { + MoveType::First => { + reordered.insert(0, to_move); + } + MoveType::Before => { + let before_idx = reordered + .iter() + .position(|f| f.id == r#move.reference_field_id()) + .unwrap(); + reordered.insert(before_idx, to_move); + } + MoveType::After => { + let after_idx = reordered + .iter() + .position(|f| f.id == r#move.reference_field_id()) + .unwrap(); + reordered.insert(after_idx + 1, to_move); + } + } + } + reordered +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + use std::sync::{Arc, LazyLock}; + + use crate::ErrorKind; + use crate::spec::{ + AddColumn, DeleteColumn, ListType, Literal, MapType, NestedField, PrimitiveType, + RenameColumn, Schema, SchemaOperation, StructType, Type, UpdateColumn, schema_update, + }; + + static SCHEMA: LazyLock = LazyLock::new(|| { + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Float.into()).into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap() + }); + + #[test] + fn no_changes() { + let base = SCHEMA.clone(); + let expected = SCHEMA.clone(); + let updated = schema_update(Arc::new(base), &[]).unwrap(); + assert_eq!(updated.as_ref(), &expected); + } + + #[test] + #[ignore = "not yet implemented"] + fn delete_fields() {} + + #[test] + #[ignore = "not yet implemented"] + fn delete_fields_case_sensitive_disabled() { + todo!() + } + + #[test] + fn update_types() { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Double.into()).into(), + NestedField::required(13, "long", PrimitiveType::Double.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + let updated = schema_update(Arc::new(SCHEMA.clone()), &[ + UpdateColumn::new_type("id", PrimitiveType::Long).into(), + UpdateColumn::new_type("locations.lat", PrimitiveType::Double).into(), + UpdateColumn::new_type("locations.long", PrimitiveType::Double).into(), + ]) + .unwrap(); + assert_eq!(&expected, updated.as_ref()); + } + + #[test] + #[ignore = "not yet implemented"] + fn update_type_preserves_other_metadata() { + todo!() + } + + #[test] + #[ignore = "not yet implemented"] + fn update_doc_preserves_other_metadata() { + todo!() + } + + #[test] + #[ignore = "not yet implemented"] + fn update_default_preserves_other_metadata() { + todo!() + } + + #[test] + #[ignore = "not yet implemented"] + fn update_types_case_insensitive() { + todo!() + } + + #[test] + fn update_failure() { + let allowed_updates: HashSet<(PrimitiveType, PrimitiveType)> = HashSet::from([ + (PrimitiveType::Int, PrimitiveType::Long), + (PrimitiveType::Float, PrimitiveType::Double), + ( + PrimitiveType::Decimal { + precision: 9, + scale: 2, + }, + PrimitiveType::Decimal { + precision: 18, + scale: 2, + }, + ), + ]); + let primitives = vec![ + PrimitiveType::Boolean, + PrimitiveType::Int, + PrimitiveType::Long, + PrimitiveType::Float, + PrimitiveType::Double, + PrimitiveType::Date, + PrimitiveType::Time, + PrimitiveType::Timestamp, + PrimitiveType::Timestamptz, + PrimitiveType::String, + PrimitiveType::Uuid, + PrimitiveType::Binary, + PrimitiveType::Fixed(3), + PrimitiveType::Fixed(4), + PrimitiveType::Decimal { + precision: 9, + scale: 2, + }, + PrimitiveType::Decimal { + precision: 9, + scale: 3, + }, + PrimitiveType::Decimal { + precision: 18, + scale: 2, + }, + // TODO: Geometry types and Geography types + ]; + for from in &primitives { + for to in &primitives { + let from_schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "col", from.clone().into()).into(), + ]) + .build() + .unwrap(), + ); + if from == to || allowed_updates.contains(&(from.clone(), to.clone())) { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "col", to.clone().into()).into(), + ]) + .build() + .unwrap(); + let result = + schema_update(from_schema, &[ + UpdateColumn::new_type("col", to.clone()).into() + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + continue; + } + let result = + schema_update(from_schema, &[ + UpdateColumn::new_type("col", to.clone()).into() + ]); + let err = result.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + format!("Cannot promote col from type {} to type {}", from, to) + ); + } + } + } + + #[test] + fn rename() { + let renamed = schema_update(Arc::new(SCHEMA.clone()), &[ + RenameColumn::builder() + .name("data") + .new_name("json") + .build() + .into(), + RenameColumn::builder() + .name("preferences") + .new_name("options") + .build() + .into(), + RenameColumn::builder() + .name("preferences.feature2") + .new_name("newfeature") + .build() + .into(), + RenameColumn::builder() + .name("locations.lat") + .new_name("latitude") + .build() + .into(), + RenameColumn::builder() + .name("points.x") + .new_name("X") + .build() + .into(), + RenameColumn::builder() + .name("points.y") + .new_name("Y") + .build() + .into(), + ]); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "json", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "options", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "newfeature", PrimitiveType::Boolean.into()) + .into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "latitude", PrimitiveType::Float.into()) + .into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "X", PrimitiveType::Long.into()).into(), + NestedField::required(16, "Y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + assert_eq!(renamed.unwrap().as_ref(), &expected); + } + + #[test] + #[ignore = "not yet implemented"] + fn rename_case_insensitive() {} + + #[test] + fn add_fields() { + let added = schema_update(Arc::new(SCHEMA.clone()), &[ + AddColumn::builder() + .name("topLevel") + .r#type(Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 2, + })) + .build() + .into(), + AddColumn::builder() + .parent("locations".to_string()) + .name("alt") + .r#type(Type::Primitive(PrimitiveType::Float)) + .build() + .into(), + AddColumn::builder() + .parent("points".to_string()) + .name("z") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build() + .into(), + AddColumn::builder() + .parent("points".to_string()) + .name("t.t") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build() + .into(), + ]) + .unwrap(); + + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Float.into()).into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + NestedField::optional(25, "alt", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + NestedField::optional(26, "z", PrimitiveType::Long.into()).into(), + NestedField::optional(27, "t.t", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + NestedField::optional( + 24, + "topLevel", + PrimitiveType::Decimal { + precision: 9, + scale: 2, + } + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + assert_eq!(added.as_struct(), expected.as_struct()); + } + + #[test] + fn add_column_with_default() { + let schema: Arc = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()) + .with_doc("description") + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema.clone(), &[AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .doc("description".into()) + .default_value(Literal::string("unknown")) + .build() + .into()]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_column_with_update_column_default() { + let schema: Arc = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema.clone(), &[ + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .build() + .into(), + UpdateColumn::new_default_value("data", Some(Literal::string("unknown"))).into(), + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_nested_struct() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let struct_type = StructType::new(vec![ + NestedField::required(1, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "long", PrimitiveType::Int.into()).into(), + ]); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional( + 2, + "location", + StructType::new(vec![ + NestedField::required(3, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(4, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + let result = schema_update(schema.clone(), &[AddColumn::builder() + .name("location") + .r#type(Type::Struct(struct_type)) + .build() + .into()]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_nested_map_of_structs() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional( + 2, + "locations", + MapType::optional( + 3, + StructType::new(vec![ + NestedField::required(5, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(6, "city", PrimitiveType::String.into()).into(), + NestedField::required(7, "state", PrimitiveType::String.into()).into(), + NestedField::required(8, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 4, + StructType::new(vec![ + NestedField::required(9, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(10, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let map = MapType::optional( + 1, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()).into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 2, + StructType::new(vec![ + NestedField::required(9, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(8, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ); + let result = schema_update(schema, &[AddColumn::builder() + .name("locations") + .r#type(map.into()) + .build() + .into()]) + .unwrap(); + assert_eq!(&expected, result.as_ref()) + } + + #[test] + #[ignore = "not yet implemented"] + fn add_nested_list_of_structs() { + todo!() + } + + #[test] + fn add_required_column_without_default() { + // Adding a required column with no default value is an incompatible change: + // existing rows have no value for the new column and there is no + // server-side fallback, so a read of the historical snapshot under the new + // schema would be undefined. Without `AllowIncompatibleChanges` this must + // fail with PreconditionFailed. + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + + let err = schema_update(schema.clone(), &[AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .is_optional(false) + .build() + .into()]) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + + // With AllowIncompatibleChanges the same add must succeed; the resulting + // column is required and has no initial/write default. + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema, &[ + SchemaOperation::AllowIncompatibleChanges, + AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .is_optional(false) + .build() + .into(), + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_required_column_with_default() { + // Adding a required column with an `initial_default` is a compatible + // change: old rows resolve to the default at read time, so no + // `AllowIncompatibleChanges` is needed. + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema, &[AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .is_optional(false) + .default_value(Literal::string("unknown")) + .build() + .into()]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_required_column_with_update_column_default() { + // Two-step variant: add optional column with a default, then use + // UpdateColumn::new_required to flip it to required. The `is_default_add` + // check on the Required arm recognizes the in-flight default, so the + // nullability change is permitted without AllowIncompatibleChanges. + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema, &[ + AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .default_value(Literal::string("unknown")) + .build() + .into(), + UpdateColumn::new_required("data", true).into(), + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + #[ignore = "not yet implemented"] + fn add_required_column_case_insensitive() { + todo!() + } + + #[test] + #[ignore = "not yet implemented"] + fn add_multiple_required_column_case_insensitive() { + todo!() + } + + #[test] + fn make_column_optional() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema.clone(), &[ + UpdateColumn::new_required("id", false).into() + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()) + } + + #[test] + fn require_column() { + let column = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + + // required to required is not an incompatible change + assert_eq!( + schema_update(Arc::new(expected.clone()), &[UpdateColumn::new_required( + "id", true + ) + .into()]) + .unwrap() + .as_ref(), + &expected + ); + + let result = schema_update(Arc::new(column), &[ + SchemaOperation::AllowIncompatibleChanges, + UpdateColumn::new_required("id", true).into(), + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn add_column_with_default_to_required_column() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = schema_update(schema.clone(), &[ + AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .default_value(Literal::string("unknown")) + .build() + .into(), + UpdateColumn::new_required("data", true).into(), + ]) + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + #[ignore = "not yet implemented"] + fn add_column_with_update_column_default_to_required_column() {} + + #[test] + #[ignore = "not yet implemented"] + fn require_column_case_insensitive() {} + + #[test] + fn test_mixed_changes() { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()) + .with_doc("unique id") + .into(), + NestedField::required(2, "json", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "options", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "newfeature", PrimitiveType::Boolean.into()) + .into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "latitude", PrimitiveType::Double.into()) + .with_doc("latitude") + .into(), + NestedField::optional(25, "alt", PrimitiveType::Float.into()).into(), + NestedField::required(28, "description", PrimitiveType::String.into()) + .with_doc("Location description") + .into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::optional(15, "X", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y.y", PrimitiveType::Long.into()).into(), + NestedField::optional(26, "z", PrimitiveType::Long.into()).into(), + NestedField::optional(27, "t.t", PrimitiveType::Long.into()) + .with_doc("name with '.'") + .into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 24, + "toplevel", + PrimitiveType::Decimal { + precision: 9, + scale: 2, + } + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + let updated = schema_update(Arc::new(SCHEMA.clone()), &[ + AddColumn::builder() + .name("toplevel") + .r#type(Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 2, + })) + .build() + .into(), + AddColumn::builder() + .parent("locations".into()) + .name("alt") + .r#type(Type::Primitive(PrimitiveType::Float)) + .build() + .into(), + AddColumn::builder() + .parent("points".into()) + .name("z") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build() + .into(), + AddColumn::builder() + .parent("points".to_string()) + .name("t.t") + .r#type(Type::Primitive(PrimitiveType::Long)) + .doc("name with '.'".into()) + .build() + .into(), + RenameColumn::builder() + .name("data") + .new_name("json") + .build() + .into(), + RenameColumn::builder() + .name("preferences") + .new_name("options") + .build() + .into(), + RenameColumn::builder() + .name("preferences.feature2") + .new_name("newfeature") + .build() + .into(), + RenameColumn::builder() + .name("locations.lat") + .new_name("latitude") + .build() + .into(), + RenameColumn::builder() + .name("points.x") + .new_name("X") + .build() + .into(), + RenameColumn::builder() + .name("points.y") + .new_name("y.y") + .build() + .into(), + UpdateColumn::new_type("id", PrimitiveType::Long).into(), + UpdateColumn::new_doc("id", Some("unique id".into())).into(), + UpdateColumn::new_type("locations.lat", PrimitiveType::Double).into(), + UpdateColumn::new_doc("locations.lat", Some("latitude".into())).into(), + DeleteColumn::new("locations.long").into(), + DeleteColumn::new("properties").into(), + UpdateColumn::new_required("points.x", false).into(), + SchemaOperation::AllowIncompatibleChanges, + UpdateColumn::new_required("data", true).into(), + AddColumn::builder() + .parent("locations".into()) + .name("description") + .r#type(Type::Primitive(PrimitiveType::String)) + .is_optional(false) + .doc("Location description".into()) + .build() + .into(), + ]) + .unwrap(); + + assert_eq!(&expected, updated.as_ref()); + } +} diff --git a/crates/iceberg/src/spec/values/primitive.rs b/crates/iceberg/src/spec/values/primitive.rs index 43d5c48c54..e3619b1f68 100644 --- a/crates/iceberg/src/spec/values/primitive.rs +++ b/crates/iceberg/src/spec/values/primitive.rs @@ -19,6 +19,8 @@ use ordered_float::OrderedFloat; +use crate::spec::Literal; + /// Values present in iceberg type #[derive(Clone, Debug, PartialOrd, PartialEq, Hash, Eq)] pub enum PrimitiveLiteral { @@ -57,3 +59,9 @@ impl PrimitiveLiteral { } } } + +impl From for Literal { + fn from(value: PrimitiveLiteral) -> Self { + Literal::Primitive(value) + } +} diff --git a/crates/iceberg/src/test_utils.rs b/crates/iceberg/src/test_utils.rs index d47c39950b..0fe491ac87 100644 --- a/crates/iceberg/src/test_utils.rs +++ b/crates/iceberg/src/test_utils.rs @@ -19,13 +19,19 @@ //! This module is pub just for internal testing. //! It is subject to change and is not intended to be used by external users. +use std::collections::HashSet; use std::sync::OnceLock; use arrow_array::RecordBatch; use expect_test::Expect; use itertools::Itertools; +use crate::Result; use crate::runtime::Runtime; +use crate::spec::{ + NestedFieldRef, PrimitiveType, Schema, SchemaVisitor, StructType, Type, VariantType, + visit_schema, visit_type, +}; /// Returns a process-wide [`Runtime`] suitable for tests that need to construct /// a [`Table`](crate::table::Table) outside a tokio context. @@ -98,3 +104,81 @@ pub fn check_record_batches( .format(",\n") )); } + +struct GetProjectedIds { + field_ids: HashSet, +} + +impl GetProjectedIds { + fn new() -> Self { + Self { + field_ids: HashSet::new(), + } + } +} + +impl SchemaVisitor for GetProjectedIds { + type T = (); + + fn schema(&mut self, _schema: &Schema, _value: Self::T) -> Result { + Ok(()) + } + + fn field(&mut self, field: &NestedFieldRef, _value: Self::T) -> Result { + if field.field_type.is_struct() + || field.field_type.is_primitive() + || field.field_type.is_variant() + { + self.field_ids.insert(field.id); + } + Ok(()) + } + + fn r#struct(&mut self, _struct: &StructType, _results: Vec) -> Result { + Ok(()) + } + + fn list(&mut self, list: &crate::spec::ListType, _value: Self::T) -> Result { + if list.element_field.field_type.is_primitive() { + self.field_ids.insert(list.element_field.id); + } + Ok(()) + } + + fn map( + &mut self, + map: &crate::spec::MapType, + _key_value: Self::T, + _value: Self::T, + ) -> Result { + if map.key_field.field_type.is_primitive() { + self.field_ids.insert(map.key_field.id); + } + if map.value_field.field_type.is_primitive() { + self.field_ids.insert(map.value_field.id); + } + Ok(()) + } + + fn primitive(&mut self, _p: &PrimitiveType) -> Result { + Ok(()) + } + + fn variant(&mut self, _v: &VariantType) -> Result { + Ok(()) + } +} + +/// Get the projected field ids of a type. +pub fn get_projected_ids_of_type(r#type: &Type) -> HashSet { + let mut visitor = GetProjectedIds::new(); + visit_type(r#type, &mut visitor).unwrap(); + visitor.field_ids +} + +/// Get the projected field ids of a schema. +pub fn get_projected_ids_of_schema(schema: &Schema) -> HashSet { + let mut visitor = GetProjectedIds::new(); + visit_schema(schema, &mut visitor).unwrap(); + visitor.field_ids +} diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index c2fde69fca..f9be4002fd 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -53,6 +53,7 @@ mod action; pub use action::*; +pub use update_schema::{AddColumn, DeleteColumn, MoveColumn, RenameColumn, UpdateColumn}; mod append; mod expire_snapshots; mod snapshot; @@ -67,7 +68,6 @@ use std::sync::Arc; use std::time::Duration; use backon::{BackoffBuilder, ExponentialBackoff, ExponentialBuilder, RetryableWithContext}; -pub use update_schema::AddColumn; use crate::error::Result; use crate::spec::TableProperties; @@ -141,11 +141,6 @@ impl Transaction { UpdatePropertiesAction::new() } - /// Creates an update schema action. - pub fn update_schema(&self) -> UpdateSchemaAction { - UpdateSchemaAction::new() - } - /// Creates a fast append action. pub fn fast_append(&self) -> FastAppendAction { FastAppendAction::new() @@ -171,6 +166,14 @@ impl Transaction { ExpireSnapshotsAction::new() } + /// Update the schema of table + pub fn update_schema(&self) -> Result { + UpdateSchemaAction::new( + self.table.current_schema_ref(), + self.table.metadata().last_column_id(), + ) + } + /// Commit transaction. pub async fn commit(self, catalog: &dyn Catalog) -> Result { if self.actions.is_empty() { diff --git a/crates/iceberg/src/transaction/update_schema.rs b/crates/iceberg/src/transaction/update_schema.rs index 953bcd64ab..abc9a40781 100644 --- a/crates/iceberg/src/transaction/update_schema.rs +++ b/crates/iceberg/src/transaction/update_schema.rs @@ -22,1144 +22,6144 @@ use async_trait::async_trait; use typed_builder::TypedBuilder; use crate::spec::{ - ListType, Literal, MapType, NestedField, NestedFieldRef, SCHEMA_NAME_DELIMITER, Schema, - StructType, Type, + IndexByName, ListType, Literal, MapType, NestedField, NestedFieldRef, PrimitiveType, Schema, + SchemaRef, SchemaVisitor, StructType, Type, VariantType, index_parents, visit_schema, + visit_struct, }; use crate::table::Table; -use crate::transaction::action::{ActionCommit, TransactionAction}; -use crate::{Error, ErrorKind, Result, TableRequirement, TableUpdate}; - -// Default ID for a new column. This will be re-assigned to a fresh ID at commit time. -const DEFAULT_FIELD_ID: i32 = 0; - -/// Declarative specification for adding a column in [`UpdateSchemaAction`]. -/// -/// Use helper constructors such as [`AddColumn::optional`] and [`AddColumn::required`], -/// optionally combined with [`AddColumn::with_parent`] and [`AddColumn::with_doc`], then pass -/// the value to -/// [`UpdateSchemaAction::add_column`]. -#[derive(TypedBuilder)] -pub struct AddColumn { - #[builder(default = None, setter(strip_option, into))] - parent: Option, - #[builder(setter(into))] - name: String, - #[builder(default = false)] - required: bool, - field_type: Type, - #[builder(default = None, setter(strip_option, into))] - doc: Option, - #[builder(default = None, setter(strip_option))] - initial_default: Option, - #[builder(default = None, setter(strip_option))] - write_default: Option, +use crate::transaction::{ActionCommit, TransactionAction}; +use crate::{Error, ErrorKind, Result, TableRequirement, TableUpdate, ensure_precondition}; + +const TABLE_ROOT_ID: i32 = -1; + +#[derive(Debug)] +pub struct UpdateSchemaAction { + schema: SchemaRef, + + updates: HashMap, + deletes: Vec, + moves: HashMap>, + parent_to_added_ids: HashMap>, + id_to_parent: HashMap, + added_name_to_id: HashMap, + identifier_field_ids: HashSet, + allow_incompatible_changes: bool, + last_column_id: i32, + case_sensitive: bool, + identifier_field_names: Option>, } -impl AddColumn { - /// Create a root-level optional column specification. - pub fn optional(name: impl ToString, field_type: Type) -> Self { - Self::builder() - .name(name.to_string()) - .field_type(field_type) - .required(false) - .build() +impl UpdateSchemaAction { + pub(crate) fn new(schema: SchemaRef, last_column_id: i32) -> Result { + let id_to_parent = index_parents(schema.as_struct())?; + Ok(Self { + schema: schema.clone(), + updates: HashMap::new(), + deletes: Vec::new(), + moves: HashMap::new(), + parent_to_added_ids: HashMap::new(), + id_to_parent, + added_name_to_id: HashMap::new(), + identifier_field_ids: schema.identifier_field_ids().collect(), + allow_incompatible_changes: false, + last_column_id, + case_sensitive: true, + identifier_field_names: None, + }) } - /// Create a root-level required column specification. - pub fn required(name: impl ToString, field_type: Type, initial_default: Literal) -> Self { - Self::builder() - .name(name.to_string()) - .field_type(field_type) - .required(true) - .initial_default(initial_default.clone()) - .write_default(initial_default) - .build() + pub fn add(mut self, add: AddColumn) -> Result { + let (parent, name, is_optional, field_type, doc, default_value) = ( + &add.parent, + &add.name, + add.is_optional, + &add.r#type, + &add.doc, + &add.default_value, + ); + if parent.is_none() && name.contains(".") { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!( + "Cannot add column with ambiguous name: {}, use addColumn(parent, name, type)", + name + ), + )); + } + let mut parent_id = TABLE_ROOT_ID; + let full_name = if let Some(Some(parent)) = parent { + let parent_field = self.find_field(parent).ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot find parent struct: {}", parent), + ))?; + let parent_field = if parent_field.field_type.is_nested() { + let parent_type = parent_field.field_type.as_ref(); + match parent_type { + Type::List(nested) => nested.element_field.as_ref(), // fields are added to the element type + Type::Map(nested) => nested.value_field.as_ref(), // fields are added to the map value type + _ => parent_field, + } + } else { + parent_field + }; + ensure_precondition!( + parent_field.field_type.is_struct(), + "Cannot add to non-struct column: {}: {}", + &parent, + parent_field.field_type + ); + parent_id = parent_field.id; + let full_name = format!("{}.{}", parent, name); + let current_field = self.find_field(&full_name); + ensure_precondition!( + !self.deletes.contains(&parent_id), + "Can not add a column that will be deleted: {}", + name + ); + ensure_precondition!( + current_field.is_none() || self.deletes.contains(¤t_field.unwrap().id), + "Cannot add column, name already exists: {}.{}", + &parent, + &name + ); + full_name + } else { + let current_field = self.find_field(name); + ensure_precondition!( + current_field.is_none() || self.deletes.contains(¤t_field.unwrap().id), + "Cannot add column, name already exists: {}", + &name + ); + name.clone() + }; + ensure_precondition!( + default_value.is_some() || is_optional || self.allow_incompatible_changes, + "Incompatible change: cannot add required column without a default value: {}", + full_name + ); + self.last_column_id += 1; + let new_id = self.last_column_id; + self.added_name_to_id + .insert(self.case_sensitivity_aware_name(&full_name), new_id); + + if parent_id != TABLE_ROOT_ID { + self.id_to_parent.insert(new_id, parent_id); + } + // TODO: Maybe we can use `ReassignFieldIds`? + let assigned_type = assign_fresh_ids(field_type.clone(), &mut self.last_column_id); + let mut new_field = NestedField::new(new_id, name, assigned_type, !is_optional); + new_field.doc = doc.clone(); + new_field.write_default = default_value.clone(); + new_field.initial_default = default_value.clone(); + self.updates.insert(new_id, new_field.into()); + self.parent_to_added_ids + .entry(parent_id) + .or_default() + .push(new_id); + Ok(self) } - fn to_nested_field(&self) -> NestedFieldRef { - let mut field = NestedField::new( - DEFAULT_FIELD_ID, - self.name.clone(), - self.field_type.clone(), - self.required, + pub fn update(mut self, update: UpdateColumn) -> Result { + let (name, ops) = (&update.name, &update.op); + let field = self.find_for_update(name)?.ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot update missing column: {}", name), + ))?; + ensure_precondition!( + !self.deletes.contains(&field.id), + "Cannot update column that will be deleted: {}", + name, ); - - field.doc = self.doc.clone(); - field.initial_default = self.initial_default.clone(); - field.write_default = self.write_default.clone(); - Arc::new(field) + let mut new_field = Arc::unwrap_or_clone(field.clone()); + for op in ops { + match op { + UpdateColumnOperation::Required(new_required) => { + if (*new_required && !field.required) || (!*new_required && field.required) { + let is_default_add = self + .added_name_to_id + .contains_key(&self.case_sensitivity_aware_name(name)) + && field.initial_default.is_some(); + ensure_precondition!( + !*new_required || is_default_add || self.allow_incompatible_changes, + "Cannot change column nullability: {}: optional -> required", + name + ); + new_field.required = *new_required; + } + } + UpdateColumnOperation::Type(new_type) => { + ensure_precondition!( + is_promotion_allowed(field.field_type.as_ref(), new_type), + "Cannot promote {} from type {} to type {}", + name, + field.field_type, + new_type + ); + *new_field.field_type = new_type.clone().into(); + } + UpdateColumnOperation::Doc(new_doc) => { + new_field.doc = new_doc.clone(); + } + UpdateColumnOperation::DefaultValue(new_default_value) => { + new_field.write_default = new_default_value.clone(); + } + } + } + self.updates.insert(field.id, Arc::new(new_field)); + Ok(self) } -} -/// Schema evolution API modeled after the Java `SchemaUpdate` implementation. -/// -/// This action accumulates schema modifications (column additions and deletions) -/// via builder methods. At commit time, it validates all operations against the -/// current table schema, auto-assigns field IDs from `table.metadata().last_column_id()`, -/// builds a new schema, and emits `AddSchema` + `SetCurrentSchema` updates with a -/// `CurrentSchemaIdMatch` requirement. -/// -/// # Example -/// -/// ```ignore -/// let tx = Transaction::new(&table); -/// let action = tx.update_schema() -/// .add_column(AddColumn::optional("new_col", Type::Primitive(PrimitiveType::Int))) -/// .add_column( -/// AddColumn::optional("email", Type::Primitive(PrimitiveType::String)) -/// .with_parent("person") -/// ) -/// .delete_column("old_col"); -/// let tx = action.apply(tx).unwrap(); -/// let table = tx.commit(&catalog).await.unwrap(); -/// ``` -pub struct UpdateSchemaAction { - additions: Vec, - deletes: Vec, -} + pub fn require_column(self, name: &str) -> Result { + self.update(UpdateColumn::builder(name).with_required(true).build()) + } -impl UpdateSchemaAction { - /// Creates a new empty `UpdateSchemaAction`. - pub(crate) fn new() -> Self { - Self { - additions: Vec::new(), - deletes: Vec::new(), + pub fn rename(mut self, rename: RenameColumn) -> Result { + let (name, new_name) = (&rename.name, &rename.new_name); + let field = self.find_field(name).ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot rename missing column: {}", name), + ))?; + ensure_precondition!( + !self.deletes.contains(&field.id), + "Cannot rename a column that will be deleted: {}", + name + ); + // merge with an update, if present + let field_id = field.id; + let update = self.updates.get(&field_id); + let new_field = if let Some(update) = update { + Arc::unwrap_or_clone(update.clone()).with_name(new_name) + } else { + Arc::unwrap_or_clone(field.clone()).with_name(new_name) + }; + self.updates.insert(field_id, Arc::new(new_field)); + if self.identifier_field_ids.contains(&field_id) { + self.identifier_field_ids.remove(&field_id); + self.identifier_field_ids.insert(field_id); } + Ok(self) + } + + pub fn delete(mut self, delete: DeleteColumn) -> Result { + let field = self.find_field(&delete.name).ok_or_else(|| { + Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete missing column: {}", delete.name), + ) + })?; + ensure_precondition!( + !self.parent_to_added_ids.contains_key(&field.id), + "Cannot delete a column that has additions: {}", + delete.name + ); + ensure_precondition!( + !self.updates.contains_key(&field.id), + "Cannot delete a column that has updates: {}", + delete.name + ); + self.deletes.push(field.id); + Ok(self) } - // --- Root-level additions --- + pub fn move_column(mut self, move_column: MoveColumn) -> Result { + let (name, reference_name, move_type) = ( + &move_column.name, + &move_column.reference_name, + &move_column.move_type, + ); + let field_id = self.find_for_move(name)?.ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot move missing column: {}", name), + ))?; + let r#move = if move_type == &MoveType::First { + Move::first(field_id) + } else { + let reference_field_id = self.find_for_move(reference_name)?.ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot move relative to missing column: {}", reference_name), + ))?; + match move_type { + MoveType::Before => Move::before(field_id, reference_field_id), + MoveType::After => Move::after(field_id, reference_field_id), + _ => unreachable!(), + } + }; + let parent_id = self.id_to_parent.get(&field_id); + if let Some(&parent_id) = parent_id { + let parent = self.schema.field_by_id(parent_id).unwrap(); + ensure_precondition!( + parent.field_type.is_struct(), + "Cannot move fields in non-struct type: {}", + parent.field_type + ); + if r#move.r#type == MoveType::After || r#move.r#type == MoveType::Before { + ensure_precondition!( + parent_id == *self.id_to_parent.get(&r#move.reference_field_id).unwrap(), + "Cannot move field {} to a different struct", + name, + ); + } + self.moves.entry(parent_id).or_default().push(r#move); + } else { + if move_type == &MoveType::After || move_type == &MoveType::Before { + ensure_precondition!( + !self.id_to_parent.contains_key(&r#move.reference_field_id), + "Cannot move field {} to a different struct", + name, + ); + } + self.moves.entry(TABLE_ROOT_ID).or_default().push(r#move); + } + Ok(self) + } - /// Add a column to the table schema. - /// - /// To add a root-level column, leave `AddColumn::parent` as `None`. - /// For nested additions, set a parent path (for example via [`AddColumn::with_parent`]). - /// If the parent resolves to a map/list, the column is added to map value/list element. - pub fn add_column(mut self, add_column: AddColumn) -> Self { - self.additions.push(add_column); + pub fn allow_incompatible_changes(mut self) -> Self { + self.allow_incompatible_changes = true; self } - // --- Other builder methods --- + pub fn case_sensitive(mut self, case_sensitive: bool) -> Self { + self.case_sensitive = case_sensitive; + self + } - /// Record a column deletion by name. - /// - /// At commit time, the column must exist in the current schema. - pub fn delete_column(mut self, name: impl ToString) -> Self { - self.deletes.push(name.to_string()); + pub fn set_identifier_fields( + mut self, + identifier_field_names: impl IntoIterator, + ) -> Self { + self.identifier_field_names = Some(identifier_field_names.into_iter().collect()); self } -} -// --------------------------------------------------------------------------- -// ID assignment helpers -// --------------------------------------------------------------------------- - -/// Recursively assign fresh field IDs to a `NestedField` and all its nested sub-fields. -/// -/// This follows the same recursive pattern as `ReassignFieldIds::reassign_ids_visit_type` -/// from `crate::spec::schema::id_reassigner`, but operates on new fields with placeholder -/// IDs rather than reassigning an existing schema. `ReassignFieldIds` cannot be used -/// directly here because it rejects duplicate old IDs (all new fields share placeholder -/// ID `DEFAULT_FIELD_ID`). -fn assign_fresh_ids(field: &NestedField, next_id: &mut i32) -> NestedFieldRef { - *next_id += 1; - let new_id = *next_id; - let new_type = assign_fresh_ids_to_type(&field.field_type, next_id); - - Arc::new(NestedField { - id: new_id, - name: field.name.clone(), - required: field.required, - field_type: Box::new(new_type), - doc: field.doc.clone(), - initial_default: field.initial_default.clone(), - write_default: field.write_default.clone(), - }) -} + pub fn apply(&self) -> Result { + let schema = self.schema.clone(); + + for &id in &self.identifier_field_ids { + let field = schema.field_by_id(id); + if let Some(field) = field { + ensure_precondition!( + !self.deletes.contains(&id), + "Cannot delete identifier field: {}. To force deletion, also call setIdentifierFields to update identifier fields.", + field.name + ); + let mut parent_id = self.id_to_parent.get(&id); + while let Some(p_id) = parent_id { + ensure_precondition!( + !self.deletes.contains(p_id), + "Cannot delete field {} as it will delete nested identifier field {}.", + p_id, + field.name + ); + parent_id = self.id_to_parent.get(p_id); + } + } + } + // apply schema changes + let mut visitor = ApplyChangesVisitor { + deletes: &self.deletes, + updates: &self.updates, + parent_to_added_ids: &self.parent_to_added_ids, + moves: &self.moves, + }; + let struct_type = visit_schema(schema.as_ref(), &mut visitor)? + .unwrap() + .to_struct_type() + .unwrap(); -/// Recursively assign fresh field IDs to all nested fields within a `Type`. -fn assign_fresh_ids_to_type(field_type: &Type, next_id: &mut i32) -> Type { - match field_type { - Type::Primitive(_) => field_type.clone(), - // Variant carries no nested fields, so there is nothing to reassign - // (matches id_reassigner.rs). - Type::Variant(v) => Type::Variant(*v), - Type::Struct(struct_type) => { - let new_fields: Vec = struct_type - .fields() - .iter() - .map(|f| assign_fresh_ids(f, next_id)) - .collect(); - Type::Struct(StructType::new(new_fields)) + // validate identifier requirements based on the latest schema (validate id done in Schema::build) + let fresh_identifier_ids = if let Some(identifier_field_names) = + &self.identifier_field_names + { + let (name_to_id, _) = { + let mut index = IndexByName::default(); + visit_struct(&struct_type, &mut index)?; + index.indexes() + }; + let mut fresh_identifier_ids = HashSet::new(); + for name in identifier_field_names { + ensure_precondition!( + name_to_id.contains_key(name), + "Cannot add field {} as an identifier field: not found in current schema or added columns", + name + ); + let id = name_to_id.get(name).unwrap(); + fresh_identifier_ids.insert(*id); + } + fresh_identifier_ids + } else { + self.identifier_field_ids.clone() + }; + let schema = Schema::builder() + .with_fields(struct_type.fields().to_vec()) + .with_identifier_field_ids(fresh_identifier_ids) + .build()? + .into(); + Ok(schema) + } + + fn find_field(&self, field_name: &str) -> Option<&NestedFieldRef> { + if self.case_sensitive { + self.schema.field_by_name(field_name) + } else { + self.schema.field_by_name_case_insensitive(field_name) + } + } + + fn find_for_update(&self, name: &str) -> Result> { + let field = self.find_field(name); + if let Some(field) = field { + let pending_update = self.updates.get(&field.id); + if let Some(pending_update) = pending_update { + Ok(Some(pending_update.clone())) + } else { + Ok(Some(field.clone())) + } + } else { + let added_id = self + .added_name_to_id + .get(&self.case_sensitivity_aware_name(name)); + if let Some(added_id) = added_id { + Ok(self.updates.get(added_id).cloned()) + } else { + Ok(None) + } } - Type::List(list_type) => { - let new_element = assign_fresh_ids(&list_type.element_field, next_id); - Type::List(ListType { - element_field: new_element, - }) + } + + fn find_for_move(&self, name: &str) -> Result> { + let added_id = self + .added_name_to_id + .get(&self.case_sensitivity_aware_name(name)); + if let Some(added_id) = added_id { + return Ok(Some(*added_id)); } - Type::Map(map_type) => { - let new_key = assign_fresh_ids(&map_type.key_field, next_id); - let new_value = assign_fresh_ids(&map_type.value_field, next_id); - Type::Map(MapType { - key_field: new_key, - value_field: new_value, - }) + let field = self.find_field(name); + if let Some(field) = field { + return Ok(Some(field.id)); } + Ok(None) } -} -// --------------------------------------------------------------------------- -// Parent path resolution -// --------------------------------------------------------------------------- - -/// Resolve a parent path to the target struct's parent field ID and a reference -/// to its `StructType`. -/// -/// If the parent is a map, navigates to the value field. If a list, navigates to -/// the element field. The final target must be a struct type. -fn resolve_parent_target<'a>( - base_schema: &'a Schema, - parent: &str, -) -> Result<(i32, &'a StructType)> { - base_schema - .field_by_name(parent) - .ok_or_else(|| { - Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot add column: parent '{parent}' not found"), - ) - }) - .and_then(|parent_field| match parent_field.field_type.as_ref() { - Type::Struct(s) => Ok((parent_field.id, s)), - Type::Map(m) => match m.value_field.field_type.as_ref() { - Type::Struct(s) => Ok((m.value_field.id, s)), - _ => Err(Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot add column: map value of '{parent}' is not a struct"), - )), - }, - Type::List(l) => match l.element_field.field_type.as_ref() { - Type::Struct(s) => Ok((l.element_field.id, s)), - _ => Err(Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot add column: list element of '{parent}' is not a struct"), - )), - }, - _ => Err(Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot add column: parent '{parent}' is not a struct, map, or list"), - )), - }) + fn case_sensitivity_aware_name(&self, name: &str) -> String { + if self.case_sensitive { + name.into() + } else { + name.to_lowercase() + } + } } -// --------------------------------------------------------------------------- -// Schema tree rebuild -// --------------------------------------------------------------------------- +#[async_trait] +impl TransactionAction for UpdateSchemaAction { + async fn commit(self: Arc, table: &Table) -> Result { + let current_schema_id = table.metadata().current_schema_id(); + let last_column_id = table.metadata().last_column_id(); + + let schema = self.apply()?; + + // TODO: apply changes to metadata(properties) + // e.g. parse and update the mapping, transform the metrics + Ok(ActionCommit::new( + vec![ + TableUpdate::AddSchema { + schema: Arc::unwrap_or_clone(schema), + }, + TableUpdate::SetCurrentSchema { schema_id: -1 }, + ], + vec![ + TableRequirement::CurrentSchemaIdMatch { current_schema_id }, + TableRequirement::LastAssignedFieldIdMatch { + last_assigned_field_id: last_column_id, + }, + ], + )) + } +} -/// Rebuild a slice of fields, applying deletions and additions at every level, -/// plus any additions keyed by `parent_id` (`None` represents the table root). -fn rebuild_fields( - fields: &[NestedFieldRef], - adds: &HashMap, Vec>, - delete_ids: &HashSet, - parent_id: Option, -) -> Vec { - fields - .iter() - .filter(|f| !delete_ids.contains(&f.id)) - .map(|f| rebuild_field(f, adds, delete_ids)) - .chain(adds.get(&parent_id).into_iter().flatten().cloned()) - .collect() +/// A column to be added to the schema. +#[derive(TypedBuilder)] +pub struct AddColumn { + #[builder(default, setter(strip_option))] + parent: Option>, + #[builder(setter(into))] + name: String, + #[builder(default = true)] + is_optional: bool, + r#type: Type, + #[builder(default, setter(strip_option))] + doc: Option, + #[builder(default, setter(strip_option))] + default_value: Option, } -/// Recursively rebuild a single field. If the field (or any descendant) is a struct -/// that has pending additions, those additions are appended to the struct's fields. -/// Fields whose IDs appear in `delete_ids` are filtered out at every struct level. -fn rebuild_field( - field: &NestedFieldRef, - adds: &HashMap, Vec>, - delete_ids: &HashSet, -) -> NestedFieldRef { - match field.field_type.as_ref() { - Type::Primitive(_) | Type::Variant(_) => field.clone(), - Type::Struct(s) => { - let new_fields = rebuild_fields(s.fields(), adds, delete_ids, Some(field.id)); - Arc::new(NestedField { - id: field.id, - name: field.name.clone(), - required: field.required, - field_type: Box::new(Type::Struct(StructType::new(new_fields))), - doc: field.doc.clone(), - initial_default: field.initial_default.clone(), - write_default: field.write_default.clone(), - }) +impl AddColumn { + /// Create a new required `AddColumn` with the given name and type. + pub fn required(name: impl Into, r#type: Type) -> Self { + Self { + parent: None, + name: name.into(), + is_optional: false, + r#type, + doc: None, + default_value: None, } - Type::List(l) => { - let new_element = rebuild_field(&l.element_field, adds, delete_ids); - Arc::new(NestedField { - id: field.id, - name: field.name.clone(), - required: field.required, - field_type: Box::new(Type::List(ListType { - element_field: new_element, - })), - doc: field.doc.clone(), - initial_default: field.initial_default.clone(), - write_default: field.write_default.clone(), - }) + } + + /// Create a new optional `AddColumn` with the given name and type. + pub fn optional(name: impl Into, r#type: Type) -> Self { + Self { + parent: None, + name: name.into(), + is_optional: true, + r#type, + doc: None, + default_value: None, } - Type::Map(m) => { - let new_key = rebuild_field(&m.key_field, adds, delete_ids); - let new_value = rebuild_field(&m.value_field, adds, delete_ids); - Arc::new(NestedField { - id: field.id, - name: field.name.clone(), - required: field.required, - field_type: Box::new(Type::Map(MapType { - key_field: new_key, - value_field: new_value, - })), - doc: field.doc.clone(), - initial_default: field.initial_default.clone(), - write_default: field.write_default.clone(), - }) + } + + /// Set the parent for the `AddColumn`. + pub fn parent(mut self, parent: impl Into>) -> Self { + self.parent = Some(parent.into()); + self + } +} + +/// A column to be deleted from the schema. +pub struct DeleteColumn { + name: String, +} + +impl DeleteColumn { + /// Create a new `DeleteColumn` with the given column name. + pub fn new(name: impl Into) -> Self { + Self { name: name.into() } + } +} + +/// A column to be renamed in the schema. +#[derive(TypedBuilder)] +pub struct RenameColumn { + #[builder(setter(into))] + name: String, + #[builder(setter(into))] + new_name: String, +} + +impl RenameColumn { + /// Create a new `RenameColumn` with the given column name and new name. + pub fn new(name: impl Into, new_name: impl Into) -> Self { + Self { + name: name.into(), + new_name: new_name.into(), } } } -// --------------------------------------------------------------------------- -// TransactionAction implementation -// --------------------------------------------------------------------------- +/// A column to be updated in the schema. +pub struct UpdateColumn { + name: String, + op: Vec, +} -#[async_trait] -impl TransactionAction for UpdateSchemaAction { - async fn commit(self: Arc, table: &Table) -> Result { - let base_schema = table.metadata().current_schema(); - let mut last_column_id = table.metadata().last_column_id(); +impl UpdateColumn { + /// Returns a builder for creating an `UpdateColumn`. + pub fn builder(name: impl Into) -> UpdateColumnBuilder { + UpdateColumnBuilder { + name: name.into(), + is_required: None, + new_type: None, + new_doc: None, + new_default_value: None, + } + } +} - // --- 1. Validate deletes --- - let delete_ids = self - .deletes - .iter() - .map(|name: &String| { - base_schema - .field_by_name(name) - .ok_or_else(|| { - Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot delete missing column: {name}"), - ) - }) - .and_then(|field| { - match base_schema - .identifier_field_ids() - .find(|id| *id == field.id) - { - Some(_) => Err(Error::new( - ErrorKind::PreconditionFailed, - format!("Cannot delete identifier field: {name}"), - )), - None => Ok(field.id), - } - }) - }) - .collect::>>()?; - - // --- 2. Resolve parents, validate additions, assign IDs, and group by parent ID --- - // We assign IDs inline (before grouping) to preserve the caller's insertion order, - // since HashMap iteration order is non-deterministic. - let mut additions_by_parent: HashMap, Vec> = HashMap::new(); - - for add in &self.additions { - let pending_field = add.to_nested_field(); - - // Check that name does not contain `SCHEMA_NAME_DELIMITER`. - if pending_field.name.contains(SCHEMA_NAME_DELIMITER) { - return Err(Error::new( - ErrorKind::PreconditionFailed, - format!( - "Cannot add column with ambiguous name: {}. Use `AddColumn::with_parent` to add a column to a nested struct.", - pending_field.name - ), - )); - } +/// A builder for constructing `UpdateColumn`. +pub struct UpdateColumnBuilder { + name: String, + is_required: Option, + new_type: Option, + new_doc: Option>, + new_default_value: Option>, +} - // Required columns without an initial default need allow_incompatible_changes. - if pending_field.required && pending_field.initial_default.is_none() { - return Err(Error::new( - ErrorKind::PreconditionFailed, - format!( - "Incompatible change: cannot add required column without an initial default: {}", - pending_field.name - ), - )); - } +impl UpdateColumnBuilder { + /// Set the required status for the column. + pub fn with_required(mut self, is_required: bool) -> Self { + self.is_required = Some(is_required); + self + } - let parent_id = match &add.parent { - None => { - // Root-level: check name conflict against root-level fields. - if let Some(existing) = base_schema.field_by_name(&pending_field.name) - && !delete_ids.contains(&existing.id) - { - return Err(Error::new( - ErrorKind::PreconditionFailed, - format!( - "Cannot add column, name already exists: {}", - pending_field.name - ), - )); - } - None - } - Some(parent_path) => { - // Nested: resolve parent, check name conflict within parent struct. - let (resolved_parent_id, parent_struct) = - resolve_parent_target(base_schema, parent_path)?; - - if parent_struct.fields().iter().any(|f| { - f.name == pending_field.name - && !delete_ids.contains(&f.id) - && !delete_ids.contains(&resolved_parent_id) - }) { - return Err(Error::new( - ErrorKind::PreconditionFailed, - format!( - "Cannot add column, name already exists in '{}': {}", - parent_path, pending_field.name - ), - )); - } + /// Set the new type for the column. + pub fn with_type(mut self, new_type: PrimitiveType) -> Self { + self.new_type = Some(new_type); + self + } - Some(resolved_parent_id) - } - }; + /// Set the doc for the column. + pub fn with_doc(mut self, new_doc: Option) -> Self { + self.new_doc = Some(new_doc); + self + } - // Assign fresh IDs immediately, preserving insertion order. - let field = assign_fresh_ids(&pending_field, &mut last_column_id); + /// Set the default value for the column. + pub fn with_default_value(mut self, new_default_value: Option) -> Self { + self.new_default_value = Some(new_default_value); + self + } - additions_by_parent - .entry(parent_id) - .or_default() - .push(field); + /// Build the `UpdateColumn`. + pub fn build(self) -> UpdateColumn { + let mut ops = Vec::new(); + if let Some(is_required) = self.is_required { + ops.push(UpdateColumnOperation::Required(is_required)); } + if let Some(new_type) = self.new_type { + ops.push(UpdateColumnOperation::Type(new_type)); + } + if let Some(new_doc) = self.new_doc { + ops.push(UpdateColumnOperation::Doc(new_doc)); + } + if let Some(new_default_value) = self.new_default_value { + ops.push(UpdateColumnOperation::DefaultValue(new_default_value)); + } + UpdateColumn { + name: self.name, + op: ops, + } + } +} - // --- 4. Rebuild the schema tree with additions and deletions --- - let new_fields = rebuild_fields( - base_schema.as_struct().fields(), - &additions_by_parent, - &delete_ids, - None, - ); +enum UpdateColumnOperation { + Required(bool), + Type(PrimitiveType), + Doc(Option), + DefaultValue(Option), +} - // --- 5. Build the new schema --- - let schema = Schema::builder() - .with_fields(new_fields) - .with_identifier_field_ids(base_schema.identifier_field_ids()) - .build()?; +/// A column to be moved in the schema. +pub struct MoveColumn { + name: String, + reference_name: String, + move_type: MoveType, +} - let updates = vec![ - TableUpdate::AddSchema { schema }, - TableUpdate::SetCurrentSchema { schema_id: -1 }, - ]; +impl MoveColumn { + /// Move the column to the first position. + pub fn first(name: impl Into) -> Self { + Self { + name: name.into(), + reference_name: String::new(), + move_type: MoveType::First, + } + } - let requirements = vec![TableRequirement::CurrentSchemaIdMatch { - current_schema_id: base_schema.schema_id(), - }]; + /// Move the column before the reference column. + pub fn before(name: impl Into, reference: impl Into) -> Self { + Self { + name: name.into(), + reference_name: reference.into(), + move_type: MoveType::Before, + } + } - Ok(ActionCommit::new(updates, requirements)) + /// Move the column after the reference column. + pub fn after(name: impl Into, reference: impl Into) -> Self { + Self { + name: name.into(), + reference_name: reference.into(), + move_type: MoveType::After, + } } } -#[cfg(test)] -mod tests { - use std::io::BufReader; - use std::sync::Arc; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum MoveType { + First, + Before, + After, +} - use as_any::Downcast; +#[derive(Clone, Debug)] +struct Move { + field_id: i32, + reference_field_id: i32, + r#type: MoveType, +} - use crate::spec::{ - DEFAULT_SCHEMA_ID, Literal, NestedField, PrimitiveType, StructType, TableMetadata, Type, - VariantType, - }; - use crate::table::Table; - use crate::transaction::Transaction; - use crate::transaction::action::{ApplyTransactionAction, TransactionAction}; - use crate::transaction::tests::make_v2_table; - use crate::transaction::update_schema::{AddColumn, DEFAULT_FIELD_ID, UpdateSchemaAction}; - use crate::{ErrorKind, TableIdent, TableRequirement, TableUpdate}; - - // The V2 test table has: - // last_column_id: 3 - // current schema (id=1): x(1, req, long), y(2, req, long), z(3, req, long) - // identifier_field_ids: [1, 2] - - /// Build a V2 test table that includes nested types: - /// - /// last_column_id: 14 - /// current schema (id=0): - /// x(1, req, long) -- identifier - /// y(2, req, long) -- identifier - /// z(3, req, long) - /// person(4, opt, struct) - /// name(5, opt, string) - /// age(6, req, int) - /// tags(7, opt, list) - /// element(8, req, struct) - /// key(9, opt, string) - /// value(10, opt, string) - /// props(11, opt, map) - /// key(12, req, string) - /// value(13, req, struct) - /// data(14, opt, string) - fn make_v2_table_with_nested() -> Table { - let json = r#"{ - "format-version": 2, - "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c2", - "location": "s3://bucket/test/location", - "last-sequence-number": 0, - "last-updated-ms": 1602638573590, - "last-column-id": 14, - "current-schema-id": 0, - "schemas": [ - { - "type": "struct", - "schema-id": 0, - "identifier-field-ids": [1, 2], - "fields": [ - {"id": 1, "name": "x", "required": true, "type": "long"}, - {"id": 2, "name": "y", "required": true, "type": "long"}, - {"id": 3, "name": "z", "required": true, "type": "long"}, - {"id": 4, "name": "person", "required": false, "type": { - "type": "struct", - "fields": [ - {"id": 5, "name": "name", "required": false, "type": "string"}, - {"id": 6, "name": "age", "required": true, "type": "int"} - ] - }}, - {"id": 7, "name": "tags", "required": false, "type": { - "type": "list", - "element-id": 8, - "element": { - "type": "struct", - "fields": [ - {"id": 9, "name": "key", "required": false, "type": "string"}, - {"id": 10, "name": "value", "required": false, "type": "string"} - ] - }, - "element-required": true - }}, - {"id": 11, "name": "props", "required": false, "type": { - "type": "map", - "key-id": 12, - "key": "string", - "value-id": 13, - "value": { - "type": "struct", - "fields": [ - {"id": 14, "name": "data", "required": false, "type": "string"} - ] - }, - "value-required": true - }} - ] - } - ], - "default-spec-id": 0, - "partition-specs": [ - {"spec-id": 0, "fields": []} - ], - "last-partition-id": 999, - "default-sort-order-id": 0, - "sort-orders": [ - {"order-id": 0, "fields": []} - ], - "properties": {}, - "current-snapshot-id": -1, - "snapshots": [] - }"#; - - let reader = BufReader::new(json.as_bytes()); - let metadata = serde_json::from_reader::<_, TableMetadata>(reader).unwrap(); - - Table::builder() - .metadata(metadata) - .metadata_location("s3://bucket/test/location/metadata/v1.json".to_string()) - .identifier(TableIdent::from_strs(["ns1", "test1"]).unwrap()) - .file_io(crate::io::FileIO::new_with_memory()) - .runtime(crate::test_utils::test_runtime()) - .build() - .unwrap() +impl Move { + fn first(field_id: i32) -> Self { + Move::new(field_id, TABLE_ROOT_ID, MoveType::First) } - // ----------------------------------------------------------------------- - // Existing root-level tests - // ----------------------------------------------------------------------- + fn before(field_id: i32, reference_field_id: i32) -> Self { + Move::new(field_id, reference_field_id, MoveType::Before) + } - #[test] - fn test_assign_fresh_ids_variant() { - // Variant carries no sub-fields, so fresh-id assignment only renames the field - // itself and leaves the type untouched. - let mut next_id = 10; - let field = NestedField::optional(1, "data", Type::Variant(VariantType)); - let assigned = super::assign_fresh_ids(&field, &mut next_id); + fn after(field_id: i32, reference_field_id: i32) -> Self { + Move::new(field_id, reference_field_id, MoveType::After) + } - assert_eq!(assigned.id, 11); - assert_eq!(*assigned.field_type, Type::Variant(VariantType)); - assert_eq!(next_id, 11); + fn new(field_id: i32, reference_field_id: i32, r#type: MoveType) -> Self { + Move { + field_id, + reference_field_id, + r#type, + } } - #[tokio::test] - async fn test_add_column() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + fn field_id(&self) -> i32 { + self.field_id + } - let action = tx.update_schema().add_column(AddColumn::optional( - "new_col", - Type::Primitive(PrimitiveType::Int), - )); + fn reference_field_id(&self) -> i32 { + self.reference_field_id + } - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); - let requirements = action_commit.take_requirements(); + fn r#type(&self) -> MoveType { + self.r#type + } +} - assert_eq!(updates.len(), 2); +struct ApplyChangesVisitor<'a> { + deletes: &'a Vec, + updates: &'a HashMap, + parent_to_added_ids: &'a HashMap>, + moves: &'a HashMap>, +} - // Extract the new schema from the AddSchema update. - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; +impl SchemaVisitor for ApplyChangesVisitor<'_> { + type T = Option; - let expected_schema = table - .metadata() - .current_schema() - .as_ref() - .clone() - .into_builder() - .with_schema_id(DEFAULT_SCHEMA_ID) - .with_fields([ - NestedField::optional(4, "new_col", Type::Primitive(PrimitiveType::Int)).into(), - ]) - .build() - .unwrap(); - assert_eq!(new_schema, &expected_schema); + fn schema(&mut self, _schema: &Schema, value: Self::T) -> Result { + let added_fields: Vec = self + .parent_to_added_ids + .get(&TABLE_ROOT_ID) + .unwrap_or(&vec![]) + .iter() + .map(|id| self.updates.get(id).unwrap().clone()) + .collect(); + let fields = add_and_move_fields( + value.clone().unwrap().to_struct_type().unwrap().fields(), + &added_fields, + self.moves.get(&TABLE_ROOT_ID).unwrap_or(&vec![]), + ); + if !fields.is_empty() { + return Ok(Some(Type::Struct(StructType::new(fields)))); + } + Ok(value) + } - assert_eq!(updates[1], TableUpdate::SetCurrentSchema { schema_id: -1 }); + fn r#struct(&mut self, r#struct: &StructType, results: Vec) -> Result { + let mut has_change = false; + let mut new_fields: Vec = Vec::with_capacity(results.len()); + for (result_type, field) in results.iter().zip(r#struct.fields()) { + if result_type.is_none() { + has_change = true; + continue; + } + let result_type = result_type.clone().unwrap(); + let update = self.updates.get(&field.id); + let updated = if let Some(update) = update { + Arc::unwrap_or_clone(update.clone()).with_type(result_type) + } else { + Arc::unwrap_or_clone(field.clone()).with_type(result_type) + }; + if field.as_ref() == &updated { + new_fields.push(field.clone()); + } else { + has_change = true; + new_fields.push(updated.into()); + } + } + if has_change { + return Ok(Some(Type::Struct(StructType::new(new_fields)))); + } + Ok(Some(Type::Struct(r#struct.clone()))) + } - // Verify requirement. - assert_eq!(requirements.len(), 1); - assert_eq!(requirements[0], TableRequirement::CurrentSchemaIdMatch { - current_schema_id: table.metadata().current_schema().schema_id() - }); + fn field(&mut self, field: &NestedFieldRef, value: Self::T) -> Result { + let field_id = field.id; + // handle deletes + if self.deletes.contains(&field_id) { + return Ok(None); + } + // handle updates + let update = self.updates.get(&field_id); + if let Some(update) = update + && update.field_type.as_ref() != field.field_type.as_ref() + { + return Ok(Some(*update.field_type.clone())); + } + // handle adds + let new_fields: Vec<_> = self + .parent_to_added_ids + .get(&field_id) + .unwrap_or(&vec![]) + .iter() + .filter_map(|id| self.updates.get(id)) + .cloned() + .collect(); + let columns_to_move = self.moves.get(&field_id).cloned().unwrap_or(vec![]); + if !new_fields.is_empty() || !columns_to_move.is_empty() { + let fields = add_and_move_fields( + value.clone().unwrap().to_struct_type().unwrap().fields(), + &new_fields, + &columns_to_move, + ); + if !fields.is_empty() { + return Ok(Some(Type::Struct(StructType::new(fields)))); + } + } + Ok(value) } - #[tokio::test] - async fn test_add_column_with_doc() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + fn list(&mut self, list: &ListType, element_result: Self::T) -> Result { + let element_field = list.element_field.clone(); + let element_type = self + .field(&element_field, element_result)? + .ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete list element type from list: {:?}", list), + ))?; + let element_update = self.updates.get(&element_field.id); + let is_element_optional = if let Some(element_update) = element_update { + !element_update.required + } else { + !element_field.required + }; + let is_element_required = !is_element_optional; + if is_element_required == element_field.required + && &element_type == list.element_field.field_type.as_ref() + { + return Ok(Some(Type::List(list.clone()))); + } + if is_element_optional { + Ok(Some(Type::List(ListType::optional( + list.element_field.id, + element_type, + )))) + } else { + Ok(Some(Type::List(ListType::required( + list.element_field.id, + element_type, + )))) + } + } - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("documented_col") - .field_type(Type::Primitive(PrimitiveType::String)) - .doc("A documented column") - .build(), - ); + fn map( + &mut self, + map: &MapType, + key_result: Self::T, + value_result: Self::T, + ) -> Result { + let key_id = map.key_field.id; + if self.deletes.contains(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete map keys: {:?}", map), + )); + } else if self.updates.contains_key(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot update map keys: {:?}", map), + )); + } else if self.parent_to_added_ids.contains_key(&key_id) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot add fields to map keys: {:?}", map), + )); + } else if map.key_field.field_type.as_ref() != &key_result.unwrap() { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot alter map keys: {:?}", map), + )); + } + let value_field = map.value_field.clone(); + let value_type = self.field(&value_field, value_result)?.ok_or(Error::new( + ErrorKind::PreconditionFailed, + format!("Cannot delete value type from map: {:?}", map), + ))?; + let value_update = self.updates.get(&value_field.id); + let is_value_required = if let Some(update) = value_update { + update.required + } else { + map.value_field.required + }; + if is_value_required == map.value_field.required + && map.value_field.field_type.as_ref() == &value_type + { + return Ok(Some(Type::Map(map.clone()))); + } + if is_value_required { + Ok(Some(Type::Map(MapType::required( + map.key_field.id, + *map.key_field.field_type.clone(), + map.value_field.id, + value_type, + )))) + } else { + Ok(Some(Type::Map(MapType::optional( + map.key_field.id, + *map.key_field.field_type.clone(), + map.value_field.id, + value_type, + )))) + } + } - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + fn primitive(&mut self, p: &PrimitiveType) -> Result { + Ok(Some(Type::Primitive(p.clone()))) + } - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + fn variant(&mut self, v: &VariantType) -> Result { + Ok(Some(Type::Variant(*v))) + } +} - let field = new_schema - .field_by_name("documented_col") - .expect("documented_col should exist"); - assert_eq!(field.id, 4); - assert!(!field.required); - assert_eq!(field.doc.as_deref(), Some("A documented column")); +fn assign_fresh_ids(field_type: Type, next_id: &mut i32) -> Type { + match field_type { + Type::Primitive(_) => field_type, + Type::Struct(s) => { + let new_fields = s + .fields() + .iter() + .map(|field| { + *next_id += 1; + let new_field_id = *next_id; + let new_type = assign_fresh_ids((*field.field_type).clone(), next_id); + Arc::unwrap_or_clone(field.clone()) + .with_id(new_field_id) + .with_type(new_type) + .into() + }) + .collect(); + StructType::new(new_fields).into() + } + Type::List(list) => { + *next_id += 1; + let element_id = *next_id; + let element_type = assign_fresh_ids((*list.element_field.field_type).clone(), next_id); + ListType::new( + Arc::unwrap_or_clone(list.element_field.clone()) + .with_id(element_id) + .with_type(element_type) + .into(), + ) + .into() + } + Type::Map(map) => { + *next_id += 1; + let key_id = *next_id; + *next_id += 1; + let value_id = *next_id; + let key_type = assign_fresh_ids((*map.key_field.field_type).clone(), next_id); + let value_type = assign_fresh_ids((*map.value_field.field_type).clone(), next_id); + let key_field = Arc::unwrap_or_clone(map.key_field.clone()) + .with_id(key_id) + .with_type(key_type); + let value_field = Arc::unwrap_or_clone(map.value_field.clone()) + .with_id(value_id) + .with_type(value_type); + MapType::new(key_field.into(), value_field.into()).into() + } + Type::Variant(_) => VariantType.into(), + } +} + +fn is_promotion_allowed(from: &Type, to: &PrimitiveType) -> bool { + let from = match from { + Type::Primitive(p) => p, + _ => return false, + }; + if from == to { + return true; + } + match from { + PrimitiveType::Int => { + matches!(to, PrimitiveType::Long) + } + PrimitiveType::Float => matches!(to, PrimitiveType::Double), + PrimitiveType::Decimal { + precision: p, + scale: s, + } => { + matches!( + to, + PrimitiveType::Decimal { + precision: to_p, + scale: to_s + } if to_p >= p && to_s == s + ) + } + _ => false, + } +} + +fn add_and_move_fields( + fields: &[NestedFieldRef], + adds: &[NestedFieldRef], + moves: &[Move], +) -> Vec { + if !adds.is_empty() { + if !moves.is_empty() { + return move_fields(&add_fields(fields, adds), moves); + } + return add_fields(fields, adds); + } else if !moves.is_empty() { + return move_fields(fields, moves); + } + vec![] +} + +fn add_fields(fields: &[NestedFieldRef], adds: &[NestedFieldRef]) -> Vec { + let mut new_fields = fields.to_owned(); + new_fields.extend(adds.iter().cloned()); + new_fields +} + +fn move_fields(fields: &[NestedFieldRef], moves: &[Move]) -> Vec { + let mut reordered = fields.to_vec(); + for r#move in moves { + let idx = reordered + .iter() + .position(|f| f.id == r#move.field_id()) + .unwrap(); + let to_move = reordered.remove(idx); + match r#move.r#type() { + MoveType::First => { + reordered.insert(0, to_move); + } + MoveType::Before => { + let before_idx = reordered + .iter() + .position(|f| f.id == r#move.reference_field_id()) + .unwrap(); + reordered.insert(before_idx, to_move); + } + MoveType::After => { + let after_idx = reordered + .iter() + .position(|f| f.id == r#move.reference_field_id()) + .unwrap(); + reordered.insert(after_idx + 1, to_move); + } + } + } + reordered +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + use std::sync::{Arc, LazyLock}; + + use super::{ + AddColumn, DeleteColumn, MoveColumn, RenameColumn, UpdateColumn, UpdateSchemaAction, + }; + use crate::spec::{ + ListType, Literal, MapType, NestedField, PrimitiveLiteral, PrimitiveType, Schema, + StructType, Type, prune_columns, + }; + use crate::test_utils::{get_projected_ids_of_schema, get_projected_ids_of_type}; + use crate::{ErrorKind, Result}; + + const SCHEMA_LAST_COLUMN_ID: i32 = 23; + + static SCHEMA: LazyLock = LazyLock::new(|| { + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Float.into()).into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap() + }); + + #[test] + fn test_no_changes() { + let base = Arc::new(SCHEMA.clone()); + let expected = SCHEMA.clone(); + let updated = UpdateSchemaAction::new(base, SCHEMA_LAST_COLUMN_ID) + .unwrap() + .apply() + .unwrap(); + assert_eq!(updated.as_ref(), &expected); + } + + #[test] + fn test_delete_fields() -> Result<()> { + let columns = [ + "id", + "data", + "preferences", + "preferences.feature1", + "preferences.feature2", + "locations", + "locations.lat", + "locations.long", + "points", + "points.x", + "points.y", + "doubles", + "properties", + ]; + let all_ids: HashSet = get_projected_ids_of_schema(&SCHEMA); + for column in columns { + let mut selected = all_ids.clone(); + let nested = SCHEMA.field_by_name(column).unwrap(); + selected.remove(&nested.id); + for id in get_projected_ids_of_type(nested.field_type.as_ref()) { + selected.remove(&id); + } + let del = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID)? + .delete(DeleteColumn::new(column))? + .apply()?; + + let struct_type = prune_columns(&SCHEMA, selected, false)? + .to_struct_type() + .unwrap(); + assert_eq!(&struct_type, del.as_struct()); + } + Ok(()) + } + + #[test] + fn test_delete_fields_case_sensitive_disabled() -> Result<()> { + let columns = [ + "Id", + "Data", + "Preferences", + "Preferences.feature1", + "Preferences.feature2", + "Locations", + "Locations.lat", + "Locations.long", + "Points", + "Points.x", + "Points.y", + "Doubles", + "Properties", + ]; + let all_ids: HashSet = get_projected_ids_of_schema(&SCHEMA); + for column in columns { + let mut selected = all_ids.clone(); + let nested = SCHEMA.field_by_name_case_insensitive(column).unwrap(); + selected.remove(&nested.id); + for id in get_projected_ids_of_type(nested.field_type.as_ref()) { + selected.remove(&id); + } + let del = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID)? + .case_sensitive(false) + .delete(DeleteColumn::new(column))? + .apply()?; + + let struct_type = prune_columns(&SCHEMA, selected, false)? + .to_struct_type() + .unwrap(); + assert_eq!(&struct_type, del.as_struct()); + } + Ok(()) + } + + #[test] + fn test_update_types() -> Result<()> { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Double.into()).into(), + NestedField::required(13, "long", PrimitiveType::Double.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("id") + .with_type(PrimitiveType::Long) + .build(), + )? + .update( + UpdateColumn::builder("locations.lat") + .with_type(PrimitiveType::Double) + .build(), + )? + .update( + UpdateColumn::builder("locations.long") + .with_type(PrimitiveType::Double) + .build(), + )? + .apply()?; + assert_eq!(&expected, updated.as_ref()); + Ok(()) + } + + #[test] + fn test_update_type_preserves_other_metadata() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(35).into()) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Long.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(35).into()) + .into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .update( + UpdateColumn::builder("i") + .with_type(PrimitiveType::Long) + .build(), + )? + .apply()?; + assert_eq!(&expected, updated.as_ref()); + Ok(()) + } + + #[test] + fn test_update_doc_preserves_other_metadata() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(35).into()) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()) + .with_doc("longer description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(35).into()) + .into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .update( + UpdateColumn::builder("i") + .with_doc(Some("longer description".to_string())) + .build(), + ) + .unwrap() + .apply() + .unwrap(); + assert_eq!(&expected, updated.as_ref()); + } + + #[test] + fn test_update_default_preserves_other_metadata() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(35).into()) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::Int(34).into()) + .with_write_default(PrimitiveLiteral::Int(123456).into()) + .into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .update( + UpdateColumn::builder("i") + .with_default_value(Some(PrimitiveLiteral::Int(123456).into())) + .build(), + ) + .unwrap() + .apply() + .unwrap(); + assert_eq!(&expected, updated.as_ref()); + } + + #[test] + fn test_update_types_case_insensitive() -> Result<()> { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Double.into()).into(), + NestedField::required(13, "long", PrimitiveType::Double.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + + let updated = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .case_sensitive(false) + .update( + UpdateColumn::builder("ID") + .with_type(PrimitiveType::Long) + .build(), + )? + .update( + UpdateColumn::builder("Locations.Lat") + .with_type(PrimitiveType::Double) + .build(), + )? + .update( + UpdateColumn::builder("Locations.Long") + .with_type(PrimitiveType::Double) + .build(), + )? + .apply()?; + + assert_eq!(&expected, updated.as_ref()); + + Ok(()) + } + + #[test] + fn test_update_failure() -> Result<()> { + let allowed_updates: HashSet<(PrimitiveType, PrimitiveType)> = HashSet::from([ + (PrimitiveType::Int, PrimitiveType::Long), + (PrimitiveType::Float, PrimitiveType::Double), + ( + PrimitiveType::Decimal { + precision: 9, + scale: 2, + }, + PrimitiveType::Decimal { + precision: 18, + scale: 2, + }, + ), + ]); + let primitives = vec![ + PrimitiveType::Boolean, + PrimitiveType::Int, + PrimitiveType::Long, + PrimitiveType::Float, + PrimitiveType::Double, + PrimitiveType::Date, + PrimitiveType::Time, + PrimitiveType::Timestamp, + PrimitiveType::Timestamptz, + PrimitiveType::String, + PrimitiveType::Uuid, + PrimitiveType::Binary, + PrimitiveType::Fixed(3), + PrimitiveType::Fixed(4), + PrimitiveType::Decimal { + precision: 9, + scale: 2, + }, + PrimitiveType::Decimal { + precision: 9, + scale: 3, + }, + PrimitiveType::Decimal { + precision: 18, + scale: 2, + }, + // TODO: Geometry types and Geography types + ]; + for from in &primitives { + for to in &primitives { + let from_schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "col", from.clone().into()).into(), + ]) + .build() + .unwrap(), + ); + if from == to || allowed_updates.contains(&(from.clone(), to.clone())) { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "col", to.clone().into()).into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(from_schema, 1) + .unwrap() + .update(UpdateColumn::builder("col").with_type(to.clone()).build())? + .apply() + .unwrap(); + assert_eq!(&expected, result.as_ref()); + continue; + } + let result = UpdateSchemaAction::new(from_schema, 1) + .unwrap() + .update(UpdateColumn::builder("col").with_type(to.clone()).build()) + .and_then(|a| a.apply()); + let err = result.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + format!("Cannot promote col from type {} to type {}", from, to) + ); + } + } + Ok(()) + } + + #[test] + fn test_rename() -> Result<()> { + let renamed = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .rename( + RenameColumn::builder() + .name("data") + .new_name("json") + .build(), + )? + .rename( + RenameColumn::builder() + .name("preferences") + .new_name("options") + .build(), + )? + .rename( + RenameColumn::builder() + .name("preferences.feature2") + .new_name("newfeature") + .build(), + )? + .rename( + RenameColumn::builder() + .name("locations.lat") + .new_name("latitude") + .build(), + )? + .rename( + RenameColumn::builder() + .name("points.x") + .new_name("X") + .build(), + )? + .rename( + RenameColumn::builder() + .name("points.y") + .new_name("Y") + .build(), + )? + .apply()?; + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "json", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "options", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "newfeature", PrimitiveType::Boolean.into()) + .into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "latitude", PrimitiveType::Float.into()) + .into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "X", PrimitiveType::Long.into()).into(), + NestedField::required(16, "Y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + assert_eq!(renamed.as_ref(), &expected); + Ok(()) + } + + #[test] + fn test_rename_case_insensitive() -> Result<()> { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "json", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "options", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "newfeature", PrimitiveType::Boolean.into()) + .into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "latitude", PrimitiveType::Float.into()) + .into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "X", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y.y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + + let updated = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .case_sensitive(false) + .rename( + RenameColumn::builder() + .name("Data") + .new_name("json") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Preferences") + .new_name("options") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Preferences.Feature2") + .new_name("newfeature") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Locations.Lat") + .new_name("latitude") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Points.X") + .new_name("X") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Points.Y") + .new_name("y.y") + .build(), + )? + .rename( + RenameColumn::builder() + .name("Data") + .new_name("json") + .build(), + )? + .apply()?; + + assert_eq!(updated.as_ref(), &expected); + + Ok(()) + } + + #[test] + fn test_add_fields() -> Result<()> { + let added = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .name("topLevel") + .r#type(Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 2, + })) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("locations".to_string())) + .name("alt") + .r#type(Type::Primitive(PrimitiveType::Float)) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("points".to_string())) + .name("z") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("points".to_string())) + .name("t.t") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build(), + )? + .apply()?; + + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Float.into()).into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + NestedField::optional(25, "alt", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + NestedField::optional(26, "z", PrimitiveType::Long.into()).into(), + NestedField::optional(27, "t.t", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + NestedField::optional( + 24, + "topLevel", + PrimitiveType::Decimal { + precision: 9, + scale: 2, + } + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + assert_eq!(added.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_add_column_with_default() -> Result<()> { + let schema: Arc = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()) + .with_doc("description") + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .doc("description".into()) + .default_value(Literal::string("unknown")) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_column_with_update_column_default() -> Result<()> { + let schema: Arc = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .build(), + )? + .update( + UpdateColumn::builder("data") + .with_default_value(Some(Literal::string("unknown"))) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_nested_struct() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let struct_type = StructType::new(vec![ + NestedField::required(1, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "long", PrimitiveType::Int.into()).into(), + ]); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional( + 2, + "location", + StructType::new(vec![ + NestedField::required(3, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(4, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("location") + .r#type(Type::Struct(struct_type)) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_nested_map_of_structs() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional( + 2, + "locations", + MapType::optional( + 3, + StructType::new(vec![ + NestedField::required(5, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(6, "city", PrimitiveType::String.into()).into(), + NestedField::required(7, "state", PrimitiveType::String.into()).into(), + NestedField::required(8, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 4, + StructType::new(vec![ + NestedField::required(9, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(10, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let map = MapType::optional( + 1, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()).into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 2, + StructType::new(vec![ + NestedField::required(9, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(8, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ); + let result = UpdateSchemaAction::new(schema, 1) + .unwrap() + .add( + AddColumn::builder() + .name("locations") + .r#type(map.into()) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_nested_list_of_structs() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let list = ListType::optional( + 1, + StructType::new(vec![ + NestedField::required(9, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(8, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional( + 2, + "locations", + ListType::optional( + 3, + StructType::new(vec![ + NestedField::required(4, "lat", PrimitiveType::Int.into()).into(), + NestedField::optional(5, "long", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema, 1) + .unwrap() + .add( + AddColumn::builder() + .name("locations") + .r#type(list.into()) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_required_column_without_default() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::String("unknown".into()).into()) + .with_write_default(PrimitiveLiteral::String("unknown".into()).into()) + .into(), + ]) + .build() + .unwrap(); + + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .is_optional(false) + .r#type(PrimitiveType::String.into()) + .doc("description".into()) + .default_value(PrimitiveLiteral::String("unknown".into()).into()) + .build(), + )? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_required_column_with_default() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_doc("description") + .with_initial_default(PrimitiveLiteral::String("unknown".into()).into()) + .with_write_default(PrimitiveLiteral::String("unknown".into()).into()) + .into(), + ]) + .build() + .unwrap(); + + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .is_optional(false) + .doc("description".into()) + .default_value(PrimitiveLiteral::String("unknown".into()).into()) + .build(), + )? + .apply()?; + + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_required_column_with_update_column_default() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let err = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .is_optional(false) + .build(), + ) + .unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Incompatible change: cannot add required column without a default value: data" + ); + Ok(()) + } + + #[test] + fn test_add_required_column_case_insensitive() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap() + .into(); + let err = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .case_sensitive(false) + .allow_incompatible_changes() + .add( + AddColumn::builder() + .name("ID") + .r#type(PrimitiveType::String.into()) + .is_optional(false) + .build(), + ) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot add column, name already exists: ID"); + } + + #[test] + #[ignore = "I do not think it is valuable"] + fn test_add_multiple_required_column_case_insensitive() {} + + #[test] + fn test_make_column_optional() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .update(UpdateColumn::builder("id").with_required(false).build())? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_require_column() -> Result<()> { + let column = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + + // required to required is not an incompatible change + assert_eq!( + UpdateSchemaAction::new(Arc::new(expected.clone()), 1) + .unwrap() + .update(UpdateColumn::builder("id").with_required(true).build())? + .apply()? + .as_ref(), + &expected + ); + + let result = UpdateSchemaAction::new(Arc::new(column), 1) + .unwrap() + .allow_incompatible_changes() + .update(UpdateColumn::builder("id").with_required(true).build())? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_column_with_default_to_required_column() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()) + .with_initial_default(Literal::string("unknown")) + .with_write_default(Literal::string("unknown")) + .into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(Type::Primitive(PrimitiveType::String)) + .default_value(Literal::string("unknown")) + .build(), + )? + .update(UpdateColumn::builder("data").with_required(true).build())? + .apply()?; + assert_eq!(&expected, result.as_ref()); + Ok(()) + } + + #[test] + fn test_add_column_with_update_column_default_to_required_column() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let err = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .build(), + ) + .unwrap() + .require_column("data") + .unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot change column nullability: data: optional -> required" + ); + } + + #[test] + fn test_require_column_case_insensitive() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let result = UpdateSchemaAction::new(schema.clone(), 1) + .unwrap() + .case_sensitive(false) + .allow_incompatible_changes() + .update(UpdateColumn::builder("ID").with_required(true).build()) + .unwrap() + .apply() + .unwrap(); + assert_eq!(&expected, result.as_ref()); + } + + #[test] + fn test_mixed_changes() -> Result<()> { + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()) + .with_doc("unique id") + .into(), + NestedField::required(2, "json", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "options", + StructType::new(vec![ + NestedField::required(8, "feature1", PrimitiveType::Boolean.into()).into(), + NestedField::optional(9, "newfeature", PrimitiveType::Boolean.into()) + .into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "latitude", PrimitiveType::Double.into()) + .with_doc("latitude") + .into(), + NestedField::optional(25, "alt", PrimitiveType::Float.into()).into(), + NestedField::required(28, "description", PrimitiveType::String.into()) + .with_doc("Location description") + .into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::optional(15, "X", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y.y", PrimitiveType::Long.into()).into(), + NestedField::optional(26, "z", PrimitiveType::Long.into()).into(), + NestedField::optional(27, "t.t", PrimitiveType::Long.into()) + .with_doc("name with '.'") + .into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 24, + "toplevel", + PrimitiveType::Decimal { + precision: 9, + scale: 2, + } + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + + let updated = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .name("toplevel") + .r#type(Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 2, + })) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("locations".into())) + .name("alt") + .r#type(Type::Primitive(PrimitiveType::Float)) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("points".into())) + .name("z") + .r#type(Type::Primitive(PrimitiveType::Long)) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("points".to_string())) + .name("t.t") + .r#type(Type::Primitive(PrimitiveType::Long)) + .doc("name with '.'".into()) + .build(), + )? + .rename( + RenameColumn::builder() + .name("data") + .new_name("json") + .build(), + )? + .rename( + RenameColumn::builder() + .name("preferences") + .new_name("options") + .build(), + )? + .rename( + RenameColumn::builder() + .name("preferences.feature2") + .new_name("newfeature") + .build(), + )? + .rename( + RenameColumn::builder() + .name("locations.lat") + .new_name("latitude") + .build(), + )? + .rename( + RenameColumn::builder() + .name("points.x") + .new_name("X") + .build(), + )? + .rename( + RenameColumn::builder() + .name("points.y") + .new_name("y.y") + .build(), + )? + .update( + UpdateColumn::builder("id") + .with_type(PrimitiveType::Long) + .build(), + )? + .update( + UpdateColumn::builder("id") + .with_doc(Some("unique id".into())) + .build(), + )? + .update( + UpdateColumn::builder("locations.lat") + .with_type(PrimitiveType::Double) + .build(), + )? + .update( + UpdateColumn::builder("locations.lat") + .with_doc(Some("latitude".into())) + .build(), + )? + .delete(DeleteColumn::new("locations.long"))? + .delete(DeleteColumn::new("properties"))? + .update( + UpdateColumn::builder("points.x") + .with_required(false) + .build(), + )? + .allow_incompatible_changes() + .update(UpdateColumn::builder("data").with_required(true).build())? + .add( + AddColumn::builder() + .parent(Some("locations".into())) + .name("description") + .r#type(Type::Primitive(PrimitiveType::String)) + .is_optional(false) + .doc("Location description".into()) + .build(), + )? + .apply()?; + + assert_eq!(&expected, updated.as_ref()); + Ok(()) + } + + #[test] + fn test_ambiguous_add() { + // preferences.booleans could be top-level or a field of preferences + let result = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .name("preferences.booleans") + .r#type(PrimitiveType::Boolean.into()) + .build(), + ) + .and_then(|a| a.apply()); + let err = result.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!( + err.message() + .starts_with("Cannot add column with ambiguous name: preferences.booleans") + ); + } + + #[test] + fn test_add_already_exists() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .parent(Some("preferences".to_string())) + .name("feature1") + .r#type(PrimitiveType::Boolean.into()) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot add column, name already exists: preferences.feature1" + ); + + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .name("preferences") + .r#type(PrimitiveType::Boolean.into()) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot add column, name already exists: preferences" + ); + } + + #[test] + fn test_delete_then_add() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(2, "id", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .delete(DeleteColumn::new("id"))? + .add( + AddColumn::builder() + .name("id") + .r#type(PrimitiveType::Int.into()) + .build(), + )? + .apply()?; + assert_eq!(updated.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_delete_then_add_nested() -> Result<()> { + let expected_nested = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "preferences", + StructType::new(vec![ + NestedField::optional(9, "feature2", PrimitiveType::Boolean.into()).into(), + NestedField::optional(24, "feature1", PrimitiveType::Boolean.into()).into(), + ]) + .into(), + ) + .with_doc("struct of named boolean options") + .into(), + NestedField::required( + 4, + "locations", + MapType::required( + 10, + StructType::new(vec![ + NestedField::required(20, "address", PrimitiveType::String.into()) + .into(), + NestedField::required(21, "city", PrimitiveType::String.into()).into(), + NestedField::required(22, "state", PrimitiveType::String.into()).into(), + NestedField::required(23, "zip", PrimitiveType::Int.into()).into(), + ]) + .into(), + 11, + StructType::new(vec![ + NestedField::required(12, "lat", PrimitiveType::Float.into()).into(), + NestedField::required(13, "long", PrimitiveType::Float.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("map of address to coordinate") + .into(), + NestedField::optional( + 5, + "points", + ListType::optional( + 14, + StructType::new(vec![ + NestedField::required(15, "x", PrimitiveType::Long.into()).into(), + NestedField::required(16, "y", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ) + .with_doc("2-D cartesian points") + .into(), + NestedField::required( + 6, + "doubles", + ListType::required(17, PrimitiveType::Double.into()).into(), + ) + .into(), + NestedField::optional( + 7, + "properties", + MapType::optional( + 18, + PrimitiveType::String.into(), + 19, + PrimitiveType::String.into(), + ) + .into(), + ) + .with_doc("string map of properties") + .into(), + ]) + .build() + .unwrap(); + + let updated_nested = + UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("preferences.feature1"))? + .add( + AddColumn::builder() + .parent(Some("preferences".to_string())) + .name("feature1") + .r#type(PrimitiveType::Boolean.into()) + .build(), + )? + .apply()?; + assert_eq!(updated_nested.as_struct(), expected_nested.as_struct()); + Ok(()) + } + + #[test] + fn test_delete_missing_column() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("col")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot delete missing column: col"); + } + + #[test] + fn test_add_delete_conflict() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .name("col") + .r#type(PrimitiveType::Int.into()) + .build(), + ) + .unwrap() + .delete(DeleteColumn::new("col")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot delete missing column: col"); + + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .parent(Some("preferences".to_string())) + .name("feature3") + .r#type(PrimitiveType::Int.into()) + .build(), + ) + .unwrap() + .delete(DeleteColumn::new("preferences")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot delete a column that has additions: preferences" + ); + } + + #[test] + fn test_rename_missing_column() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .rename(RenameColumn::builder().name("col").new_name("fail").build()) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot rename missing column: col"); + } + + #[test] + fn test_rename_delete_conflict() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .rename(RenameColumn::builder().name("id").new_name("col").build()) + .unwrap() + .delete(DeleteColumn::new("id")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot delete a column that has updates: id"); + + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .rename(RenameColumn::builder().name("id").new_name("col").build()) + .unwrap() + .delete(DeleteColumn::new("col")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot delete missing column: col"); + } + + #[test] + fn test_delete_rename_conflict() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("id")) + .unwrap() + .rename( + RenameColumn::builder() + .name("id") + .new_name("identifier") + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot rename a column that will be deleted: id" + ); + } + + #[test] + fn test_update_missing_column() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("col") + .with_type(PrimitiveType::Date) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot update missing column: col"); + } + + #[test] + fn test_update_missing_column_doc() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("col") + .with_doc(Some("description".into())) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot update missing column: col"); + } + + #[test] + fn test_update_missing_column_default_value() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("col") + .with_default_value(Some(Literal::int(34))) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot update missing column: col"); + } + + #[test] + fn test_update_delete_conflict() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("id") + .with_type(PrimitiveType::Long) + .build(), + ) + .unwrap() + .delete(DeleteColumn::new("id")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot delete a column that has updates: id"); + } + + #[test] + fn test_delete_update_conflict() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("id")) + .unwrap() + .update( + UpdateColumn::builder("id") + .with_type(PrimitiveType::Long) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot update column that will be deleted: id" + ); + } + + #[test] + fn test_delete_map_key() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("locations.key")) + .unwrap() + .apply() + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!(err.message().starts_with("Cannot delete map keys")); + } + + #[test] + fn test_delete_map_value() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("locations.value")) + .unwrap() + .apply() + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!( + err.message() + .starts_with("Cannot delete value type from map") + ); + } + + #[test] + fn test_add_field_to_map_key() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .add( + AddColumn::builder() + .parent(Some("locations.key".to_string())) + .name("address_line_2") + .r#type(PrimitiveType::String.into()) + .build(), + ) + .unwrap() + .apply() + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!(err.message().starts_with("Cannot add fields to map keys")); + } + + #[test] + fn test_alter_map_key() { + let err = UpdateSchemaAction::new(Arc::new(SCHEMA.clone()), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .update( + UpdateColumn::builder("locations.key.zip") + .with_type(PrimitiveType::Long) + .build(), + ) + .unwrap() + .apply() + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!(err.message().starts_with("Cannot alter map keys")); + } + + #[test] + fn test_update_map_key() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required( + 1, + "m", + MapType::optional( + 2, + PrimitiveType::Int.into(), + 3, + PrimitiveType::Double.into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let err = UpdateSchemaAction::new(schema, 3) + .unwrap() + .update( + UpdateColumn::builder("m.key") + .with_type(PrimitiveType::Long) + .build(), + ) + .unwrap() + .apply() + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert!(err.message().starts_with("Cannot update map keys")); + } + + #[test] + fn test_update_added_column_type() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "value", PrimitiveType::Long.into()).into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .add( + AddColumn::builder() + .name("value") + .r#type(PrimitiveType::Int.into()) + .build(), + )? + .update( + UpdateColumn::builder("value") + .with_type(PrimitiveType::Long) + .build(), + )? + .apply()?; + assert_eq!(updated.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_update_added_column_doc() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()).into(), + NestedField::optional(2, "value", PrimitiveType::Long.into()) + .with_doc("a value") + .into(), + ]) + .build() + .unwrap(); + let updated = UpdateSchemaAction::new(schema, 1) + .unwrap() + .add( + AddColumn::builder() + .name("value") + .r#type(PrimitiveType::Long.into()) + .build(), + )? + .update( + UpdateColumn::builder("value") + .with_doc(Some("a value".into())) + .build(), + )? + .apply()?; + assert_eq!(updated.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_update_deleted_column_doc() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "i", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let err = UpdateSchemaAction::new(schema, 3) + .unwrap() + .delete(DeleteColumn::new("i")) + .unwrap() + .update( + UpdateColumn::builder("i") + .with_doc(Some("a value".into())) + .build(), + ) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot update column that will be deleted: i" + ); + } + + #[test] + fn test_multiple_moves() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "a", PrimitiveType::Int.into()).into(), + NestedField::required(2, "b", PrimitiveType::Int.into()).into(), + NestedField::required(3, "c", PrimitiveType::Int.into()).into(), + NestedField::required(4, "d", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(3, "c", PrimitiveType::Int.into()).into(), + NestedField::required(2, "b", PrimitiveType::Int.into()).into(), + NestedField::required(4, "d", PrimitiveType::Int.into()).into(), + NestedField::required(1, "a", PrimitiveType::Int.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .move_column(MoveColumn::first("d"))? + .move_column(MoveColumn::first("c"))? + .move_column(MoveColumn::after("b", "d"))? + .move_column(MoveColumn::before("d", "a"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_top_level_column_first() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::first("data"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_top_level_column_before_first() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::before("data", "id"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_top_level_column_after_last() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::after("id", "data"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_top_level_column_after() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 3) + .unwrap() + .move_column(MoveColumn::after("ts", "id"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_top_level_column_before() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 3) + .unwrap() + .move_column(MoveColumn::before("ts", "data"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(4, "data", Types.StringType.get()), + required(3, "count", Types.LongType.get())))); + + Schema actual = new SchemaUpdate(schema, 4).moveFirst("struct.data").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_nested_field_first() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .move_column(MoveColumn::first("struct.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(4, "data", Types.StringType.get()), + required(3, "count", Types.LongType.get())))); + + Schema actual = new SchemaUpdate(schema, 4).moveBefore("struct.data", "struct.count").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_nested_field_before_first() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .move_column(MoveColumn::before("struct.data", "struct.count")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(4, "data", Types.StringType.get()), + required(3, "count", Types.LongType.get())))); + + Schema actual = new SchemaUpdate(schema, 4).moveAfter("struct.count", "struct.data").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_nested_field_after_last() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .move_column(MoveColumn::after("struct.count", "struct.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()), + optional(5, "ts", Types.TimestampType.withZone())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + optional(5, "ts", Types.TimestampType.withZone()), + required(4, "data", Types.StringType.get())))); + + Schema actual = new SchemaUpdate(schema, 5).moveAfter("struct.ts", "struct.count").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_nested_field_after() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 5) + .unwrap() + .move_column(MoveColumn::after("struct.ts", "struct.count")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + optional(5, "ts", Types.TimestampType.withZone()), + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + optional(5, "ts", Types.TimestampType.withZone()), + required(4, "data", Types.StringType.get())))); + + Schema actual = new SchemaUpdate(schema, 5).moveBefore("struct.ts", "struct.data").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_nested_field_before() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 5) + .unwrap() + .move_column(MoveColumn::before("struct.ts", "struct.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "list", + Types.ListType.ofOptional( + 6, + Types.StructType.of( + optional(5, "ts", Types.TimestampType.withZone()), + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()))))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "list", + Types.ListType.ofOptional( + 6, + Types.StructType.of( + required(3, "count", Types.LongType.get()), + optional(5, "ts", Types.TimestampType.withZone()), + required(4, "data", Types.StringType.get()))))); + + Schema actual = new SchemaUpdate(schema, 6).moveBefore("list.ts", "list.data").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_list_element_field() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "list", + ListType::required( + 6, + StructType::new(vec![ + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()) + .into(), + NestedField::required(3, "count", PrimitiveType::Long.into()) + .into(), + NestedField::required(4, "data", PrimitiveType::String.into()) + .into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "list", + ListType::required( + 6, + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 6) + .unwrap() + .move_column(MoveColumn::before("list.ts", "list.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + #[test] + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "map", + Types.MapType.ofOptional( + 6, + 7, + Types.StringType.get(), + Types.StructType.of( + optional(5, "ts", Types.TimestampType.withZone()), + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()))))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "map", + Types.MapType.ofOptional( + 6, + 7, + Types.StringType.get(), + Types.StructType.of( + required(3, "count", Types.LongType.get()), + optional(5, "ts", Types.TimestampType.withZone()), + required(4, "data", Types.StringType.get()))))); + + Schema actual = new SchemaUpdate(schema, 7).moveBefore("map.ts", "map.data").apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + fn test_move_map_value_struct_field() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "map", + MapType::optional( + 6, + PrimitiveType::String.into(), + 7, + StructType::new(vec![ + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "map", + MapType::optional( + 6, + PrimitiveType::String.into(), + 7, + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 7) + .unwrap() + .move_column(MoveColumn::before("map.ts", "map.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_ref(), &expected); + } + + #[test] + fn test_move_added_top_level_column() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 2) + .unwrap() + .add( + AddColumn::builder() + .name("ts") + .r#type(PrimitiveType::Timestamptz.into()) + .build(), + )? + .move_column(MoveColumn::after("ts", "id"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_move_added_top_level_column_after_added_column() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional(3, "ts", PrimitiveType::Timestamptz.into()).into(), + NestedField::optional(4, "count", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 2) + .unwrap() + .add( + AddColumn::builder() + .name("ts") + .r#type(PrimitiveType::Timestamptz.into()) + .build(), + )? + .add( + AddColumn::builder() + .name("count") + .r#type(PrimitiveType::Long.into()) + .build(), + )? + .move_column(MoveColumn::after("ts", "id"))? + .move_column(MoveColumn::after("count", "ts"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + optional(5, "ts", Types.TimestampType.withZone()), + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + + Schema actual = + new SchemaUpdate(schema, 4) + .addColumn("struct", "ts", Types.TimestampType.withZone()) + .moveBefore("struct.ts", "struct.count") + .apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + fn test_move_added_nested_struct_field() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .add( + AddColumn::builder() + .name("ts") + .r#type(PrimitiveType::Timestamp.into()) + .parent(Some("struct".into())) + .build(), + ) + .unwrap() + .move_column(MoveColumn::before("struct.ts", "struct.count")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + optional(6, "size", Types.LongType.get()), + optional(5, "ts", Types.TimestampType.withZone()), + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get())))); + + Schema actual = + new SchemaUpdate(schema, 4) + .addColumn("struct", "ts", Types.TimestampType.withZone()) + .addColumn("struct", "size", Types.LongType.get()) + .moveBefore("struct.ts", "struct.count") + .moveBefore("struct.size", "struct.ts") + .apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_added_nested_struct_field_before_added_column() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::optional(6, "size", PrimitiveType::Long.into()).into(), + NestedField::optional(5, "ts", PrimitiveType::Timestamp.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 4) + .unwrap() + .add( + AddColumn::builder() + .name("ts") + .r#type(PrimitiveType::Timestamp.into()) + .parent(Some("struct".into())) + .build(), + ) + .unwrap() + .add( + AddColumn::builder() + .name("size") + .r#type(PrimitiveType::Long.into()) + .parent(Some("struct".into())) + .build(), + ) + .unwrap() + .move_column(MoveColumn::before("struct.ts", "struct.count")) + .unwrap() + .move_column(MoveColumn::before("struct.size", "struct.ts")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } + + #[test] + #[ignore = "not yet implemented: self-reference move check"] + fn test_move_self_reference_fails() {} + + #[test] + fn test_move_missing_column_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema.clone(), 2) + .unwrap() + .move_column(MoveColumn::first("items")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: items"); + + let err = UpdateSchemaAction::new(schema.clone(), 2) + .unwrap() + .move_column(MoveColumn::before("items", "id")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: items"); + + let err = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::after("items", "data")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: items"); + } + + #[test] + fn test_move_before_add_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema.clone(), 2) + .unwrap() + .move_column(MoveColumn::first("ts")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: ts"); + + let err = UpdateSchemaAction::new(schema.clone(), 2) + .unwrap() + .move_column(MoveColumn::before("ts", "id")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: ts"); + + let err = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::after("ts", "data")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move missing column: ts"); + } + + #[test] + fn test_move_missing_reference_column_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema.clone(), 2) + .unwrap() + .move_column(MoveColumn::before("id", "items")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot move relative to missing column: items" + ); + + let err = UpdateSchemaAction::new(schema, 2) + .unwrap() + .move_column(MoveColumn::after("data", "items")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot move relative to missing column: items" + ); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required(2, "data", Types.StringType.get()), + optional( + 3, + "map", + Types.MapType.ofRequired(4, 5, Types.StringType.get(), Types.StringType.get()))); + + assertThatThrownBy(() -> new SchemaUpdate(schema, 5).moveBefore("map.key", "map.value").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: map"); + */ + #[test] + fn test_move_primitive_map_key_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "map", + MapType::optional( + 4, + PrimitiveType::String.into(), + 5, + PrimitiveType::String.into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema, 5) + .unwrap() + .move_column(MoveColumn::before("map.key", "map.value")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move fields in non-struct type: map"); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required(2, "data", Types.StringType.get()), + optional( + 3, + "map", + Types.MapType.ofRequired(4, 5, Types.StringType.get(), Types.StructType.of()))); + + assertThatThrownBy(() -> new SchemaUpdate(schema, 5).moveBefore("map.value", "map.key").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: map>"); + */ + #[test] + fn test_move_primitive_map_value_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "map", + MapType::optional( + 4, + PrimitiveType::String.into(), + 5, + StructType::new(vec![]).into(), + ) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema, 5) + .unwrap() + .move_column(MoveColumn::before("map.value", "map.key")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move fields in non-struct type: map"); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required(2, "data", Types.StringType.get()), + optional(3, "list", Types.ListType.ofRequired(4, Types.StringType.get()))); + + assertThatThrownBy(() -> new SchemaUpdate(schema, 4).moveBefore("list.element", "list").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: list"); + */ + #[test] + fn test_move_primitive_list_element_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::optional( + 3, + "list", + ListType::optional(4, PrimitiveType::String.into()).into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema, 4) + .unwrap() + .move_column(MoveColumn::before("list.element", "list")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move fields in non-struct type: list"); + } + + /* + Schema schema = + new Schema( + required(1, "a", Types.IntegerType.get()), + required(2, "b", Types.IntegerType.get()), + required( + 3, + "struct", + Types.StructType.of( + required(4, "x", Types.IntegerType.get()), + required(5, "y", Types.IntegerType.get())))); + + assertThatThrownBy(() -> new SchemaUpdate(schema, 5).moveBefore("a", "struct.x").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move field a to a different struct"); + */ + #[test] + fn test_move_top_level_between_structs_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "a", PrimitiveType::Int.into()).into(), + NestedField::required(2, "b", PrimitiveType::Int.into()).into(), + NestedField::required( + 3, + "struct", + StructType::new(vec![ + NestedField::required(4, "x", PrimitiveType::Int.into()).into(), + NestedField::required(5, "y", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema, 5) + .unwrap() + .move_column(MoveColumn::before("a", "struct.x")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!(err.message(), "Cannot move field a to a different struct"); + } + + /* + Schema schema = + new Schema( + required( + 1, + "s1", + Types.StructType.of( + required(3, "a", Types.IntegerType.get()), + required(4, "b", Types.IntegerType.get()))), + required( + 2, + "s2", + Types.StructType.of( + required(5, "x", Types.IntegerType.get()), + required(6, "y", Types.IntegerType.get())))); + + assertThatThrownBy(() -> new SchemaUpdate(schema, 6).moveBefore("s2.x", "s1.a").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move field s2.x to a different struct"); + */ + #[test] + fn test_move_between_structs_fails() { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required( + 1, + "s1", + StructType::new(vec![ + NestedField::required(3, "a", PrimitiveType::Int.into()).into(), + NestedField::required(4, "b", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + NestedField::required( + 2, + "s2", + StructType::new(vec![ + NestedField::required(5, "x", PrimitiveType::Int.into()).into(), + NestedField::required(6, "y", PrimitiveType::Int.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(), + ); + + let err = UpdateSchemaAction::new(schema, 6) + .unwrap() + .move_column(MoveColumn::before("s2.x", "s1.a")) + .err() + .unwrap(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot move field s2.x to a different struct" + ); + } + + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("add an existing field as identifier field should succeed") + .containsExactly(newSchema.findField("id").fieldId()); + */ + #[test] + fn test_add_existing_identifier_fields() { + let schema: Arc<_> = SCHEMA.clone().into(); + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() + ); } - #[tokio::test] - async fn test_add_required_column_with_initial_default() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn("new_field", Types.StringType.get()) + .setIdentifierFields("id", "new_field") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("add column then set as identifier should succeed") + .containsExactly( + newSchema.findField("id").fieldId(), newSchema.findField("new_field").fieldId()); + + newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .setIdentifierFields("id", "new_field") + .addRequiredColumn("new_field", Types.StringType.get()) + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("set identifier then add column should succeed") + .containsExactly( + newSchema.findField("id").fieldId(), newSchema.findField("new_field").fieldId()); + */ + #[test] + fn test_add_new_identifier_field_columns() { + let schema: Arc<_> = SCHEMA.clone().into(); + let id_field_id = schema.clone().field_by_name("id").unwrap().id; - let action = tx.update_schema().add_column(AddColumn::required( - "req_col", - Type::Primitive(PrimitiveType::Int), - Literal::int(0), - )); + // Test: add column then set as identifier should succeed + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "new_field", + PrimitiveType::String.into(), + )) + .unwrap() + .set_identifier_fields(vec!["id".to_string(), "new_field".to_string()]) + .apply() + .unwrap(); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + let new_field_id = new_schema.field_by_name("new_field").unwrap().id; + let identifier_ids: HashSet = new_schema.identifier_field_ids().collect(); + assert_eq!( + identifier_ids, + vec![id_field_id, new_field_id].into_iter().collect() + ); - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + // Test: set identifier then add column should succeed + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .set_identifier_fields(vec!["id".to_string(), "new_field".to_string()]) + .add(AddColumn::required( + "new_field", + PrimitiveType::String.into(), + )) + .unwrap() + .apply() + .unwrap(); - let field = new_schema - .field_by_name("req_col") - .expect("req_col should exist"); - assert_eq!(field.id, 4); - assert!(field.required); - assert_eq!(field.initial_default, Some(Literal::int(0))); - assert_eq!(field.write_default, Some(Literal::int(0))); - } - - #[tokio::test] - async fn test_add_column_name_conflict_fails() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - - // "x" already exists in the V2 test schema. - let action = tx.update_schema().add_column(AddColumn::optional( - "x", - Type::Primitive(PrimitiveType::Int), - )); - - let result = Arc::new(action).commit(&table).await; - let err = match result { - Err(e) => e, - Ok(_) => panic!("should reject adding a column with an existing name"), - }; - assert_eq!(err.kind(), ErrorKind::PreconditionFailed); - assert!( - err.message().contains("already exists"), - "error should mention name conflict, got: {}", - err.message() + let new_field_id = new_schema.field_by_name("new_field").unwrap().id; + let identifier_ids: HashSet = new_schema.identifier_field_ids().collect(); + assert_eq!( + identifier_ids, + vec![id_field_id, new_field_id].into_iter().collect() ); } - #[tokio::test] - async fn test_delete_column() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn( + "required_struct", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 2, "field", Types.StringType.get()))) + .apply(); + + newSchema = + new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID + 2) + .setIdentifierFields("required_struct.field") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("set existing nested field as identifier should succeed") + .containsExactly(newSchema.findField("required_struct.field").fieldId()); + + newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn( + "new", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 2, "field", Types.StringType.get()))) + .setIdentifierFields("new.field") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("set newly added nested field as identifier should succeed") + .containsExactly(newSchema.findField("new.field").fieldId()); + + newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn( + "new", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 2, + "field", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 3, "nested", Types.StringType.get()))))) + .setIdentifierFields("new.field.nested") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("set newly added multi-layer nested field as identifier should succeed") + .containsExactly(newSchema.findField("new.field.nested").fieldId()); + */ + #[test] + fn test_add_nested_identifier_field_columns() { + let schema: Arc<_> = SCHEMA.clone().into(); - // z is not an identifier field, so we can delete it. - let action = tx.update_schema().delete_column("z"); + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "required_struct", + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 2, + "field", + PrimitiveType::String.into(), + ) + .into(), + ])), + )) + .unwrap() + .apply() + .unwrap(); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + let new_schema = UpdateSchemaAction::new(new_schema.clone(), SCHEMA_LAST_COLUMN_ID + 2) + .unwrap() + .set_identifier_fields(vec!["required_struct.field".to_string()]) + .apply() + .unwrap(); - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + let identifier_ids: Vec = new_schema.identifier_field_ids().collect(); + assert_eq!(identifier_ids, vec![ + new_schema + .field_by_name("required_struct.field") + .unwrap() + .id + ]); - assert!( - new_schema.field_by_name("z").is_none(), - "z should be deleted" - ); - assert!(new_schema.field_by_name("x").is_some()); - assert!(new_schema.field_by_name("y").is_some()); - } + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "new", + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 2, + "field", + PrimitiveType::String.into(), + ) + .into(), + ])), + )) + .unwrap() + .set_identifier_fields(vec!["new.field".to_string()]) + .apply() + .unwrap(); - #[tokio::test] - async fn test_delete_missing_column_fails() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + let new_field_id = new_schema.field_by_name("new.field").unwrap().id; + let identifier_ids: Vec = new_schema.identifier_field_ids().collect(); + assert_eq!(identifier_ids, vec![new_field_id]); - let action = tx.update_schema().delete_column("nonexistent"); + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "new", + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 2, + "field", + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 3, + "nested", + PrimitiveType::String.into(), + ) + .into(), + ])), + ) + .into(), + ])), + )) + .unwrap() + .set_identifier_fields(vec!["new.field.nested".to_string()]) + .apply() + .unwrap(); - let result = Arc::new(action).commit(&table).await; - let err = match result { - Err(e) => e, - Ok(_) => panic!("should reject deleting a non-existent column"), - }; - assert_eq!(err.kind(), ErrorKind::PreconditionFailed); - assert!( - err.message().contains("nonexistent"), - "error should mention the missing column, got: {}", - err.message() - ); + let nested_field_id = new_schema.field_by_name("new.field.nested").unwrap().id; + let identifier_ids: Vec = new_schema.identifier_field_ids().collect(); + assert_eq!(identifier_ids, vec![nested_field_id]); } - #[tokio::test] - async fn test_add_and_delete_combined() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - - // Delete z, add a new column. - let action = tx - .update_schema() - .delete_column("z") - .add_column(AddColumn::optional( - "w", - Type::Primitive(PrimitiveType::Boolean), - )); - - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn(null, "dot.field", Types.StringType.get()) + .setIdentifierFields("id", "dot.field") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("add a field with dot as identifier should succeed") + .containsExactly( + newSchema.findField("id").fieldId(), newSchema.findField("dot.field").fieldId()); + */ + #[test] + fn test_add_dotted_identifier_field_columns() { + let schema: Arc<_> = SCHEMA.clone().into(); - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + let id_field_id = schema.field_by_name("id").unwrap().id; - assert!( - new_schema.field_by_name("z").is_none(), - "z should be deleted" - ); - let w = new_schema.field_by_name("w").expect("w should exist"); - assert_eq!(w.id, 4); - assert!(!w.required); - } - - #[tokio::test] - async fn test_delete_and_readd_same_name() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - - // Delete z, then add a new column named z -- should succeed. - let action = tx - .update_schema() - .delete_column("z") - .add_column(AddColumn::optional( - "z", - Type::Primitive(PrimitiveType::Boolean), - )); + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required("dot.field", PrimitiveType::String.into()).parent(None)) + .unwrap() + .set_identifier_fields(vec!["id".to_string(), "dot.field".to_string()]) + .apply() + .unwrap(); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + let dot_field_id = new_schema.field_by_name("dot.field").unwrap().id; - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + let identifier_ids: HashSet = new_schema.identifier_field_ids().collect(); + assert_eq!( + identifier_ids, + vec![id_field_id, dot_field_id].into_iter().collect() + ); + } - let z = new_schema - .field_by_name("z") - .expect("z should exist with new type"); - assert_eq!(z.id, 4); // new ID, not the old 3 - assert_eq!(*z.field_type, Type::Primitive(PrimitiveType::Boolean)); + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn("new_field", Types.StringType.get()) + .addRequiredColumn("new_field2", Types.StringType.get()) + .setIdentifierFields("id", "new_field", "new_field2") + .apply(); + + newSchema = + new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("new_field", "new_field2") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("remove an identifier field should succeed") + .containsExactly( + newSchema.findField("new_field").fieldId(), + newSchema.findField("new_field2").fieldId()); + + newSchema = + new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields(Sets.newHashSet()) + .apply(); + + assertThat(newSchema.identifierFieldIds()).isEmpty(); + */ + #[test] + fn test_remove_identifier_fields() { + let schema: Arc<_> = SCHEMA.clone().into(); + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + let new_schema = UpdateSchemaAction::new(new_schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec![]) + .apply() + .unwrap(); + assert!( + new_schema + .identifier_field_ids() + .collect::>() + .is_empty() + ); } + /* + Schema testSchema = + new Schema( + optional(1, "id", Types.IntegerType.get()), + required(2, "float", Types.FloatType.get()), + required(3, "double", Types.DoubleType.get())); + + assertThatThrownBy(() -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(999))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add fieldId 999 as an identifier field: field does not exist"); + + assertThatThrownBy(() -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(1))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add field id as an identifier field: not a required field"); + + assertThatThrownBy(() -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(2))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field float as an identifier field: must not be float or double field"); + + assertThatThrownBy(() -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(3))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field double as an identifier field: must not be float or double field"); + + assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("unknown") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field unknown as an identifier field: not found in current schema or added columns"); + + assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("locations") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field locations as an identifier field: not a primitive type field"); + + assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("data").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add field data as an identifier field: not a required field"); + + assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("locations.key.zip") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field zip as an identifier field: must not be nested in " + + SCHEMA.findField("locations")); + + assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("points.element.x") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field x as an identifier field: must not be nested in " + + SCHEMA.findField("points")); + + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn("col_float", Types.FloatType.get()) + .addRequiredColumn("col_double", Types.DoubleType.get()) + .addRequiredColumn( + "new", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 4, + "fields", + Types.ListType.ofRequired( + SCHEMA_LAST_COLUMN_ID + 5, + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 6, + "nested", + Types.StringType.get())))))) + .addRequiredColumn( + "new_map", + Types.MapType.ofRequired( + SCHEMA_LAST_COLUMN_ID + 8, + SCHEMA_LAST_COLUMN_ID + 9, + Types.StructType.of( + required(SCHEMA_LAST_COLUMN_ID + 10, "key_col", Types.StringType.get())), + Types.StructType.of( + required(SCHEMA_LAST_COLUMN_ID + 11, "val_col", Types.StringType.get()))), + "map of address to coordinate") + .addRequiredColumn( + "required_list", + Types.ListType.ofRequired( + SCHEMA_LAST_COLUMN_ID + 13, + Types.StructType.of( + required(SCHEMA_LAST_COLUMN_ID + 14, "x", Types.LongType.get()), + required(SCHEMA_LAST_COLUMN_ID + 15, "y", Types.LongType.get())))) + .apply(); + + int lastColId = SCHEMA_LAST_COLUMN_ID + 15; + + assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("required_list.element.x") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field x as an identifier field: must not be nested in " + + newSchema.findField("required_list")); + + assertThatThrownBy( + () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_double").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field col_double as an identifier field: must not be float or double field"); + + assertThatThrownBy( + () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_float").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field col_float as an identifier field: must not be float or double field"); + + assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("new_map.value.val_col") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field val_col as an identifier field: must not be nested in " + + newSchema.findField("new_map")); + + assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("new.fields.element.nested") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field nested as an identifier field: must not be nested in " + + newSchema.findField("new.fields")); + + assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("preferences.feature1") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field feature1 as an identifier field: must not be nested in an optional field " + + newSchema.findField("preferences")); + */ #[test] - fn test_apply() { - let table = make_v2_table(); - let tx = Transaction::new(&table); + fn test_set_identifier_fields_fails() { + let schema: Arc<_> = SCHEMA.clone().into(); - let tx = tx - .update_schema() - .add_column(AddColumn::optional( - "new_col", - Type::Primitive(PrimitiveType::Int), + let new_schema = UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "col_float", + PrimitiveType::Float.into(), + )) + .unwrap() + .add(AddColumn::required( + "col_double", + PrimitiveType::Double.into(), + )) + .unwrap() + .add(AddColumn::required( + "new", + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 4, + "fields", + Type::List(ListType::required( + SCHEMA_LAST_COLUMN_ID + 5, + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 6, + "nested", + PrimitiveType::String.into(), + ) + .into(), + ])), + )), + ) + .into(), + ])), )) - .apply(tx) + .unwrap() + .add(AddColumn::required( + "new_map", + Type::Map(MapType::required( + SCHEMA_LAST_COLUMN_ID + 8, + PrimitiveType::String.into(), + SCHEMA_LAST_COLUMN_ID + 9, + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 11, + "val_col", + PrimitiveType::String.into(), + ) + .into(), + ])), + )), + )) + .unwrap() + .add(AddColumn::required( + "required_list", + Type::List(ListType::required( + SCHEMA_LAST_COLUMN_ID + 13, + Type::Struct(StructType::new(vec![ + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 14, + "x", + PrimitiveType::Long.into(), + ) + .into(), + NestedField::required( + SCHEMA_LAST_COLUMN_ID + 15, + "y", + PrimitiveType::Long.into(), + ) + .into(), + ])), + )), + )) + .unwrap() + .apply() .unwrap(); - assert_eq!(tx.actions.len(), 1); - (*tx.actions[0]) - .downcast_ref::() - .expect("UpdateSchemaAction was not applied to Transaction!"); - } - - // ----------------------------------------------------------------------- - // Nested add tests - // ----------------------------------------------------------------------- + let last_col_id = SCHEMA_LAST_COLUMN_ID + 15; - #[tokio::test] - async fn test_add_column_to_struct() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + let err = UpdateSchemaAction::new(new_schema.clone(), last_col_id) + .unwrap() + .set_identifier_fields(vec!["required_list.element.x".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + format!( + "Cannot add field x as an identifier field: must not be nested in {:?}", + new_schema.field_by_name("required_list").unwrap() + ) + ); - // Add "email" to the "person" struct. - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("email") - .field_type(Type::Primitive(PrimitiveType::String)) - .parent("person") - .build(), + let err = UpdateSchemaAction::new(new_schema.clone(), last_col_id) + .unwrap() + .set_identifier_fields(vec!["col_double".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + "Cannot add identifier field col_double: cannot be a float or double type" ); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + let err = UpdateSchemaAction::new(new_schema.clone(), last_col_id) + .unwrap() + .set_identifier_fields(vec!["col_float".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + "Cannot add identifier field col_float: cannot be a float or double type" + ); - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + let err = UpdateSchemaAction::new(new_schema.clone(), last_col_id) + .unwrap() + .set_identifier_fields(vec!["new_map.value.val_col".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + format!( + "Cannot add field val_col as an identifier field: must not be nested in {:?}", + new_schema.field_by_name("new_map").unwrap() + ) + ); - // "email" should be nested under "person" with ID = last_column_id + 1 = 15. - let email = new_schema - .field_by_name("person.email") - .expect("person.email should exist"); - assert_eq!(email.id, 15); - assert!(!email.required); - assert_eq!(*email.field_type, Type::Primitive(PrimitiveType::String)); + let err = UpdateSchemaAction::new(new_schema.clone(), last_col_id) + .unwrap() + .set_identifier_fields(vec!["new.fields.element.nested".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + format!( + "Cannot add field nested as an identifier field: must not be nested in {:?}", + new_schema.field_by_name("new.fields").unwrap() + ) + ); - // Original nested fields should still be there. - assert!(new_schema.field_by_name("person.name").is_some()); - assert!(new_schema.field_by_name("person.age").is_some()); + let err = UpdateSchemaAction::new(new_schema, last_col_id) + .unwrap() + .set_identifier_fields(vec!["preferences.feature1".to_string()]) + .apply() + .unwrap_err(); + assert_eq!( + err.message(), + format!( + "Cannot add field feature1 as an identifier field: must not be nested in an optional field {}", + schema.field_by_name("preferences").unwrap() + ) + ); } - #[tokio::test] - async fn test_add_column_to_struct_with_doc() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); - - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("phone") - .field_type(Type::Primitive(PrimitiveType::String)) - .parent("person") - .doc("Phone number") - .build(), + /* + Schema schemaWithIdentifierFields = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); + + assertThat( + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .deleteColumn("id") + .setIdentifierFields(Sets.newHashSet()) + .apply() + .identifierFieldIds()) + .as("delete column and then reset identifier field should succeed") + .isEmpty(); + + assertThat( + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields(Sets.newHashSet()) + .deleteColumn("id") + .apply() + .identifierFieldIds()) + .as("delete reset identifier field and then delete column should succeed") + .isEmpty(); + */ + #[test] + fn test_delete_identifier_field_columns() { + let schema: Arc<_> = SCHEMA.clone().into(); + + let schema_with_identifier_fields = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + + let err = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("id")) + .unwrap() + .set_identifier_fields(vec![]) + .apply() + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot delete identifier field: id. To force deletion, also call setIdentifierFields to update identifier fields." ); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + let err = UpdateSchemaAction::new(schema_with_identifier_fields, SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec![]) + .delete(DeleteColumn::new("id")) + .unwrap() + .apply() + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::PreconditionFailed); + assert_eq!( + err.message(), + "Cannot delete identifier field: id. To force deletion, also call setIdentifierFields to update identifier fields." + ); + } - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + /* + Schema schemaWithIdentifierFields = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); + + assertThatThrownBy( + () -> + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .deleteColumn("id") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot delete identifier field 1: id: required int. To force deletion, also call setIdentifierFields to update identifier fields."); + */ + #[test] + fn test_delete_identifier_field_columns_fails() { + let schema: Arc<_> = SCHEMA.clone().into(); + let schema_with_identifier_fields = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + + let err = UpdateSchemaAction::new(schema_with_identifier_fields, SCHEMA_LAST_COLUMN_ID) + .unwrap() + .delete(DeleteColumn::new("id")) + .unwrap() + .apply() + .unwrap_err(); - let phone = new_schema - .field_by_name("person.phone") - .expect("person.phone should exist"); - assert_eq!(phone.id, 15); - assert_eq!(phone.doc.as_deref(), Some("Phone number")); + assert_eq!( + err.message(), + "Cannot delete identifier field: id. To force deletion, also call setIdentifierFields to update identifier fields." + ); } - #[tokio::test] - async fn test_add_column_to_list_element_struct() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + /* + Schema newSchema = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .allowIncompatibleChanges() + .addRequiredColumn( + "out", + Types.StructType.of( + Types.NestedField.required( + SCHEMA_LAST_COLUMN_ID + 2, "nested", Types.StringType.get()))) + .setIdentifierFields("out.nested") + .apply(); + + assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID + 2).deleteColumn("out").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot delete field 24: out: required struct<25: nested: required string> " + + "as it will delete nested identifier field 25: nested: required string"); + */ + #[test] + fn test_delete_containing_nested_identifier_field_columns_fails() { + let schema: Arc<_> = SCHEMA.clone().into(); + + // Add a struct column with a nested identifier field + let schema_with_nested_field = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .allow_incompatible_changes() + .add(AddColumn::required( + "out", + Type::Struct(StructType::new(vec![Arc::new(NestedField::required( + SCHEMA_LAST_COLUMN_ID + 2, + "nested", + Type::Primitive(PrimitiveType::String), + ))])), + )) + .unwrap() + .set_identifier_fields(vec!["out.nested".to_string()]) + .apply() + .unwrap(); + + // Try to delete the struct column containing the nested identifier field + let err = UpdateSchemaAction::new(schema_with_nested_field, SCHEMA_LAST_COLUMN_ID + 2) + .unwrap() + .delete(DeleteColumn::new("out")) + .unwrap() + .apply() + .unwrap_err(); - // "tags" is a list. Adding to the list navigates to its - // element struct automatically. - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("score") - .field_type(Type::Primitive(PrimitiveType::Double)) - .parent("tags") - .build(), + assert!(err.message().contains("Cannot delete field")); + assert!( + err.message() + .contains("as it will delete nested identifier field") ); + } - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); - - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + /* + Schema schemaWithIdentifierFields = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); - // The list element struct should now contain "score". - let score = new_schema - .field_by_name("tags.element.score") - .expect("tags.element.score should exist"); - assert_eq!(score.id, 15); - assert!(!score.required); + Schema newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .renameColumn("id", "id2") + .apply(); - // Existing fields preserved. - assert!(new_schema.field_by_name("tags.element.key").is_some()); - assert!(new_schema.field_by_name("tags.element.value").is_some()); + assertThat(newSchema.identifierFieldIds()) + .as("rename should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); + */ + #[test] + fn test_rename_identifier_fields() { + let schema: Arc<_> = SCHEMA.clone().into(); + let id_field_id = schema.field_by_name("id").unwrap().id; + + let schema_with_identifier_fields = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields, SCHEMA_LAST_COLUMN_ID) + .unwrap() + .rename(RenameColumn::new("id", "id2")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + vec![id_field_id] + ); } - #[tokio::test] - async fn test_add_column_to_map_value_struct() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + /* + Schema schemaWithIdentifierFields = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); - // "props" is a map. Adding to the map navigates to its - // value struct automatically. - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("version") - .field_type(Type::Primitive(PrimitiveType::Int)) - .parent("props") - .build(), - ); + Schema newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .moveAfter("id", "locations") + .apply(); - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + assertThat(newSchema.identifierFieldIds()) + .as("move after should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .moveBefore("id", "locations") + .apply(); - let version = new_schema - .field_by_name("props.value.version") - .expect("props.value.version should exist"); - assert_eq!(version.id, 15); + assertThat(newSchema.identifierFieldIds()) + .as("move before should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); - // Existing map value fields preserved. - assert!(new_schema.field_by_name("props.value.data").is_some()); - } + newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID).moveFirst("id").apply(); - #[tokio::test] - async fn test_add_column_to_nonexistent_parent_fails() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + assertThat(newSchema.identifierFieldIds()) + .as("move first should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()) + */ + #[test] + fn test_move_identifier_fields() { + let schema: Arc<_> = SCHEMA.clone().into(); + let schema_with_identifier_fields = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .move_column(MoveColumn::after("id", "locations")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() + ); - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("col") - .field_type(Type::Primitive(PrimitiveType::Int)) - .parent("nonexistent") - .build(), + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .move_column(MoveColumn::before("id", "locations")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() ); - let err = match Arc::new(action).commit(&table).await { - Err(e) => e, - Ok(_) => panic!("should reject adding to a nonexistent parent"), - }; - assert_eq!(err.kind(), ErrorKind::PreconditionFailed); - assert!( - err.message().contains("nonexistent"), - "error should mention the missing parent, got: {}", - err.message() + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .move_column(MoveColumn::first("id")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() ); } - #[tokio::test] - async fn test_add_column_to_primitive_parent_fails() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + /* + Schema schemaWithIdentifierFields = + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); + + Schema newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .caseSensitive(false) + .moveAfter("iD", "locations") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("move after should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); + + newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .caseSensitive(false) + .moveBefore("ID", "locations") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("move before should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); + + newSchema = + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .caseSensitive(false) + .moveFirst("ID") + .apply(); + + assertThat(newSchema.identifierFieldIds()) + .as("move first should not affect identifier fields") + .containsExactly(SCHEMA.findField("id").fieldId()); + */ + #[test] + fn test_move_identifier_fields_case_insensitive() { + let schema: Arc<_> = SCHEMA.clone().into(); + let schema_with_identifier_fields = + UpdateSchemaAction::new(schema.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .set_identifier_fields(vec!["id".to_string()]) + .apply() + .unwrap(); + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .case_sensitive(false) + .move_column(MoveColumn::after("iD", "locations")) + .unwrap() + .apply() + .unwrap(); + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() + ); - // "x" is a primitive (long), not a struct. - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("col") - .field_type(Type::Primitive(PrimitiveType::Int)) - .parent("x") - .build(), + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .case_sensitive(false) + .move_column(MoveColumn::before("ID", "locations")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() ); - let err = match Arc::new(action).commit(&table).await { - Err(e) => e, - Ok(_) => panic!("should reject adding to a primitive parent"), - }; - assert_eq!(err.kind(), ErrorKind::PreconditionFailed); - assert!( - err.message().contains("not a struct"), - "error should mention type mismatch, got: {}", - err.message() + let new_schema = + UpdateSchemaAction::new(schema_with_identifier_fields.clone(), SCHEMA_LAST_COLUMN_ID) + .unwrap() + .case_sensitive(false) + .move_column(MoveColumn::first("ID")) + .unwrap() + .apply() + .unwrap(); + + assert_eq!( + new_schema.identifier_field_ids().collect::>(), + schema + .clone() + .field_by_name("id") + .map(|f| f.id) + .map(|id| vec![id]) + .unwrap() ); } - #[tokio::test] - async fn test_add_column_to_nested_name_conflict_fails() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + #[test] + fn test_move_top_deleted_column_after_another_column() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), + ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(4, "id", PrimitiveType::Int.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 3) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("id"))? + .add( + AddColumn::builder() + .name("id") + .r#type(PrimitiveType::Int.into()) + .is_optional(false) + .build(), + )? + .move_column(MoveColumn::after("id", "data"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } - // "name" already exists in the "person" struct. - let action = tx.update_schema().add_column( - AddColumn::builder() - .name("name") - .field_type(Type::Primitive(PrimitiveType::String)) - .parent("person") - .build(), + #[test] + fn test_move_top_deleted_column_before_another_column() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(4, "id", PrimitiveType::Int.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 3) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("id"))? + .add( + AddColumn::builder() + .name("id") + .r#type(PrimitiveType::Int.into()) + .is_optional(false) + .build(), + )? + .move_column(MoveColumn::before("id", "data_1"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } - let err = match Arc::new(action).commit(&table).await { - Err(e) => e, - Ok(_) => panic!("should reject adding a column with conflicting name"), - }; - assert_eq!(err.kind(), ErrorKind::PreconditionFailed); - assert!( - err.message().contains("already exists"), - "error should mention name conflict, got: {}", - err.message() + #[test] + fn test_move_top_deleted_column_to_first() -> Result<()> { + let schema = Arc::new( + Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(), ); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(4, "id", PrimitiveType::Int.into()).into(), + NestedField::required(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(3, "data_1", PrimitiveType::String.into()).into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 3) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("id"))? + .add( + AddColumn::builder() + .name("id") + .r#type(PrimitiveType::Int.into()) + .is_optional(false) + .build(), + )? + .move_column(MoveColumn::first("id"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) } - #[tokio::test] - async fn test_root_and_nested_add_combined() { - let table = make_v2_table_with_nested(); - let tx = Transaction::new(&table); + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()), + required(5, "data_1", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(6, "data", Types.IntegerType.get()), + required(5, "data_1", Types.StringType.get())))); + + Schema actual = + new SchemaUpdate(schema, 5) + .allowIncompatibleChanges() + .deleteColumn("struct.data") + .addRequiredColumn("struct", "data", Types.IntegerType.get()) + .moveAfter("struct.data", "struct.count") + .apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_deleted_nested_struct_field_after_another_column() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(6, "data", PrimitiveType::Int.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 5) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("struct.data")) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::Int.into()) + .parent(Some("struct".into())) + .is_optional(false) + .build(), + ) + .unwrap() + .move_column(MoveColumn::after("struct.data", "struct.count")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } - // Add a root column and a nested column in the same action. - let action = tx - .update_schema() - .add_column(AddColumn::optional( - "root_col", - Type::Primitive(PrimitiveType::Boolean), - )) - .add_column( + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()), + required(5, "data_1", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(6, "data", Types.IntegerType.get()), + required(5, "data_1", Types.StringType.get())))); + + Schema actual = + new SchemaUpdate(schema, 5) + .allowIncompatibleChanges() + .deleteColumn("struct.data") + .addRequiredColumn("struct", "data", Types.IntegerType.get()) + .moveBefore("struct.data", "struct.data_1") + .apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_deleted_nested_struct_field_before_another_column() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(6, "data", PrimitiveType::Int.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 5) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("struct.data")) + .unwrap() + .add( AddColumn::builder() - .name("email") - .field_type(Type::Primitive(PrimitiveType::String)) - .parent("person") + .name("data") + .r#type(PrimitiveType::Int.into()) + .parent(Some("struct".into())) + .is_optional(false) .build(), - ); + ) + .unwrap() + .move_column(MoveColumn::before("struct.data", "struct.data_1")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } + + /* + Schema schema = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(3, "count", Types.LongType.get()), + required(4, "data", Types.StringType.get()), + required(5, "data_1", Types.StringType.get())))); + Schema expected = + new Schema( + required(1, "id", Types.LongType.get()), + required( + 2, + "struct", + Types.StructType.of( + required(6, "data", Types.IntegerType.get()), + required(3, "count", Types.LongType.get()), + required(5, "data_1", Types.StringType.get())))); + + Schema actual = + new SchemaUpdate(schema, 5) + .allowIncompatibleChanges() + .deleteColumn("struct.data") + .addRequiredColumn("struct", "data", Types.IntegerType.get()) + .moveFirst("struct.data") + .apply(); + + assertThat(actual.asStruct()).isEqualTo(expected.asStruct()); + */ + #[test] + fn test_move_deleted_nested_struct_field_to_first() { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(4, "data", PrimitiveType::String.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap() + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::required( + 2, + "struct", + StructType::new(vec![ + NestedField::required(6, "data", PrimitiveType::Int.into()).into(), + NestedField::required(3, "count", PrimitiveType::Long.into()).into(), + NestedField::required(5, "data_1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build() + .unwrap(); + let actual = UpdateSchemaAction::new(schema, 5) + .unwrap() + .allow_incompatible_changes() + .delete(DeleteColumn::new("struct.data")) + .unwrap() + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::Int.into()) + .parent(Some("struct".into())) + .is_optional(false) + .build(), + ) + .unwrap() + .move_column(MoveColumn::first("struct.data")) + .unwrap() + .apply() + .unwrap(); + assert_eq!(actual.as_struct(), expected.as_struct()); + } + + #[test] + #[ignore = "not yet implemented: PrimitiveType::Unknown not supported in iceberg-rust"] + fn test_add_unknown() {} - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); + #[test] + #[ignore = "not yet implemented: PrimitiveType::Unknown not supported in iceberg-rust"] + fn test_add_unknown_non_null_default() {} - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + #[test] + #[ignore = "not yet implemented: PrimitiveType::Unknown not supported in iceberg-rust"] + fn test_add_required_unknown() {} - // Root column gets the first fresh ID. - let root_col = new_schema - .field_by_name("root_col") - .expect("root_col should exist"); - assert_eq!(root_col.id, 15); - - // Nested column gets the next ID. - let email = new_schema - .field_by_name("person.email") - .expect("person.email should exist"); - assert_eq!(email.id, 16); - } - - #[tokio::test] - async fn test_add_nested_struct_type_with_fresh_ids() { - // Adding a new column whose TYPE contains nested fields (e.g. a struct column). All sub-fields must receive - // fresh IDs, not placeholder `DEFAULT_FIELD_ID`. - let table = make_v2_table(); - let tx = Transaction::new(&table); - - let action = tx.update_schema().add_column(AddColumn::optional( - "address", - Type::Struct(StructType::new(vec![ + #[test] + fn test_case_insensitive_add_top_level_and_move() -> Result<()> { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + ]) + .build()? + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::optional(2, "data", PrimitiveType::String.into()).into(), + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + ]) + .build()?; + let actual = UpdateSchemaAction::new(schema, 1)? + .case_sensitive(false) + .add( + AddColumn::builder() + .name("data") + .r#type(PrimitiveType::String.into()) + .build(), + )? + .move_column(MoveColumn::first("dAtA"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } + + #[test] + fn test_case_insensitive_add_nested_and_move() -> Result<()> { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), NestedField::optional( - DEFAULT_FIELD_ID, - "street", - Type::Primitive(PrimitiveType::String), + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "field1", PrimitiveType::String.into()).into(), + ]) + .into(), ) .into(), + ]) + .build()? + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), NestedField::optional( - DEFAULT_FIELD_ID, - "city", - Type::Primitive(PrimitiveType::String), + 2, + "struct", + StructType::new(vec![ + NestedField::optional(4, "field2", PrimitiveType::Int.into()).into(), + NestedField::required(3, "field1", PrimitiveType::String.into()).into(), + ]) + .into(), ) .into(), - ])), - )); - - let mut action_commit = Arc::new(action).commit(&table).await.unwrap(); - let updates = action_commit.take_updates(); - - let new_schema = match &updates[0] { - TableUpdate::AddSchema { schema } => schema, - other => panic!("expected AddSchema, got {other:?}"), - }; + ]) + .build()?; + let actual = UpdateSchemaAction::new(schema, 3)? + .case_sensitive(false) + .add( + AddColumn::builder() + .name("field2") + .r#type(PrimitiveType::Int.into()) + .parent(Some("STRUCT".into())) + .build(), + )? + .move_column(MoveColumn::first("STRUCT.FIELD2"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) + } - // "address" gets ID 4 (last_column_id=3, +1). - let address = new_schema - .field_by_name("address") - .expect("address should exist"); - assert_eq!(address.id, 4); - - // Sub-fields get IDs 5 and 6. - let street = new_schema - .field_by_name("address.street") - .expect("address.street should exist"); - assert_eq!(street.id, 5); - - let city = new_schema - .field_by_name("address.city") - .expect("address.city should exist"); - assert_eq!(city.id, 6); + #[test] + fn test_case_insensitive_move_after_newly_added_field() -> Result<()> { + let schema: Arc<_> = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "field1", PrimitiveType::String.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build()? + .into(); + let expected = Schema::builder() + .with_fields(vec![ + NestedField::required(1, "id", PrimitiveType::Long.into()).into(), + NestedField::optional( + 2, + "struct", + StructType::new(vec![ + NestedField::required(3, "field1", PrimitiveType::String.into()).into(), + NestedField::optional(4, "field2", PrimitiveType::Int.into()).into(), + NestedField::optional(5, "field3", PrimitiveType::Double.into()).into(), + ]) + .into(), + ) + .into(), + ]) + .build()?; + let actual = UpdateSchemaAction::new(schema, 3)? + .case_sensitive(false) + .add( + AddColumn::builder() + .name("field2") + .r#type(PrimitiveType::Int.into()) + .parent(Some("STRUCT".into())) + .build(), + )? + .add( + AddColumn::builder() + .parent(Some("STRUCT".into())) + .name("field3") + .r#type(PrimitiveType::Double.into()) + .build(), + )? + .move_column(MoveColumn::after("STRUCT.FIELD2", "STRUCT.FIELD1"))? + .apply()?; + assert_eq!(actual.as_struct(), expected.as_struct()); + Ok(()) } }