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