nsivabalan commented on PR #13580:
URL: https://github.com/apache/hudi/pull/13580#issuecomment-3111550913

   > Feedback is not yet fully addressed :( I did callout that BaseMergeHelper 
needs to be fixed so that cow merge uses new way of merging. i.e. instead of 
calling
   > 
   > ```
   > mergeHandle.write(HoodieRecord<T> oldRecord)
   > ```
   > 
   > we need to leverage
   > 
   > ```
   > mergeHandle.write() 
   > ```
   > 
   > so that FG reader will be used. how did you even validate that new FG 
reader is used for COW merges. can you point me to test case where you 
validated this
   
   
   
   I tired executing a simple COW merge w/ this patch, and I do see we are 
hitting BaseMergeHelper.consume() method :( 
   Then, I added breakpoint in FilegroupReaderBasedMergeHandle constructor and 
I don't see it being invoked. With this patch, we are still using 
HoodieWriteMergeHandle. :( 
   


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