Skip to content

[python] Reject overlapping row-id update batches - #9484

Merged
JingsongLi merged 2 commits into
apache:masterfrom
YannByron:codex/table-update-row-id-overlap
Aug 31, 2026
Merged

[python] Reject overlapping row-id update batches#9484
JingsongLi merged 2 commits into
apache:masterfrom
YannByron:codex/table-update-row-id-overlap

Conversation

@YannByron

Copy link
Copy Markdown
Contributor

Purpose

Prevent local BatchTableUpdate.update_by_arrow_batches_with_row_id calls from silently producing conflicting rewrites when multiple input batches target the same data file.

The update now tracks each batch's first_row_id file groups, rejects overlap, and aborts files staged by earlier batches. This change is intentionally limited to the local table-update path; distributed Ray updates already coalesce rows by target file.

Tests

  • python3.13 -m pytest -q pypaimon/tests/table_update_test.py pypaimon/tests/table_update_by_row_id_chunked_test.py
  • flake8 on the three changed files with dev/cfg.ini
  • python3.13 -m py_compile on the three changed files

Reject local batch updates that target the same first-row-id file group and abort files staged by earlier batches.

Co-Authored-By: Codex <noreply@anthropic.com>
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5
AI-Contributed/Feature: 20/20
AI-Contributed/UT: 29/29
updater = TableUpdateByRowId(
self.table, self.commit_user, commit_identifier)
updater.update_columns(table, cols)
overlapping_first_row_ids = updated_first_row_ids.intersection(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Detect overlap before staging the current batch

Could we perform this check before update_columns writes the current batch? At this point, update_columns has already read and rewritten every targeted file group. For the overlapping-batch case this change is intended to reject, we therefore rewrite potentially large data or blob files only to abort them, and cleanup is best-effort, so a failed deletion can leave orphan files. Please split or extend update_columns so it calculates the first_row_id set, validates it against the previously seen set, and only then writes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed in 4944d1d: TableUpdateByRowId.update_columns now calculates the current first_row_id set and checks it against prior updates before _write_by_first_row_id, so the conflicting batch is never staged. The outer batch operation still aborts files staged by earlier batches. I also added a unit test asserting the write method is called only for the first batch.

updater = TableUpdateByRowId(
self.table, self.commit_user, commit_identifier)
updater.update_columns(table, cols)
overlapping_first_row_ids = updated_first_row_ids.intersection(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Batches with the same first_row_id should not conflict when they update different columns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Fixed in 4944d1d by tracking first_row_id groups per updated column. Batches that target the same file group with disjoint columns are now allowed; only overlap on the same column is rejected. Added an end-to-end test updating age and city in separate batches within the same file group.

Track updated first-row-id groups per column in the reused updater, reject conflicting overlap before staging the current batch, and allow disjoint columns to update the same file group.

Co-Authored-By: Codex <noreply@anthropic.com>
Co-Authored-By: Codex <noreply@openai.com>
AI-Model: gpt-5
AI-Contributed/Feature: 33/33
AI-Contributed/UT: 50/50
@JingsongLi

Copy link
Copy Markdown
Contributor

+1

@JingsongLi
JingsongLi merged commit ae83ef3 into apache:master Aug 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants