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

Reply via email to