[GitHub] [hudi] vinothchandar commented on a change in pull request #2141: [HUDI-898] Add new backwards compatible API to expose schema in preCombine

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

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



##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordPayload.java
##########
@@ -41,9 +41,23 @@
    * When more than one HoodieRecord have the same HoodieKey, this function combines them before attempting to
    * insert/upsert (if combining turned on in HoodieClientConfig).
    */
-  @PublicAPIMethod(maturity = ApiMaturityLevel.STABLE)
+  @Deprecated
   T preCombine(T another);
 
+  /**
+   * When more than one HoodieRecord have the same HoodieKey, this function combines them before attempting to
+   * insert/upsert (if combining turned on in HoodieClientConfig).
+   * Any custom Record payload implementation that requires schema must override this method.
+   *
+   * @param another Record payload to combine with.
+   * @param schema  Schema used for writing to dataset.
+   * @return Combined Record.
+   */
+  @PublicAPIMethod(maturity = ApiMaturityLevel.STABLE)
+  default T preCombine(T another, Schema schema) {

Review comment:
       Could we just introduce a config that simply uses `combineAndGetUpdateValue()` to perform the preCombine as well. We have debated doing this for a while. Do you see a reason not to do something like that.  




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