JingsongLi commented on PR #10161:
URL: https://github.com/apache/paimon/pull/10161#issuecomment-5832070923

   Reviewed the current head across option parsing, read/write merge dispatch, 
partial-update row-kind handling, and split planning. This has a real 
cross-engine use case: a Python reader should be able to consume an 
`ignore-delete` partial-update table containing Java-written retract rows. The 
implementation skips DELETE/UPDATE_BEFORE in `PartialUpdateMergeFunction`, 
keeps the default non-ignore path rejecting them, and now uses the same 
canonical boolean parser as `CoreOptions`. I also checked that the primary-key 
scan's raw-convertible path requires no delete rows, so a file with recorded 
retracts should enter the merge path.
   
   Local verification: 71 partial-update, merge-function, dispatch, and 
adjacent aggregation tests passed; `git diff --check` passed. The head's Python 
version matrix and Native CI are green. The earlier `ignore-delete=yes` parser 
discrepancy is addressed.
   
   **Production verification gap:** the new real-table test writes only INSERT 
rows, and the DELETE/UPDATE_BEFORE behavior is tested only by calling the merge 
function directly. Python's current writer always emits INSERT row kinds, so 
that test does not exercise the actual use case through manifest planning, file 
reading, and merge dispatch. Please add a Java-written (or equivalent 
persisted-file) DELETE/UPDATE_BEFORE fixture with `ignore-delete=true`, then 
assert the Python table read retains the correct merged row; also cover a 
retract-only key. I would hold production merge until this end-to-end path is 
verified.
   


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