lokeshj1703 opened a new pull request, #19749:
URL: https://github.com/apache/hudi/pull/19749

   ### Describe the issue this Pull Request addresses
   
   closes #19748
   
   ### Summary and Changelog
   
   On a table-version-6 MOR table using `PostgresDebeziumAvroPayload`, an 
unchanged Postgres TOAST column (emitted as the sentinel 
`__debezium_unavailable_value`) leaked to readers instead of being backfilled 
from the prior value.
   
   The payload backfills by mutating the incoming Avro record in place and 
returning the same reference. `HoodieAvroRecordMerger.merge` had an identity 
shortcut `if (updatedRecord == newerAvroRecord) return newer;` that returned 
the engine-native `newer` record, which never received the in-place mutation, 
so the merge result was discarded. This removes the shortcut so the result is 
always rebuilt from `updatedRecord`.
   
   There are two ways to fix this. This PR takes the merger-side approach, 
which also protects any other payload that mutates the incoming record in place 
and returns the same reference. The alternative, #19280, changes 
`PostgresDebeziumAvroPayload` to return a new record on backfill (preserving 
the merger shortcut). Only one of the two is needed; opening this to compare 
the approaches.
   
   Adds `TestPostgresDebeziumToastV6ReadMerge`, which is red without the fix 
(read returns the raw sentinel) and green with it.
   
   ### Impact
   
   Correctness is restored for payloads that backfill by mutating the incoming 
record in place. Trade-off: the removed shortcut was a general fast-path for 
the common "newer record wins" case of every CUSTOM merge-mode payload (not 
just Debezium), so the result is now always rebuilt via `convertAvroRecord` + 
`BufferedRecords.fromEngineRecord`. That adds a per-record avro-to-engine 
round-trip on the CUSTOM-mode merge path (v6 MOR and any custom-payload table), 
used by both snapshot reads and compaction. The payload-side alternative #19280 
avoids this by returning a new record only when a backfill occurs; that is the 
main reason to prefer one approach over the other.
   
   ### Risk Level
   
   low. Behavior change is confined to the record merge path and covered by a 
new functional test.
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [ ] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [ ] Enough context is provided in the sections above
   - [ ] Adequate tests were added if applicable
   


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