bhasudha commented on pull request #1704: URL: https://github.com/apache/hudi/pull/1704#issuecomment-663954904
> @bhasudha The PR looks good to me. Looks like the same ordering field will be honored in all places. One high level question before I accept it -> If `preCombine` & `combineAndGetUpdateValue` are using the same `orderingVal`, I'm guessing it is expected from the user to use the constructor with the `orderingVal` and up to the user to ensure the `orderingVal` used in the constructor is the same as the one passed in `Map<..>`. If this is true, does `HoodieDeltaStreamer` allow for this kind of constructor invocation ? > Also, please rebase and push the PR, once the build succeeds can merge it. @n3nash Thanks! Yes user needs to ensure that the Map uses the same orderingVal as the one configured for precombine. We are kind of abstracting this logic for users by passing in the precombine field ourselves in the HoodieMergeHandle where this method is invoked. In DeltaStreamer this constructor invocation is happening already here - https://github.com/apache/hudi/blob/0cb24e4a2defd8e639437b6cd145a26f038ef1af/hudi-utilities/src/main/java/org/apache/hudi/utilities/deltastreamer/DeltaSync.java#L341 which passes the cfg.sourceOrderingField for ordering val. And in this PR we are also setting the same cfg.sourceOrderingField in the https://github.com/apache/hudi/pull/1704/files#diff-2f9f867c64be16dd87a816fe90f88e87R517 which is what is used in the MergeHandle - https://github.com/apache/hudi/pull/1704/files#diff-78cfbf493ffb0a24c10b52d9602531a1R222 . Hope that answers your question. ---------------------------------------------------------------- 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: [email protected]
