mapleFU opened a new issue, #1583:
URL: https://github.com/apache/iceberg-rust/issues/1583

   I'm a rust newbie, when I go thourgh the code in 
https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/src/transaction/mod.rs#L136-L164
 , I found these factory functions requires `&self` but not used. Can these 
replaced with the code below?
   
   ```diff
   diff --git a/crates/catalog/rest/src/catalog.rs 
b/crates/catalog/rest/src/catalog.rs
   index 7d81982f..1a356085 100644
   --- a/crates/catalog/rest/src/catalog.rs
   +++ b/crates/catalog/rest/src/catalog.rs
   @@ -2360,8 +2360,8 @@ mod tests {
            };
    
            let tx = Transaction::new(&table1);
   -        let table = tx
   -            .upgrade_table_version()
   +        let table =
   +            Transaction::upgrade_table_version()
                .set_format_version(FormatVersion::V2)
                .apply(tx)
                .unwrap()
   @@ -2500,8 +2500,7 @@ mod tests {
            };
    
            let tx = Transaction::new(&table1);
   -        let table_result = tx
   -            .upgrade_table_version()
   +        let table_result = Transaction::upgrade_table_version()
                .set_format_version(FormatVersion::V2)
                .apply(tx)
                .unwrap()
   diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs 
b/crates/catalog/rest/tests/rest_catalog_test.rs
   index 393b2435..0ad8e1df 100644
   --- a/crates/catalog/rest/tests/rest_catalog_test.rs
   +++ b/crates/catalog/rest/tests/rest_catalog_test.rs
   @@ -348,8 +348,8 @@ async fn test_update_table() {
    
        let tx = Transaction::new(&table);
        // Update table by committing transaction
   -    let table2 = tx
   -        .update_table_properties()
   +    let table2 =
   +        Transaction::update_table_properties()
            .set("prop1".to_string(), "v1".to_string())
            .apply(tx)
            .unwrap()
   diff --git a/crates/iceberg/src/catalog/memory/catalog.rs 
b/crates/iceberg/src/catalog/memory/catalog.rs
   index 12d18b9f..69614ec7 100644
   --- a/crates/iceberg/src/catalog/memory/catalog.rs
   +++ b/crates/iceberg/src/catalog/memory/catalog.rs
   @@ -1803,8 +1803,7 @@ mod tests {
    
            // Update table metadata
            let tx = Transaction::new(&table);
   -        let updated_table = tx
   -            .update_table_properties()
   +        let updated_table = Transaction::update_table_properties()
                .set("key".to_string(), "value".to_string())
                .apply(tx)
                .unwrap()
   @@ -1839,8 +1838,7 @@ mod tests {
            let table = build_table(table_ident);
    
            let tx = Transaction::new(&table);
   -        let err = tx
   -            .update_table_properties()
   +        let err = Transaction::update_table_properties()
                .set("key".to_string(), "value".to_string())
                .apply(tx)
                .unwrap()
   diff --git a/crates/iceberg/src/transaction/mod.rs 
b/crates/iceberg/src/transaction/mod.rs
   index 06549a95..6f63019e 100644
   --- a/crates/iceberg/src/transaction/mod.rs
   +++ b/crates/iceberg/src/transaction/mod.rs
   @@ -134,32 +134,32 @@ impl Transaction {
        }
    
        /// Sets table to a new version.
   -    pub fn upgrade_table_version(&self) -> UpgradeFormatVersionAction {
   +    pub fn upgrade_table_version() -> UpgradeFormatVersionAction {
            UpgradeFormatVersionAction::new()
        }
    
        /// Update table's property.
   -    pub fn update_table_properties(&self) -> UpdatePropertiesAction {
   +    pub fn update_table_properties() -> UpdatePropertiesAction {
            UpdatePropertiesAction::new()
        }
    
        /// Creates a fast append action.
   -    pub fn fast_append(&self) -> FastAppendAction {
   +    pub fn fast_append() -> FastAppendAction {
            FastAppendAction::new()
        }
    
        /// Creates replace sort order action.
   -    pub fn replace_sort_order(&self) -> ReplaceSortOrderAction {
   +    pub fn replace_sort_order() -> ReplaceSortOrderAction {
            ReplaceSortOrderAction::new()
        }
    
        /// Set the location of table
   -    pub fn update_location(&self) -> UpdateLocationAction {
   +    pub fn update_location() -> UpdateLocationAction {
            UpdateLocationAction::new()
        }
    
        /// Update the statistics of table
   -    pub fn update_statistics(&self) -> UpdateStatisticsAction {
   +    pub fn update_statistics() -> UpdateStatisticsAction {
            UpdateStatisticsAction::new()
        }
    
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to