From 00e1799852840c4b32775838651e720fd8e07a76 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 11:48:35 -0700 Subject: [PATCH 01/30] Impl tx action for updata_location, update_properties, and upgrade_format_version, add as_any to tx_action trait --- crates/iceberg/src/transaction/action.rs | 10 ++ crates/iceberg/src/transaction/mod.rs | 155 ++---------------- .../src/transaction/update_location.rs | 104 ++++++++++++ .../src/transaction/update_properties.rs | 138 ++++++++++++++++ .../src/transaction/upgrade_format_version.rs | 126 ++++++++++++++ 5 files changed, 391 insertions(+), 142 deletions(-) create mode 100644 crates/iceberg/src/transaction/update_location.rs create mode 100644 crates/iceberg/src/transaction/update_properties.rs create mode 100644 crates/iceberg/src/transaction/upgrade_format_version.rs diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index 51f54c4ce9..7715354634 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -16,6 +16,8 @@ // under the License. #![allow(dead_code)] + +use std::any::Any; use std::mem::take; use std::sync::Arc; @@ -35,6 +37,9 @@ pub type BoxedTransactionAction = Arc; /// to modify the table metadata. #[async_trait] pub(crate) trait TransactionAction: Sync + Send { + /// Returns the action as [`Any`] so it can be downcast to concrete types later + fn as_any(self: Arc) -> Arc; + /// Commits this action against the provided table and returns the resulting updates. /// NOTE: This function is intended for internal use only and should not be called directly by users. /// @@ -105,6 +110,7 @@ impl ActionCommit { #[cfg(test)] mod tests { + use std::any::Any; use std::str::FromStr; use std::sync::Arc; @@ -121,6 +127,10 @@ mod tests { #[async_trait] impl TransactionAction for TestAction { + fn as_any(self: Arc) -> Arc { + self + } + async fn commit(self: Arc, _table: &Table) -> Result { Ok(ActionCommit::new( vec![TableUpdate::SetLocation { diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index a592258a0d..f04055d54d 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -21,6 +21,9 @@ mod action; mod append; mod snapshot; mod sort_order; +mod update_location; +mod update_properties; +mod upgrade_format_version; use std::cmp::Ordering; use std::collections::HashMap; @@ -36,6 +39,9 @@ use crate::table::Table; use crate::transaction::action::BoxedTransactionAction; use crate::transaction::append::FastAppendAction; use crate::transaction::sort_order::ReplaceSortOrderAction; +use crate::transaction::update_location::UpdateLocationAction; +use crate::transaction::update_properties::UpdatePropertiesAction; +use crate::transaction::upgrade_format_version::UpgradeFormatVersionAction; use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdate}; /// Table transaction. @@ -104,32 +110,13 @@ impl Transaction { } /// Sets table to a new version. - pub fn upgrade_table_version(mut self, format_version: FormatVersion) -> Result { - let current_version = self.current_table.metadata().format_version(); - match current_version.cmp(&format_version) { - Ordering::Greater => { - return Err(Error::new( - ErrorKind::DataInvalid, - format!( - "Cannot downgrade table version from {} to {}", - current_version, format_version - ), - )); - } - Ordering::Less => { - self.apply(vec![UpgradeFormatVersion { format_version }], vec![])?; - } - Ordering::Equal => { - // Do nothing. - } - } - Ok(self) + pub fn upgrade_table_version(&self) -> UpgradeFormatVersionAction { + UpgradeFormatVersionAction::new() } /// Update table's property. - pub fn set_properties(mut self, props: HashMap) -> Result { - self.apply(vec![TableUpdate::SetProperties { updates: props }], vec![])?; - Ok(self) + pub fn update_properties(&self) -> UpdatePropertiesAction { + UpdatePropertiesAction::new() } fn generate_unique_snapshot_id(&self) -> i64 { @@ -178,19 +165,9 @@ impl Transaction { } } - /// Remove properties in table. - pub fn remove_properties(mut self, keys: Vec) -> Result { - self.apply( - vec![TableUpdate::RemoveProperties { removals: keys }], - vec![], - )?; - Ok(self) - } - /// Set the location of table - pub fn set_location(mut self, location: String) -> Result { - self.apply(vec![TableUpdate::SetLocation { location }], vec![])?; - Ok(self) + pub fn update_location(&self) -> UpdateLocationAction { + UpdateLocationAction::new() } /// Commit transaction. @@ -207,7 +184,6 @@ impl Transaction { #[cfg(test)] mod tests { - use std::collections::HashMap; use std::fs::File; use std::io::BufReader; @@ -217,7 +193,7 @@ mod tests { use crate::transaction::Transaction; use crate::{TableIdent, TableUpdate}; - fn make_v1_table() -> Table { + pub fn make_v1_table() -> Table { let file = File::open(format!( "{}/testdata/table_metadata/{}", env!("CARGO_MANIFEST_DIR"), @@ -273,109 +249,4 @@ mod tests { .build() .unwrap() } - - #[test] - fn test_upgrade_table_version_v1_to_v2() { - let table = make_v1_table(); - let tx = Transaction::new(&table); - let tx = tx.upgrade_table_version(FormatVersion::V2).unwrap(); - - assert_eq!( - vec![TableUpdate::UpgradeFormatVersion { - format_version: FormatVersion::V2 - }], - tx.updates - ); - } - - #[test] - fn test_upgrade_table_version_v2_to_v2() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - let tx = tx.upgrade_table_version(FormatVersion::V2).unwrap(); - - assert!( - tx.updates.is_empty(), - "Upgrade table to same version should not generate any updates" - ); - assert!( - tx.requirements.is_empty(), - "Upgrade table to same version should not generate any requirements" - ); - } - - #[test] - fn test_downgrade_table_version() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - let tx = tx.upgrade_table_version(FormatVersion::V1); - - assert!(tx.is_err(), "Downgrade table version should fail!"); - } - - #[test] - fn test_set_table_property() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - let tx = tx - .set_properties(HashMap::from([("a".to_string(), "b".to_string())])) - .unwrap(); - - assert_eq!( - vec![TableUpdate::SetProperties { - updates: HashMap::from([("a".to_string(), "b".to_string())]) - }], - tx.updates - ); - } - - #[test] - fn test_remove_property() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - let tx = tx - .remove_properties(vec!["a".to_string(), "b".to_string()]) - .unwrap(); - - assert_eq!( - vec![TableUpdate::RemoveProperties { - removals: vec!["a".to_string(), "b".to_string()] - }], - tx.updates - ); - } - - #[test] - fn test_set_location() { - let table = make_v2_table(); - let tx = Transaction::new(&table); - let tx = tx - .set_location(String::from("s3://bucket/prefix/new_table")) - .unwrap(); - - assert_eq!( - vec![TableUpdate::SetLocation { - location: String::from("s3://bucket/prefix/new_table") - }], - tx.updates - ) - } - - #[tokio::test] - async fn test_transaction_apply_upgrade() { - let table = make_v1_table(); - let tx = Transaction::new(&table); - // Upgrade v1 to v1, do nothing. - let tx = tx.upgrade_table_version(FormatVersion::V1).unwrap(); - // Upgrade v1 to v2, success. - let tx = tx.upgrade_table_version(FormatVersion::V2).unwrap(); - assert_eq!( - vec![TableUpdate::UpgradeFormatVersion { - format_version: FormatVersion::V2 - }], - tx.updates - ); - // Upgrade v2 to v1, return error. - assert!(tx.upgrade_table_version(FormatVersion::V1).is_err()); - } } diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs new file mode 100644 index 0000000000..06819976cc --- /dev/null +++ b/crates/iceberg/src/transaction/update_location.rs @@ -0,0 +1,104 @@ +// 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::any::Any; +use std::sync::Arc; + +use async_trait::async_trait; + +use crate::table::Table; +use crate::transaction::action::{ActionCommit, TransactionAction}; +use crate::{Error, ErrorKind, Result, TableUpdate}; + +/// A transaction action that sets or updates the location of a table. +/// +/// This action is used to explicitly set a new metadata location during a transaction, +/// typically as part of advanced commit or recovery flows. The location is optional until +/// explicitly set via [`set_location`]. +pub struct UpdateLocationAction { + location: Option, +} + +impl UpdateLocationAction { + /// Creates a new [`UpdateLocationAction`] with no location set. + pub fn new() -> Self { + UpdateLocationAction { location: None } + } + + /// Sets the target location for this action and returns the updated instance. + /// + /// # Arguments + /// + /// * `location` - A string representing the table's location. + /// + /// # Returns + /// + /// The [`UpdateLocationAction`] with the new location set. + pub fn set_location(mut self, location: String) -> Self { + self.location = Some(location); + self + } +} + +#[async_trait] +impl TransactionAction for UpdateLocationAction { + fn as_any(self: Arc) -> Arc { + self + } + + async fn commit(self: Arc, _table: &Table) -> Result { + let updates: Vec; + if let Some(location) = self.location.clone() { + updates = vec![TableUpdate::SetLocation { location }]; + } else { + return Err(Error::new( + ErrorKind::DataInvalid, + "Location is not set for UpdateLocationAction!", + )); + } + + Ok(ActionCommit::new(updates, vec![])) + } +} + +mod tests { + use crate::transaction::Transaction; + use crate::transaction::action::ApplyTransactionAction; + use crate::transaction::tests::make_v2_table; + use crate::transaction::update_location::UpdateLocationAction; + + #[test] + fn test_set_location() { + let table = make_v2_table(); + let tx = Transaction::new(&table); + let tx = tx + .update_location() + .set_location(String::from("s3://bucket/prefix/new_table")) + .apply(tx) + .unwrap(); + + assert_eq!(tx.actions.len(), 1); + + let any = tx.actions[0].clone().as_any(); + let action = any.downcast_ref::().unwrap(); + + assert_eq!( + action.location, + Some(String::from("s3://bucket/prefix/new_table")) + ) + } +} diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs new file mode 100644 index 0000000000..2efb3be46d --- /dev/null +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -0,0 +1,138 @@ +// 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::any::Any; +use std::collections::{HashMap, HashSet}; +use std::sync::Arc; + +use async_trait::async_trait; + +use crate::table::Table; +use crate::transaction::action::{ActionCommit, TransactionAction}; +use crate::{Result, TableUpdate}; + +/// A transactional action that updates or removes table properties +/// +/// This action is used to modify key-value pairs in a table's metadata +/// properties during a transaction. It supports setting new values for existing keys +/// or adding new keys, as well as removing existing keys. Each key can only be updated +/// or removed in a single action, not both. +pub struct UpdatePropertiesAction { + updates: HashMap, + removals: HashSet, +} + +impl UpdatePropertiesAction { + /// Creates a new [`UpdatePropertiesAction`] with no updates or removals. + pub fn new() -> Self { + UpdatePropertiesAction { + updates: HashMap::default(), + removals: HashSet::default(), + } + } + + /// Adds a key-value pair to the update set of this action. + /// + /// # Panics + /// + /// Panics if the key was previously marked for removal. + /// + /// # Arguments + /// + /// * `key` - The property key to update. + /// * `value` - The new value to associate with the key. + /// + /// # Returns + /// + /// The updated [`UpdatePropertiesAction`] with the key-value pair added to the update set. + pub fn set(mut self, key: String, value: String) -> Self { + assert!(!self.removals.contains(&key)); + self.updates.insert(key, value); + self + } + + /// Adds a key to the removal set of this action. + /// + /// # Panics + /// + /// Panics if the key was already marked for update. + /// + /// # Arguments + /// + /// * `key` - The property key to remove. + /// + /// # Returns + /// + /// The updated [`UpdatePropertiesAction`] with the key added to the removal set. + pub fn remove(mut self, key: String) -> Self { + assert!(!self.updates.contains_key(&key)); + self.removals.insert(key); + self + } +} + +#[async_trait] +impl TransactionAction for UpdatePropertiesAction { + fn as_any(self: Arc) -> Arc { + self + } + + async fn commit(self: Arc, _table: &Table) -> Result { + let updates: Vec = vec![ + TableUpdate::SetProperties { + updates: self.updates.clone(), + }, + TableUpdate::RemoveProperties { + removals: self.removals.clone().into_iter().collect::>(), + }, + ]; + + Ok(ActionCommit::new(updates, vec![])) + } +} + +mod tests { + use std::collections::{HashMap, HashSet}; + + use crate::transaction::Transaction; + use crate::transaction::action::ApplyTransactionAction; + use crate::transaction::tests::make_v2_table; + use crate::transaction::update_properties::UpdatePropertiesAction; + + #[test] + fn test_update_table_property() { + let table = make_v2_table(); + let tx = Transaction::new(&table); + let tx = tx + .update_properties() + .set("a".to_string(), "b".to_string()) + .remove("b".to_string()) + .apply(tx) + .unwrap(); + + assert_eq!(tx.actions.len(), 1); + + let any = tx.actions[0].clone().as_any(); + let action = any.downcast_ref::().unwrap(); + assert_eq!( + action.updates, + HashMap::from([("a".to_string(), "b".to_string())]) + ); + + assert_eq!(action.removals, HashSet::from(["b".to_string()])); + } +} diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs new file mode 100644 index 0000000000..34d917bdcd --- /dev/null +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -0,0 +1,126 @@ +// 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::any::Any; +use std::cmp::Ordering; +use std::sync::Arc; + +use async_trait::async_trait; + +use crate::TableUpdate::UpgradeFormatVersion; +use crate::spec::FormatVersion; +use crate::table::Table; +use crate::transaction::action::{ActionCommit, TransactionAction}; +use crate::{Error, ErrorKind, Result, TableUpdate}; + +/// A transaction action to upgrade a table's format version. +/// +/// This action is used within a transaction to indicate that the +/// table's format version should be upgraded to a specified version. +/// The location remains optional until explicitly set via [`set_format_version`]. +pub struct UpgradeFormatVersionAction { + format_version: Option, +} + +impl UpgradeFormatVersionAction { + /// Creates a new `UpgradeFormatVersionAction` with no version set. + pub fn new() -> Self { + UpgradeFormatVersionAction { + format_version: None, + } + } + + /// Sets the target format version for the upgrade. + /// + /// # Arguments + /// + /// * `format_version` - The version to upgrade the table format to. + /// + /// # Returns + /// + /// Returns the updated `UpgradeFormatVersionAction` with the format version set. + pub fn set_format_version(mut self, format_version: FormatVersion) -> Self { + self.format_version = Some(format_version); + self + } +} + +#[async_trait] +impl TransactionAction for UpgradeFormatVersionAction { + fn as_any(self: Arc) -> Arc { + self + } + + async fn commit(self: Arc, table: &Table) -> Result { + let current_version = table.metadata().format_version(); + let updates: Vec; + + if let Some(format_version) = self.format_version { + match current_version.cmp(&format_version) { + Ordering::Greater => { + return Err(Error::new( + ErrorKind::DataInvalid, + format!( + "Cannot downgrade table version from {} to {}", + current_version, format_version + ), + )); + } + Ordering::Less => { + updates = vec![UpgradeFormatVersion { format_version }]; + } + Ordering::Equal => { + // do nothing + updates = vec![]; + } + } + } else { + // error + return Err(Error::new( + ErrorKind::DataInvalid, + "FormatVersion is not set for UpgradeFormatVersionAction!", + )); + } + + Ok(ActionCommit::new(updates, vec![])) + } +} + +mod tests { + use crate::spec::FormatVersion; + use crate::transaction::Transaction; + use crate::transaction::action::ApplyTransactionAction; + use crate::transaction::upgrade_format_version::UpgradeFormatVersionAction; + + #[test] + fn test_upgrade_format_version() { + let table = crate::transaction::tests::make_v1_table(); + let tx = Transaction::new(&table); + let tx = tx + .upgrade_table_version() + .set_format_version(FormatVersion::V2) + .apply(tx) + .unwrap(); + + assert_eq!(tx.actions.len(), 1); + + let any = tx.actions[0].clone().as_any(); + let action = any.downcast_ref::().unwrap(); + + assert_eq!(action.format_version, FormatVersion::V2); + } +} From c46f288b26b0ff7fdacb1aec3363c557831ce1fa Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 11:55:53 -0700 Subject: [PATCH 02/30] minor --- crates/iceberg/src/transaction/mod.rs | 5 +---- crates/iceberg/src/transaction/upgrade_format_version.rs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index f04055d54d..b3ce678cfd 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -25,16 +25,13 @@ mod update_location; mod update_properties; mod upgrade_format_version; -use std::cmp::Ordering; use std::collections::HashMap; use std::mem::discriminant; use std::sync::Arc; use uuid::Uuid; -use crate::TableUpdate::UpgradeFormatVersion; use crate::error::Result; -use crate::spec::FormatVersion; use crate::table::Table; use crate::transaction::action::BoxedTransactionAction; use crate::transaction::append::FastAppendAction; @@ -42,7 +39,7 @@ use crate::transaction::sort_order::ReplaceSortOrderAction; use crate::transaction::update_location::UpdateLocationAction; use crate::transaction::update_properties::UpdatePropertiesAction; use crate::transaction::upgrade_format_version::UpgradeFormatVersionAction; -use crate::{Catalog, Error, ErrorKind, TableCommit, TableRequirement, TableUpdate}; +use crate::{Catalog, TableCommit, TableRequirement, TableUpdate}; /// Table transaction. pub struct Transaction { diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 34d917bdcd..ad8d6dac24 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -121,6 +121,6 @@ mod tests { let any = tx.actions[0].clone().as_any(); let action = any.downcast_ref::().unwrap(); - assert_eq!(action.format_version, FormatVersion::V2); + assert_eq!(action.format_version, Some(FormatVersion::V2)); } } From 6e56ae19b023701917c115d21ae01d1944705683 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 11:58:45 -0700 Subject: [PATCH 03/30] build --- crates/iceberg/src/transaction/mod.rs | 5 ++--- crates/iceberg/src/transaction/update_location.rs | 1 + crates/iceberg/src/transaction/update_properties.rs | 1 + crates/iceberg/src/transaction/upgrade_format_version.rs | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index b3ce678cfd..e9654feb8c 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -185,10 +185,9 @@ mod tests { use std::io::BufReader; use crate::io::FileIOBuilder; - use crate::spec::{FormatVersion, TableMetadata}; + use crate::spec::TableMetadata; use crate::table::Table; - use crate::transaction::Transaction; - use crate::{TableIdent, TableUpdate}; + use crate::TableIdent; pub fn make_v1_table() -> Table { let file = File::open(format!( diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs index 06819976cc..91eeae051b 100644 --- a/crates/iceberg/src/transaction/update_location.rs +++ b/crates/iceberg/src/transaction/update_location.rs @@ -75,6 +75,7 @@ impl TransactionAction for UpdateLocationAction { } } +#[cfg(test)] mod tests { use crate::transaction::Transaction; use crate::transaction::action::ApplyTransactionAction; diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 2efb3be46d..599d1925cc 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -105,6 +105,7 @@ impl TransactionAction for UpdatePropertiesAction { } } +#[cfg(test)] mod tests { use std::collections::{HashMap, HashSet}; diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index ad8d6dac24..afda58f6be 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -100,6 +100,7 @@ impl TransactionAction for UpgradeFormatVersionAction { } } +#[cfg(test)] mod tests { use crate::spec::FormatVersion; use crate::transaction::Transaction; From 37ce5b3e21dd6987ea9cc0209a03266452f1335d Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 12:00:10 -0700 Subject: [PATCH 04/30] fmt --- crates/iceberg/src/transaction/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index e9654feb8c..3ab070dc22 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -184,10 +184,10 @@ mod tests { use std::fs::File; use std::io::BufReader; + use crate::TableIdent; use crate::io::FileIOBuilder; use crate::spec::TableMetadata; use crate::table::Table; - use crate::TableIdent; pub fn make_v1_table() -> Table { let file = File::open(format!( From 7598bdc8d2d3d76963916583271bc9d46a24a4df Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 14:25:02 -0700 Subject: [PATCH 05/30] Fix tests --- crates/catalog/rest/src/catalog.rs | 15 +++++++++++---- crates/catalog/rest/tests/rest_catalog_test.rs | 8 ++++++-- crates/iceberg/src/transaction/action.rs | 2 +- crates/iceberg/src/transaction/mod.rs | 4 +++- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index 8518190699..7ffedb0626 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -803,6 +803,7 @@ mod tests { UnboundPartitionField, UnboundPartitionSpec, }; use iceberg::transaction::Transaction; + use iceberg::transaction::action::ApplyTransactionAction; use mockito::{Mock, Server, ServerGuard}; use serde_json::json; use uuid::uuid; @@ -2125,8 +2126,11 @@ mod tests { .unwrap() }; - let table = Transaction::new(&table1) - .upgrade_table_version(FormatVersion::V2) + let tx = Transaction::new(&table1); + let table = tx + .upgrade_table_version() + .set_format_version(FormatVersion::V2) + .apply(tx) .unwrap() .commit(&catalog) .await @@ -2250,8 +2254,11 @@ mod tests { .unwrap() }; - let table_result = Transaction::new(&table1) - .upgrade_table_version(FormatVersion::V2) + let tx = Transaction::new(&table1); + let table_result = tx + .upgrade_table_version() + .set_format_version(FormatVersion::V2) + .apply(tx) .unwrap() .commit(&catalog) .await; diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs index ab7ea3d62c..8aa082960f 100644 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ b/crates/catalog/rest/tests/rest_catalog_test.rs @@ -31,6 +31,7 @@ use iceberg_test_utils::{normalize_test_name, set_up}; use port_scanner::scan_port_addr; use tokio::time::sleep; use tracing::info; +use iceberg::transaction::action::ApplyTransactionAction; const REST_CATALOG_PORT: u16 = 8181; static DOCKER_COMPOSE_ENV: RwLock> = RwLock::new(None); @@ -346,9 +347,12 @@ async fn test_update_table() { &TableIdent::new(ns.name().clone(), "t1".to_string()) ); + let tx = Transaction::new(&table); // Update table by committing transaction - let table2 = Transaction::new(&table) - .set_properties(HashMap::from([("prop1".to_string(), "v1".to_string())])) + let table2 = tx + .update_properties() + .set("prop1".to_string(), "v1".to_string()) + .apply(tx) .unwrap() .commit(&catalog) .await diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index 7715354634..afa0529d56 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -28,7 +28,7 @@ use crate::transaction::Transaction; use crate::{Result, TableRequirement, TableUpdate}; /// A boxed, thread-safe reference to a `TransactionAction`. -pub type BoxedTransactionAction = Arc; +pub(crate) type BoxedTransactionAction = Arc; /// A trait representing an atomic action that can be part of a transaction. /// diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 3ab070dc22..aa27c2b987 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -17,7 +17,9 @@ //! This module contains transaction api. -mod action; +/// The `ApplyTransactionAction` trait provides an `apply` method +/// that allows users to apply a transaction action to a `Transaction`. +pub mod action; mod append; mod snapshot; mod sort_order; From f929a2dd85f0f565f253360dfc6965bbd192d000 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 14:25:53 -0700 Subject: [PATCH 06/30] fmt --- crates/catalog/rest/tests/rest_catalog_test.rs | 2 +- crates/iceberg/src/transaction/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs index 8aa082960f..c798035470 100644 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ b/crates/catalog/rest/tests/rest_catalog_test.rs @@ -24,6 +24,7 @@ use std::sync::RwLock; use ctor::{ctor, dtor}; use iceberg::spec::{FormatVersion, NestedField, PrimitiveType, Schema, Type}; use iceberg::transaction::Transaction; +use iceberg::transaction::action::ApplyTransactionAction; use iceberg::{Catalog, Namespace, NamespaceIdent, TableCreation, TableIdent}; use iceberg_catalog_rest::{RestCatalog, RestCatalogConfig}; use iceberg_test_utils::docker::DockerCompose; @@ -31,7 +32,6 @@ use iceberg_test_utils::{normalize_test_name, set_up}; use port_scanner::scan_port_addr; use tokio::time::sleep; use tracing::info; -use iceberg::transaction::action::ApplyTransactionAction; const REST_CATALOG_PORT: u16 = 8181; static DOCKER_COMPOSE_ENV: RwLock> = RwLock::new(None); diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index aa27c2b987..d68f8e7b48 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -17,7 +17,7 @@ //! This module contains transaction api. -/// The `ApplyTransactionAction` trait provides an `apply` method +/// The `ApplyTransactionAction` trait provides an `apply` method /// that allows users to apply a transaction action to a `Transaction`. pub mod action; mod append; From ffbfb31ba122b778c228e40c1285df19a5ccf1d3 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 16:14:54 -0700 Subject: [PATCH 07/30] impl default --- crates/iceberg/src/transaction/update_location.rs | 6 ++++++ crates/iceberg/src/transaction/update_properties.rs | 6 ++++++ crates/iceberg/src/transaction/upgrade_format_version.rs | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs index 91eeae051b..ae657fc453 100644 --- a/crates/iceberg/src/transaction/update_location.rs +++ b/crates/iceberg/src/transaction/update_location.rs @@ -54,6 +54,12 @@ impl UpdateLocationAction { } } +impl Default for UpdateLocationAction { + fn default() -> Self { + Self::new() + } +} + #[async_trait] impl TransactionAction for UpdateLocationAction { fn as_any(self: Arc) -> Arc { diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 599d1925cc..34635c70cb 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -85,6 +85,12 @@ impl UpdatePropertiesAction { } } +impl Default for UpdatePropertiesAction { + fn default() -> Self { + Self::new() + } +} + #[async_trait] impl TransactionAction for UpdatePropertiesAction { fn as_any(self: Arc) -> Arc { diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index afda58f6be..2beda831f7 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -59,6 +59,12 @@ impl UpgradeFormatVersionAction { } } +impl Default for UpgradeFormatVersionAction { + fn default() -> Self { + Self::new() + } +} + #[async_trait] impl TransactionAction for UpgradeFormatVersionAction { fn as_any(self: Arc) -> Arc { From eb71ada8ff8a00d049b414bb13b9deab81984ccd Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 16:36:25 -0700 Subject: [PATCH 08/30] add do_commit --- crates/iceberg/src/transaction/mod.rs | 42 ++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index d68f8e7b48..bf92c6c23d 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -170,11 +170,45 @@ impl Transaction { } /// Commit transaction. - pub async fn commit(self, catalog: &dyn Catalog) -> Result { + pub async fn commit(mut self, catalog: &dyn Catalog) -> Result
{ + if self.actions.is_empty() && self.updates.is_empty() { + // nothing to commit + return Ok(self.base_table.clone()); + } + + self.do_commit(catalog).await + } + + async fn do_commit(&mut self, catalog: &dyn Catalog) -> Result
{ + let base_table_identifier = self.base_table.identifier().to_owned(); + + let refreshed = catalog + .load_table(&base_table_identifier.clone()) + .await + .expect(format!("Failed to refresh table {}", base_table_identifier).as_str()); + + if self.base_table.metadata() != refreshed.metadata() + || self.base_table.metadata_location() != refreshed.metadata_location() + { + // current base is stale, use refreshed as base and re-apply transaction actions + self.base_table = refreshed.clone(); + } + + let current_table = self.base_table.clone(); + + for action in self.actions.clone() { + let mut action_commit = action.commit(¤t_table).await?; + // apply changes to current_table + self.apply( + action_commit.take_updates(), + action_commit.take_requirements(), + )?; + } + let table_commit = TableCommit::builder() - .ident(self.base_table.identifier().clone()) - .updates(self.updates) - .requirements(self.requirements) + .ident(base_table_identifier) + .updates(self.updates.clone()) + .requirements(self.requirements.clone()) .build(); catalog.update_table(table_commit).await From 7e402792ab322893286d8a57a9b51349e21d93b7 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 16:37:40 -0700 Subject: [PATCH 09/30] fmt --- crates/iceberg/src/transaction/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index bf92c6c23d..9de0619f4c 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -175,10 +175,10 @@ impl Transaction { // nothing to commit return Ok(self.base_table.clone()); } - + self.do_commit(catalog).await } - + async fn do_commit(&mut self, catalog: &dyn Catalog) -> Result
{ let base_table_identifier = self.base_table.identifier().to_owned(); @@ -186,14 +186,14 @@ impl Transaction { .load_table(&base_table_identifier.clone()) .await .expect(format!("Failed to refresh table {}", base_table_identifier).as_str()); - + if self.base_table.metadata() != refreshed.metadata() || self.base_table.metadata_location() != refreshed.metadata_location() { // current base is stale, use refreshed as base and re-apply transaction actions self.base_table = refreshed.clone(); } - + let current_table = self.base_table.clone(); for action in self.actions.clone() { From 43fc17a2fbcb435ae8470e021f76f074c88ed504 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 16:42:38 -0700 Subject: [PATCH 10/30] clippy --- crates/iceberg/src/transaction/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 9de0619f4c..2e3af9606e 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -185,7 +185,7 @@ impl Transaction { let refreshed = catalog .load_table(&base_table_identifier.clone()) .await - .expect(format!("Failed to refresh table {}", base_table_identifier).as_str()); + .unwrap_or_else(|_| panic!("Failed to refresh table {}", base_table_identifier)); if self.base_table.metadata() != refreshed.metadata() || self.base_table.metadata_location() != refreshed.metadata_location() From 29e7255b9045f271a21d8e2f18c1448ae3ada037 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 16:49:34 -0700 Subject: [PATCH 11/30] err handling --- crates/iceberg/src/transaction/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 2e3af9606e..716a7d4304 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -184,8 +184,7 @@ impl Transaction { let refreshed = catalog .load_table(&base_table_identifier.clone()) - .await - .unwrap_or_else(|_| panic!("Failed to refresh table {}", base_table_identifier)); + .await?; if self.base_table.metadata() != refreshed.metadata() || self.base_table.metadata_location() != refreshed.metadata_location() From 550867a4e3e27ac969f8e2e2f26c682ceaa1b223 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 17:10:45 -0700 Subject: [PATCH 12/30] fmt --- crates/iceberg/src/transaction/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 716a7d4304..01ac990daf 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -182,9 +182,7 @@ impl Transaction { async fn do_commit(&mut self, catalog: &dyn Catalog) -> Result
{ let base_table_identifier = self.base_table.identifier().to_owned(); - let refreshed = catalog - .load_table(&base_table_identifier.clone()) - .await?; + let refreshed = catalog.load_table(&base_table_identifier.clone()).await?; if self.base_table.metadata() != refreshed.metadata() || self.base_table.metadata_location() != refreshed.metadata_location() From 823659646c4043692b93a197ef00d4c6eedbef98 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 17:28:38 -0700 Subject: [PATCH 13/30] fix update table test --- crates/catalog/rest/src/catalog.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index 7ffedb0626..6871541138 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -2094,6 +2094,17 @@ mod tests { let config_mock = create_config_mock(&mut server).await; + let load_table_mock = server + .mock("GET", "/v1/namespaces/ns1/tables/test1") + .with_status(200) + .with_body_from_file(format!( + "{}/testdata/{}", + env!("CARGO_MANIFEST_DIR"), + "load_table_response.json" + )) + .create_async() + .await; + let update_table_mock = server .mock("POST", "/v1/namespaces/ns1/tables/test1") .with_status(200) @@ -2208,6 +2219,7 @@ mod tests { config_mock.assert_async().await; update_table_mock.assert_async().await; + load_table_mock.assert_async().await } #[tokio::test] From edfc32e18a677a35751a106ae5f2947bcd5d5b7b Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 10 Jun 2025 17:34:05 -0700 Subject: [PATCH 14/30] fix test, mock get request --- crates/catalog/rest/src/catalog.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index 6871541138..a417659057 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -2228,6 +2228,17 @@ mod tests { let config_mock = create_config_mock(&mut server).await; + let load_table_mock = server + .mock("GET", "/v1/namespaces/ns1/tables/test1") + .with_status(200) + .with_body_from_file(format!( + "{}/testdata/{}", + env!("CARGO_MANIFEST_DIR"), + "load_table_response.json" + )) + .create_async() + .await; + let update_table_mock = server .mock("POST", "/v1/namespaces/ns1/tables/test1") .with_status(404) @@ -2286,5 +2297,6 @@ mod tests { config_mock.assert_async().await; update_table_mock.assert_async().await; + load_table_mock.assert_async().await; } } From e365cb1d5e16d218c9e99e401bba82294428625a Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Wed, 11 Jun 2025 15:06:42 -0700 Subject: [PATCH 15/30] Update crates/iceberg/src/transaction/action.rs Co-authored-by: Renjie Liu --- crates/iceberg/src/transaction/action.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index afa0529d56..c712f13b6f 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -38,7 +38,9 @@ pub(crate) type BoxedTransactionAction = Arc; #[async_trait] pub(crate) trait TransactionAction: Sync + Send { /// Returns the action as [`Any`] so it can be downcast to concrete types later - fn as_any(self: Arc) -> Arc; + fn as_any(&self) -> &Any { + self + } /// Commits this action against the provided table and returns the resulting updates. /// NOTE: This function is intended for internal use only and should not be called directly by users. From 3fd17f33f31e4aca0faef07ec381badf407be471 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Wed, 11 Jun 2025 15:06:53 -0700 Subject: [PATCH 16/30] Update crates/iceberg/src/transaction/mod.rs Co-authored-by: Renjie Liu --- crates/iceberg/src/transaction/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 01ac990daf..8e3ddec045 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -114,7 +114,7 @@ impl Transaction { } /// Update table's property. - pub fn update_properties(&self) -> UpdatePropertiesAction { + pub fn update_table_properties(&self) -> UpdatePropertiesAction { UpdatePropertiesAction::new() } From f9a4593d12528b09ba7fb8c7d4d84fc0307d90a7 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Wed, 11 Jun 2025 17:46:14 -0700 Subject: [PATCH 17/30] addressing comments --- crates/catalog/rest/tests/rest_catalog_test.rs | 2 +- crates/iceberg/src/transaction/action.rs | 6 ++---- crates/iceberg/src/transaction/update_location.rs | 9 ++++++--- crates/iceberg/src/transaction/update_properties.rs | 11 +++++++---- .../iceberg/src/transaction/upgrade_format_version.rs | 9 ++++++--- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs index c798035470..549cf99008 100644 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ b/crates/catalog/rest/tests/rest_catalog_test.rs @@ -350,7 +350,7 @@ async fn test_update_table() { let tx = Transaction::new(&table); // Update table by committing transaction let table2 = tx - .update_properties() + .update_table_properties() .set("prop1".to_string(), "v1".to_string()) .apply(tx) .unwrap() diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index c712f13b6f..ced2f7a1cb 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -38,9 +38,7 @@ pub(crate) type BoxedTransactionAction = Arc; #[async_trait] pub(crate) trait TransactionAction: Sync + Send { /// Returns the action as [`Any`] so it can be downcast to concrete types later - fn as_any(&self) -> &Any { - self - } + fn as_any(&self) -> &dyn Any; /// Commits this action against the provided table and returns the resulting updates. /// NOTE: This function is intended for internal use only and should not be called directly by users. @@ -129,7 +127,7 @@ mod tests { #[async_trait] impl TransactionAction for TestAction { - fn as_any(self: Arc) -> Arc { + fn as_any(&self) -> &dyn Any { self } diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs index ae657fc453..7468960458 100644 --- a/crates/iceberg/src/transaction/update_location.rs +++ b/crates/iceberg/src/transaction/update_location.rs @@ -62,7 +62,7 @@ impl Default for UpdateLocationAction { #[async_trait] impl TransactionAction for UpdateLocationAction { - fn as_any(self: Arc) -> Arc { + fn as_any(&self) -> &dyn Any { self } @@ -100,8 +100,11 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let any = tx.actions[0].clone().as_any(); - let action = any.downcast_ref::().unwrap(); + let action_clone = tx.actions[0].clone(); + let action = action_clone + .as_any() + .downcast_ref::() + .unwrap(); assert_eq!( action.location, diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 34635c70cb..21eb5dbe8d 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -93,7 +93,7 @@ impl Default for UpdatePropertiesAction { #[async_trait] impl TransactionAction for UpdatePropertiesAction { - fn as_any(self: Arc) -> Arc { + fn as_any(&self) -> &dyn Any { self } @@ -125,7 +125,7 @@ mod tests { let table = make_v2_table(); let tx = Transaction::new(&table); let tx = tx - .update_properties() + .update_table_properties() .set("a".to_string(), "b".to_string()) .remove("b".to_string()) .apply(tx) @@ -133,8 +133,11 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let any = tx.actions[0].clone().as_any(); - let action = any.downcast_ref::().unwrap(); + let action_clone = tx.actions[0].clone(); + let action = action_clone + .as_any() + .downcast_ref::() + .unwrap(); assert_eq!( action.updates, HashMap::from([("a".to_string(), "b".to_string())]) diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 2beda831f7..2471ac87de 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -67,7 +67,7 @@ impl Default for UpgradeFormatVersionAction { #[async_trait] impl TransactionAction for UpgradeFormatVersionAction { - fn as_any(self: Arc) -> Arc { + fn as_any(&self) -> &dyn Any { self } @@ -125,8 +125,11 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let any = tx.actions[0].clone().as_any(); - let action = any.downcast_ref::().unwrap(); + let action_clone = tx.actions[0].clone(); + let action = action_clone + .as_any() + .downcast_ref::() + .unwrap(); assert_eq!(action.format_version, Some(FormatVersion::V2)); } From 1f321f9acfebb87fe6d0d8ef15f3314b461b08e6 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Wed, 11 Jun 2025 18:14:40 -0700 Subject: [PATCH 18/30] Remove unneeded format version check --- .../src/transaction/upgrade_format_version.rs | 36 +++++-------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 2471ac87de..8900967d7b 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -16,7 +16,6 @@ // under the License. use std::any::Any; -use std::cmp::Ordering; use std::sync::Arc; use async_trait::async_trait; @@ -25,7 +24,7 @@ use crate::TableUpdate::UpgradeFormatVersion; use crate::spec::FormatVersion; use crate::table::Table; use crate::transaction::action::{ActionCommit, TransactionAction}; -use crate::{Error, ErrorKind, Result, TableUpdate}; +use crate::{Error, ErrorKind, Result}; /// A transaction action to upgrade a table's format version. /// @@ -71,30 +70,8 @@ impl TransactionAction for UpgradeFormatVersionAction { self } - async fn commit(self: Arc, table: &Table) -> Result { - let current_version = table.metadata().format_version(); - let updates: Vec; - - if let Some(format_version) = self.format_version { - match current_version.cmp(&format_version) { - Ordering::Greater => { - return Err(Error::new( - ErrorKind::DataInvalid, - format!( - "Cannot downgrade table version from {} to {}", - current_version, format_version - ), - )); - } - Ordering::Less => { - updates = vec![UpgradeFormatVersion { format_version }]; - } - Ordering::Equal => { - // do nothing - updates = vec![]; - } - } - } else { + async fn commit(self: Arc, _table: &Table) -> Result { + if self.format_version.is_none() { // error return Err(Error::new( ErrorKind::DataInvalid, @@ -102,7 +79,12 @@ impl TransactionAction for UpgradeFormatVersionAction { )); } - Ok(ActionCommit::new(updates, vec![])) + Ok(ActionCommit::new( + vec![UpgradeFormatVersion { + format_version: self.format_version.unwrap(), + }], + vec![], + )) } } From cbe28af6adcb283979dabe9a86f242f299a48bcb Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:18:00 -0700 Subject: [PATCH 19/30] Update crates/iceberg/src/transaction/mod.rs Co-authored-by: Renjie Liu --- crates/iceberg/src/transaction/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 8e3ddec045..fc2edcf996 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -19,7 +19,8 @@ /// The `ApplyTransactionAction` trait provides an `apply` method /// that allows users to apply a transaction action to a `Transaction`. -pub mod action; +mod action; +pub use action::*; mod append; mod snapshot; mod sort_order; From 6047a0664b53cec709a1b7014215169a54942542 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:19:17 -0700 Subject: [PATCH 20/30] remove allow dead code in action.rs --- crates/iceberg/src/transaction/action.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index ced2f7a1cb..e788dbebb0 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -#![allow(dead_code)] - use std::any::Any; use std::mem::take; use std::sync::Arc; From fc2efa01bbd9704500ba8b20022f9dd014e75816 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:25:54 -0700 Subject: [PATCH 21/30] fix build --- crates/catalog/rest/src/catalog.rs | 3 +-- crates/catalog/rest/tests/rest_catalog_test.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index a417659057..3d1f7cf83c 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -802,8 +802,7 @@ mod tests { SnapshotLog, SortDirection, SortField, SortOrder, Summary, Transform, Type, UnboundPartitionField, UnboundPartitionSpec, }; - use iceberg::transaction::Transaction; - use iceberg::transaction::action::ApplyTransactionAction; + use iceberg::transaction::{ApplyTransactionAction, Transaction}; use mockito::{Mock, Server, ServerGuard}; use serde_json::json; use uuid::uuid; diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs index 549cf99008..94d2e3927c 100644 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ b/crates/catalog/rest/tests/rest_catalog_test.rs @@ -23,8 +23,7 @@ use std::sync::RwLock; use ctor::{ctor, dtor}; use iceberg::spec::{FormatVersion, NestedField, PrimitiveType, Schema, Type}; -use iceberg::transaction::Transaction; -use iceberg::transaction::action::ApplyTransactionAction; +use iceberg::transaction::{ApplyTransactionAction, Transaction}; use iceberg::{Catalog, Namespace, NamespaceIdent, TableCreation, TableIdent}; use iceberg_catalog_rest::{RestCatalog, RestCatalogConfig}; use iceberg_test_utils::docker::DockerCompose; From a5ae3c6d7ada78597c026d01b7feb09ef03f1e63 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:35:29 -0700 Subject: [PATCH 22/30] check overlapping keys in commit --- .../src/transaction/update_properties.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 21eb5dbe8d..b766b99201 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -23,7 +23,7 @@ use async_trait::async_trait; use crate::table::Table; use crate::transaction::action::{ActionCommit, TransactionAction}; -use crate::{Result, TableUpdate}; +use crate::{Error, ErrorKind, Result, TableUpdate}; /// A transactional action that updates or removes table properties /// @@ -47,10 +47,6 @@ impl UpdatePropertiesAction { /// Adds a key-value pair to the update set of this action. /// - /// # Panics - /// - /// Panics if the key was previously marked for removal. - /// /// # Arguments /// /// * `key` - The property key to update. @@ -60,17 +56,12 @@ impl UpdatePropertiesAction { /// /// The updated [`UpdatePropertiesAction`] with the key-value pair added to the update set. pub fn set(mut self, key: String, value: String) -> Self { - assert!(!self.removals.contains(&key)); self.updates.insert(key, value); self } /// Adds a key to the removal set of this action. /// - /// # Panics - /// - /// Panics if the key was already marked for update. - /// /// # Arguments /// /// * `key` - The property key to remove. @@ -79,7 +70,6 @@ impl UpdatePropertiesAction { /// /// The updated [`UpdatePropertiesAction`] with the key added to the removal set. pub fn remove(mut self, key: String) -> Self { - assert!(!self.updates.contains_key(&key)); self.removals.insert(key); self } @@ -98,6 +88,16 @@ impl TransactionAction for UpdatePropertiesAction { } async fn commit(self: Arc, _table: &Table) -> Result { + if let Some(overlapping_key) = self.removals.iter().find(|k| self.updates.contains_key(*k)) { + return Err(Error::new( + ErrorKind::PreconditionFailed, + format!( + "Key {} is present in both the HashSet and the HashMap", + overlapping_key + ), + )); + } + let updates: Vec = vec![ TableUpdate::SetProperties { updates: self.updates.clone(), From cbb52a3a5ffbcd5724fd0dc7b0d5993303f16e07 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:38:09 -0700 Subject: [PATCH 23/30] fmt --- crates/iceberg/src/transaction/update_properties.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index b766b99201..0b98b6594e 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -88,7 +88,8 @@ impl TransactionAction for UpdatePropertiesAction { } async fn commit(self: Arc, _table: &Table) -> Result { - if let Some(overlapping_key) = self.removals.iter().find(|k| self.updates.contains_key(*k)) { + if let Some(overlapping_key) = self.removals.iter().find(|k| self.updates.contains_key(*k)) + { return Err(Error::new( ErrorKind::PreconditionFailed, format!( From f34e098c4395494a4ed7dffe7247b8f02df68ff5 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:43:52 -0700 Subject: [PATCH 24/30] allow dead code for as_any --- crates/iceberg/src/transaction/action.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index e788dbebb0..e7c43c5312 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -35,6 +35,7 @@ pub(crate) type BoxedTransactionAction = Arc; /// to modify the table metadata. #[async_trait] pub(crate) trait TransactionAction: Sync + Send { + #[allow(dead_code)] /// Returns the action as [`Any`] so it can be downcast to concrete types later fn as_any(&self) -> &dyn Any; From bdd54ad912e318d4354ccb894c49f60ed04b572d Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 12 Jun 2025 11:44:54 -0700 Subject: [PATCH 25/30] minor --- crates/iceberg/src/transaction/action.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index e7c43c5312..9929f1908c 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -35,8 +35,8 @@ pub(crate) type BoxedTransactionAction = Arc; /// to modify the table metadata. #[async_trait] pub(crate) trait TransactionAction: Sync + Send { - #[allow(dead_code)] /// Returns the action as [`Any`] so it can be downcast to concrete types later + #[allow(dead_code)] fn as_any(&self) -> &dyn Any; /// Commits this action against the provided table and returns the resulting updates. From 25125c35537e97f716ae9491c37c29f0fe61c178 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 13 Jun 2025 08:56:57 -0700 Subject: [PATCH 26/30] Update crates/iceberg/src/transaction/update_properties.rs Co-authored-by: Renjie Liu --- crates/iceberg/src/transaction/update_properties.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 0b98b6594e..62aa458911 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -93,7 +93,7 @@ impl TransactionAction for UpdatePropertiesAction { return Err(Error::new( ErrorKind::PreconditionFailed, format!( - "Key {} is present in both the HashSet and the HashMap", + "Key {} is present in both removal set and update set", overlapping_key ), )); From af7ec1c0a7c0d52ca9e33dd96459c5ef05764760 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 13 Jun 2025 09:32:29 -0700 Subject: [PATCH 27/30] use ok or else to get format version --- .../src/transaction/upgrade_format_version.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 8900967d7b..1dc49eff9e 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -71,18 +71,15 @@ impl TransactionAction for UpgradeFormatVersionAction { } async fn commit(self: Arc, _table: &Table) -> Result { - if self.format_version.is_none() { - // error - return Err(Error::new( + let format_version = self.format_version.ok_or_else(|| { + Err(Error::new( ErrorKind::DataInvalid, "FormatVersion is not set for UpgradeFormatVersionAction!", - )); - } + )) + })?; Ok(ActionCommit::new( - vec![UpgradeFormatVersion { - format_version: self.format_version.unwrap(), - }], + vec![UpgradeFormatVersion { format_version }], vec![], )) } From 08e5f4fcedc09a4a6f172bcb16d9da6a6a0b59f6 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 13 Jun 2025 12:27:33 -0700 Subject: [PATCH 28/30] use AsAny --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + crates/iceberg/Cargo.toml | 1 + crates/iceberg/src/transaction/action.rs | 19 +++++++------------ .../src/transaction/update_location.rs | 11 +++-------- .../src/transaction/update_properties.rs | 11 +++-------- .../src/transaction/upgrade_format_version.rs | 15 +++++---------- 7 files changed, 27 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0bf3efa66f..73ebf15e01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -419,6 +419,12 @@ dependencies = [ "regex-syntax 0.8.5", ] +[[package]] +name = "as-any" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0f477b951e452a0b6b4a10b53ccd569042d1d01729b519e02074a9c0958a063" + [[package]] name = "assert-json-diff" version = "2.0.2" @@ -3488,6 +3494,7 @@ dependencies = [ "arrow-schema", "arrow-select", "arrow-string", + "as-any", "async-std", "async-trait", "base64 0.22.1", diff --git a/Cargo.toml b/Cargo.toml index cae0c85cb1..43d0352003 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ arrow-ord = { version = "55" } arrow-schema = { version = "55" } arrow-select = { version = "55" } arrow-string = { version = "55" } +as-any = "0.3.2" async-std = "1.12" async-trait = "0.1.88" aws-config = "1.6.1" diff --git a/crates/iceberg/Cargo.toml b/crates/iceberg/Cargo.toml index d1ddc82463..9f5d490128 100644 --- a/crates/iceberg/Cargo.toml +++ b/crates/iceberg/Cargo.toml @@ -54,6 +54,7 @@ arrow-ord = { workspace = true } arrow-schema = { workspace = true } arrow-select = { workspace = true } arrow-string = { workspace = true } +as-any = { workspace = true } async-std = { workspace = true, optional = true, features = ["attributes"] } async-trait = { workspace = true } base64 = { workspace = true } diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index 9929f1908c..873a22db93 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -15,10 +15,10 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; use std::mem::take; use std::sync::Arc; +use as_any::AsAny; use async_trait::async_trait; use crate::table::Table; @@ -34,11 +34,7 @@ pub(crate) type BoxedTransactionAction = Arc; /// Each action is responsible for generating the updates and requirements needed /// to modify the table metadata. #[async_trait] -pub(crate) trait TransactionAction: Sync + Send { - /// Returns the action as [`Any`] so it can be downcast to concrete types later - #[allow(dead_code)] - fn as_any(&self) -> &dyn Any; - +pub(crate) trait TransactionAction: AsAny + Sync + Send { /// Commits this action against the provided table and returns the resulting updates. /// NOTE: This function is intended for internal use only and should not be called directly by users. /// @@ -109,10 +105,10 @@ impl ActionCommit { #[cfg(test)] mod tests { - use std::any::Any; use std::str::FromStr; use std::sync::Arc; + use as_any::Downcast; use async_trait::async_trait; use uuid::Uuid; @@ -126,10 +122,6 @@ mod tests { #[async_trait] impl TransactionAction for TestAction { - fn as_any(&self) -> &dyn Any { - self - } - async fn commit(self: Arc, _table: &Table) -> Result { Ok(ActionCommit::new( vec![TableUpdate::SetLocation { @@ -167,9 +159,12 @@ mod tests { let tx = Transaction::new(&table); let updated_tx = action.apply(tx).unwrap(); - // There should be one action in the transaction now assert_eq!(updated_tx.actions.len(), 1); + + (&*updated_tx.actions[0]) + .downcast_ref::() + .expect("TestAction was not applied to Transaction!"); } #[test] diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs index 7468960458..71a717bad1 100644 --- a/crates/iceberg/src/transaction/update_location.rs +++ b/crates/iceberg/src/transaction/update_location.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; use std::sync::Arc; use async_trait::async_trait; @@ -62,10 +61,6 @@ impl Default for UpdateLocationAction { #[async_trait] impl TransactionAction for UpdateLocationAction { - fn as_any(&self) -> &dyn Any { - self - } - async fn commit(self: Arc, _table: &Table) -> Result { let updates: Vec; if let Some(location) = self.location.clone() { @@ -83,6 +78,8 @@ impl TransactionAction for UpdateLocationAction { #[cfg(test)] mod tests { + use as_any::Downcast; + use crate::transaction::Transaction; use crate::transaction::action::ApplyTransactionAction; use crate::transaction::tests::make_v2_table; @@ -100,9 +97,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action_clone = tx.actions[0].clone(); - let action = action_clone - .as_any() + let action = (&*tx.actions[0]) .downcast_ref::() .unwrap(); diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index 62aa458911..dc2d5820ee 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; use std::collections::{HashMap, HashSet}; use std::sync::Arc; @@ -83,10 +82,6 @@ impl Default for UpdatePropertiesAction { #[async_trait] impl TransactionAction for UpdatePropertiesAction { - fn as_any(&self) -> &dyn Any { - self - } - async fn commit(self: Arc, _table: &Table) -> Result { if let Some(overlapping_key) = self.removals.iter().find(|k| self.updates.contains_key(*k)) { @@ -116,6 +111,8 @@ impl TransactionAction for UpdatePropertiesAction { mod tests { use std::collections::{HashMap, HashSet}; + use as_any::Downcast; + use crate::transaction::Transaction; use crate::transaction::action::ApplyTransactionAction; use crate::transaction::tests::make_v2_table; @@ -134,9 +131,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action_clone = tx.actions[0].clone(); - let action = action_clone - .as_any() + let action = (&*tx.actions[0]) .downcast_ref::() .unwrap(); assert_eq!( diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 1dc49eff9e..50f56b0706 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; use std::sync::Arc; use async_trait::async_trait; @@ -66,16 +65,12 @@ impl Default for UpgradeFormatVersionAction { #[async_trait] impl TransactionAction for UpgradeFormatVersionAction { - fn as_any(&self) -> &dyn Any { - self - } - async fn commit(self: Arc, _table: &Table) -> Result { let format_version = self.format_version.ok_or_else(|| { - Err(Error::new( + Error::new( ErrorKind::DataInvalid, "FormatVersion is not set for UpgradeFormatVersionAction!", - )) + ) })?; Ok(ActionCommit::new( @@ -87,6 +82,8 @@ impl TransactionAction for UpgradeFormatVersionAction { #[cfg(test)] mod tests { + use as_any::Downcast; + use crate::spec::FormatVersion; use crate::transaction::Transaction; use crate::transaction::action::ApplyTransactionAction; @@ -104,9 +101,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action_clone = tx.actions[0].clone(); - let action = action_clone - .as_any() + let action = (&*tx.actions[0]) .downcast_ref::() .unwrap(); From ce85b641a8028159b2df9828e20d9f43222df2e9 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 13 Jun 2025 12:34:47 -0700 Subject: [PATCH 29/30] clippy --- crates/iceberg/src/transaction/action.rs | 2 +- crates/iceberg/src/transaction/update_location.rs | 2 +- crates/iceberg/src/transaction/update_properties.rs | 2 +- crates/iceberg/src/transaction/upgrade_format_version.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/iceberg/src/transaction/action.rs b/crates/iceberg/src/transaction/action.rs index 873a22db93..aa0a05d0d9 100644 --- a/crates/iceberg/src/transaction/action.rs +++ b/crates/iceberg/src/transaction/action.rs @@ -162,7 +162,7 @@ mod tests { // There should be one action in the transaction now assert_eq!(updated_tx.actions.len(), 1); - (&*updated_tx.actions[0]) + (*updated_tx.actions[0]) .downcast_ref::() .expect("TestAction was not applied to Transaction!"); } diff --git a/crates/iceberg/src/transaction/update_location.rs b/crates/iceberg/src/transaction/update_location.rs index 71a717bad1..0c32c75355 100644 --- a/crates/iceberg/src/transaction/update_location.rs +++ b/crates/iceberg/src/transaction/update_location.rs @@ -97,7 +97,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action = (&*tx.actions[0]) + let action = (*tx.actions[0]) .downcast_ref::() .unwrap(); diff --git a/crates/iceberg/src/transaction/update_properties.rs b/crates/iceberg/src/transaction/update_properties.rs index dc2d5820ee..825d6bec8d 100644 --- a/crates/iceberg/src/transaction/update_properties.rs +++ b/crates/iceberg/src/transaction/update_properties.rs @@ -131,7 +131,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action = (&*tx.actions[0]) + let action = (*tx.actions[0]) .downcast_ref::() .unwrap(); assert_eq!( diff --git a/crates/iceberg/src/transaction/upgrade_format_version.rs b/crates/iceberg/src/transaction/upgrade_format_version.rs index 50f56b0706..ff15926d00 100644 --- a/crates/iceberg/src/transaction/upgrade_format_version.rs +++ b/crates/iceberg/src/transaction/upgrade_format_version.rs @@ -101,7 +101,7 @@ mod tests { assert_eq!(tx.actions.len(), 1); - let action = (&*tx.actions[0]) + let action = (*tx.actions[0]) .downcast_ref::() .unwrap(); From 65a2358e31fb52ca64b07608c4ed7d4afee7bdab Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 13 Jun 2025 16:26:26 -0700 Subject: [PATCH 30/30] rerun CI