ZENOTME commented on code in PR #797: URL: https://github.com/apache/iceberg-rust/pull/797#discussion_r1899922784
########## crates/iceberg/src/spec/manifest.rs: ########## @@ -1189,6 +1211,47 @@ impl DataFile { self.sort_order_id } } + +/// Convert data files to avro bytes. +pub fn data_files_to_avro( Review Comment: > To maintain api consistency, I would suggest to have sth like following: > > ```rust > /// comments > pub struct DataFileWriter<W> { > writer: W, > version: FormatVersion, > partition_type: &StructType > } > > impl<W> DataFileWriter<W> { > pub fn v1_writer(w: W, partition_type: &StructType) { > } > } > > impl<W: Write> DataFileWriter { > pub fn append(data_file: &DataFile) { ... } > pub fn fluse(mut self) {...} > } > ``` > > It would be better to have similar data structures for reader. We couldn't have something like following because of https://github.com/apache/avro-rs/issues/56. This means that we have to store the DataFile in DataFileWriter again. I'm concerned is worth doing that.🤔 ``` pub struct DataFileWriter<'a, W> { avro_writer: AvroWriter<'a, W>, version: FormatVersion, } ``` -- 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