XiaoHongbo-Hope commented on code in PR #8092:
URL: https://github.com/apache/paimon/pull/8092#discussion_r3346208186
##########
paimon-python/pypaimon/write/table_update_by_row_id.py:
##########
@@ -109,12 +109,10 @@ def _load_existing_files_info(self) -> _FilesInfo:
else:
entry[1].append(file)
- # Multiple physical files may share the same first_row_id (data
evolution);
- # summing row_count per file would over-count logical rows and widen
- # the _ROW_ID validation range incorrectly.
+ # Use max row_id + 1 as upper bound; row_id ranges may have holes.
if row_id_ranges:
merged = Range.sort_and_merge_overlap(row_id_ranges, True, True)
- total_row_count = sum(r.count() for r in merged)
+ total_row_count = max(r.to for r in merged) + 1
Review Comment:
> This fixes the valid high row_id case, but using only `max(row_id) + 1` as
the validation bound also makes row IDs in deleted holes look valid. For
example, after truncating the first partition the remaining range can be `[3,
4]`; an `update_by_arrow_with_row_id` for `_ROW_ID = 0` now passes the range
check, `bisect_right([3], 0) - 1` maps it to `first_row_id = 3`, and
`replace_with_mask` ends up with no matching relative index, so the requested
update is silently dropped. Could we keep the actual merged `row_id_ranges` in
`_FilesInfo` and validate that every input row id belongs to one of those
ranges (or otherwise fail) before calculating `_FIRST_ROW_ID`? The upsert path
only needs to allow holes between ranges, not accept updates for rows that no
longer exist.
Updated
--
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]
