nsivabalan commented on code in PR #13305:
URL: https://github.com/apache/hudi/pull/13305#discussion_r2115067890


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/WriteStatus.java:
##########
@@ -61,6 +61,8 @@ public class WriteStatus implements Serializable {
 
   private final List<Pair<HoodieRecordDelegate, Throwable>> failedRecords = 
new ArrayList<>();
 
+  // true if this WriteStatus refers to a write happening in metadata table.
+  private boolean isMetadataTable;

Review Comment:
   I agree w/ the intent that we should have sub classes. 
   
   lets think through this. 
   metadata table's WriteStatus will be lean. its only the data table write 
status that will have lots of stats like rli, col stats etc will be embedded 
with it. 
   
   Here is what I am thinking. 
   
   Base class we can leave it as is. 
   Add a boolean flag to this class named isMetadataTable().
   We will introduce a sub class named DataTableWriteStatus which will can hold 
additional stats. 
   
   Essentially the base class is common denominator. 
   
   So, in the driver when we process a set of writeStatuses 
   
   ```
   for (WriteStatus writeStatus: list of writeStatus) {
    if (!writeStatus.isMetadataTable()) {
       //. data table write status. 
       DataTableWriteStats dataTableWriteStatus = (DataTableWriteStats) 
writeStatus;
    } else {
       process writeStatus as is. since there is no special handling required 
for metadata table. 
    }
   
   ```
   
   But from a unification standpoint, I don't feel it might make sense. 
   bcoz, for flink and java, data table write status is not going to hold any 
additional stats. But still we might have to fix all those callers to use 
DataTableWriteStats. even though there is no material change for these engines. 
   
   
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to