YannByron commented on code in PR #9445:
URL: https://github.com/apache/paimon/pull/9445#discussion_r3886832613
##########
paimon-python/pypaimon/write/table_update.py:
##########
@@ -166,6 +166,26 @@ def _update_by_arrow_with_row_id(
self.table, self.commit_user, commit_identifier,
).update_columns(table, cols)
+ def _update_by_arrow_batches_with_row_id(
+ self, tables: Iterable[pa.Table], commit_identifier: int
+ ) -> List[CommitMessage]:
+ updater = None
+ try:
+ for table in tables:
+ cols = self.update_cols if self.update_cols is not None else [
+ c for c in table.column_names
+ if c != SpecialFields.ROW_ID.name
+ ]
+ if updater is None:
+ updater = TableUpdateByRowId(
+ self.table, self.commit_user, commit_identifier)
+ updater.update_columns(table, cols)
Review Comment:
Thanks for reproducing this. In the current local RoboMIND backfill path,
updates come directly from Paimon planned splits. DataEvolutionSplitGenerator
groups files by first_row_id before packing splits, and _iter_raw materializes
each complete split as one batch, so the same first_row_id group is not
expected to cross batches in this specific flow.
However, you are right that the local batch API neither expresses nor
enforces this precondition, so arbitrary batches can hit this correctness
issue. I also overlooked the existing Ray implementations. The public Ray
update_by_row_id API already determines first_row_id before writing, shuffles
and groups updates by it, coalesces all updates for the same target file in one
worker, and performs one coordinated commit. RoboMIND tables enable deletion
vectors, so their same-table backfill should instead reuse the existing
optimized Ray self-merge path, which reads live rows from a pinned snapshot and
schedules one update task per target file group. Therefore, no separate
distributed update engine is needed.
For this PR, I will add exact overlap detection for the affected
first_row_ids in the local batch API and fail before commit, aborting staged
files instead of silently losing updates. I will also add the same-file local
end-to-end test and distributed multi-block same-file coverage, and wire the
RoboMIND Ray backfill to the existing self-merge path.
--
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]
