[GitHub] [hudi] vinothchandar commented on a change in pull request #1929: [HUDI-1160] Support update partial fields for CoW table

[GitBox <gi...@apache.org>]

vinothchandar commented on a change in pull request #1929:
URL: https://github.com/apache/hudi/pull/1929#discussion_r511534369



##########
File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -94,6 +95,11 @@
   public static final String BULKINSERT_SORT_MODE = "hoodie.bulkinsert.sort.mode";
   public static final String DEFAULT_BULKINSERT_SORT_MODE = BulkInsertSortMode.GLOBAL_SORT
       .toString();
+  public static final String DELETE_MARKER_FIELD_PROP = "hoodie.write.delete.marker.field";

Review comment:
       +1 

##########
File path: hudi-client/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -117,7 +118,17 @@ public boolean commitStats(String instantTime, List<HoodieWriteStat> stats, Opti
     if (extraMetadata.isPresent()) {
       extraMetadata.get().forEach(metadata::addMetadata);
     }
-    metadata.addMetadata(HoodieCommitMetadata.SCHEMA_KEY, config.getSchema());
+    String schema = config.getSchema();
+    if (config.updatePartialFields()) {

Review comment:
       Can we have this so that whether or not we can update partial fields is simply controlled by the payload impl. 
   I am concerned that a config like `config.updatePartialFields` is not inline with how we treat payloads. for e.g for MOR, there is no writeConfig when we merge the fields via record reader 

##########
File path: hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
##########
@@ -94,6 +95,11 @@
   public static final String BULKINSERT_SORT_MODE = "hoodie.bulkinsert.sort.mode";
   public static final String DEFAULT_BULKINSERT_SORT_MODE = BulkInsertSortMode.GLOBAL_SORT
       .toString();
+  public static final String DELETE_MARKER_FIELD_PROP = "hoodie.write.delete.marker.field";
+  public static final String DEFAULT_DELETE_MARKER_FIELD = "_hoodie_is_deleted";
+
+  public static final String UPDATE_PARTIAL_FIELDS = "hoodie.update.partial.fields";

Review comment:
       javadocs on what these mean please. 

##########
File path: hudi-client/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -117,7 +118,17 @@ public boolean commitStats(String instantTime, List<HoodieWriteStat> stats, Opti
     if (extraMetadata.isPresent()) {
       extraMetadata.get().forEach(metadata::addMetadata);
     }
-    metadata.addMetadata(HoodieCommitMetadata.SCHEMA_KEY, config.getSchema());
+    String schema = config.getSchema();
+    if (config.updatePartialFields()) {
+      try {
+        TableSchemaResolver resolver = new TableSchemaResolver(table.getMetaClient());

Review comment:
       We can get the last Schema from the commit metadata, right? 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org