ZENOTME commented on PR #135:
URL: https://github.com/apache/iceberg-rust/pull/135#issuecomment-1894811712

   > Trait is usually used to abstract things and hide concrete implementation. 
I don't quite understand the goal of this trait CurrentStatus and when it will 
be useful?
   
   Sorry, Let me try to illustrate more clearly. Let me use a example to 
illustrate it:
   We may have kinds of partition writer in the future: fanout partition writer 
will compute the partition and dispatcher them and precompute partition writer 
will accept the data with the partition value computed before. But both of them 
will expose the same metrics `PartitionMetrics`. Like following:
   ```
   struct PartitionMetrics {
     partition_num: usize 
   }
   
   struct FanoutPartitionWriter 
   
   impl CurrentStatus<PartitionMetrics> for FanoutPartitionWriter
   
   struct PrecomputePartitionWriter
   
   impl CurrentStatus<PartitionMetrics> for PrecomputePartitionWriter
   ```
   And through `CurrentStatus<PartitionMetrics>`, we can write a metrics 
monitor writer and accept both of them as a input.
   ```
   struct PartitionMetricsMonitorWriter<W: IcebergWriter + 
CurrentStatus<PartitionMetrics>> {
     writer: W
   }
   ``` 
   
   ### Do we have another way?
   
   Yes, we also can do like this: 
   ```
   pub trait CurrentFileStatus {
       /// Get the current file path.
       fn current_file_path(&self) -> String;
       /// Get the current file row number.
       fn current_row_num(&self) -> usize;
       /// Get the current file written size.
       fn current_written_size(&self) -> usize;
   }
    
   trait CurrentPartitionMetrics {
     fn metrics() -> PartitionMetrics;
   }
   
   trait CurrentDataFileWriterMetrics {
     fn metrics() -> DataFileWriterMetrics
   }
   ```
   But I feel that all these traits can be abstract as `pub trait 
CurrentStatus<T>` so that instead of defining more traits, we can just define 
more data types in the future. The benefit of this may not be obvious. So both 
of way look good to me.


-- 
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