CTTY commented on code in PR #1448:
URL: https://github.com/apache/iceberg-rust/pull/1448#discussion_r2162617409


##########
crates/iceberg/src/transaction/append.rs:
##########
@@ -62,101 +59,51 @@ impl FastAppendAction {
     }
 
     /// Add data files to the snapshot.
-    pub fn add_data_files(
-        &mut self,
-        data_files: impl IntoIterator<Item = DataFile>,
-    ) -> Result<&mut Self> {
-        self.snapshot_produce_action.add_data_files(data_files)?;
-        Ok(self)
+    pub fn add_data_files(mut self, data_files: impl IntoIterator<Item = 
DataFile>) -> Self {
+        self.added_data_files.extend(data_files);
+        self
     }
 
-    /// Set snapshot summary properties.
-    pub fn set_snapshot_properties(
-        &mut self,
-        snapshot_properties: HashMap<String, String>,
-    ) -> Result<&mut Self> {
-        self.snapshot_produce_action
-            .set_snapshot_properties(snapshot_properties)?;
-        Ok(self)
+    /// Set commit UUID for the snapshot.
+    pub fn set_commit_uuid(mut self, commit_uuid: Uuid) -> Self {
+        self.commit_uuid = Some(commit_uuid);
+        self
     }
 
-    /// Adds existing parquet files
-    ///
-    /// Note: This API is not yet fully supported in version 0.5.x.  
-    /// It is currently incomplete and should not be used in production.
-    /// Specifically, schema compatibility checks and support for adding to 
partitioned tables
-    /// have not yet been implemented.
-    #[allow(dead_code)]
-    async fn add_parquet_files(mut self, file_path: Vec<String>) -> 
Result<Transaction> {
-        if !self
-            .snapshot_produce_action
-            .tx
-            .current_table
-            .metadata()
-            .default_spec
-            .is_unpartitioned()
-        {
-            return Err(Error::new(
-                ErrorKind::FeatureUnsupported,
-                "Appending to partitioned tables is not supported",
-            ));
-        }
-
-        let table_metadata = 
self.snapshot_produce_action.tx.current_table.metadata();
-
-        let data_files = ParquetWriter::parquet_files_to_data_files(
-            self.snapshot_produce_action.tx.current_table.file_io(),
-            file_path,
-            table_metadata,
-        )
-        .await?;
-
-        self.add_data_files(data_files)?;
+    /// Set key metadata for manifest files.
+    pub fn set_key_metadata(mut self, key_metadata: Vec<u8>) -> Self {
+        self.key_metadata = Some(key_metadata);
+        self
+    }
 
-        self.apply().await
+    /// Set snapshot summary properties.
+    pub fn set_snapshot_properties(mut self, snapshot_properties: 
HashMap<String, String>) -> Self {
+        self.snapshot_properties = snapshot_properties;
+        self
     }
+}
+
+#[async_trait]
+impl TransactionAction for FastAppendAction {
+    async fn commit(self: Arc<Self>, table: &Table) -> Result<ActionCommit> {
+        // validate added files
+        SnapshotProducer::validate_added_data_files(table, 
&self.added_data_files)?;
 
-    /// Finished building the action and apply it to the transaction.
-    pub async fn apply(self) -> Result<Transaction> {

Review Comment:
   Updated the PR description accordingly as well



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to