Skip to content

[core][python] Improve data evolution row-id conflict diagnostics#8491

Merged
JingsongLi merged 1 commit into
apache:masterfrom
JingsongLi:codex/fix-dedicated-row-id-conflict
Jul 7, 2026
Merged

[core][python] Improve data evolution row-id conflict diagnostics#8491
JingsongLi merged 1 commit into
apache:masterfrom
JingsongLi:codex/fix-dedicated-row-id-conflict

Conversation

@JingsongLi

Copy link
Copy Markdown
Contributor

Summary

This PR improves Data Evolution row-id range conflict detection so normal data files and dedicated storage files are checked separately. It prevents adjacent normal data files from being reported without the blob/vector file that actually connects their row-id ranges.

Changes

  • Split Java row-id range conflict checks into normal data-file conflicts and dedicated blob/vector file anchor validation.
  • Added matching PyPaimon logic for blob and vector dedicated files.
  • Added Java and Python regression coverage for a dedicated file spanning adjacent data-file ranges.

Testing

  • mvn -pl paimon-core -Pfast-build -Dtest=ConflictDetectionTest surefire:test
  • PYTHONPATH=paimon-python python -m pytest paimon-python/pypaimon/tests/write/conflict_detection_test.py -q

Notes

The Java test was run with the surefire goal after locally compiling the touched classes, because the full local paimon-core test lifecycle is currently blocked by unrelated compile errors in this checkout.

@JingsongLi JingsongLi changed the title Improve data evolution row-id conflict diagnostics [core][python] Improve data evolution row-id conflict diagnostics Jul 7, 2026
@JingsongLi JingsongLi force-pushed the codex/fix-dedicated-row-id-conflict branch 2 times, most recently from 7806961 to 9d0e3cc Compare July 7, 2026 07:59
@leaves12138

Copy link
Copy Markdown
Contributor

Thanks for the clarification. If the goal of this PR is only to improve the diagnostics, I think this direction is reasonable.

Before merging, could we please tighten two small points so Java and PyPaimon stay aligned and the intended non-conflict cases are covered?

  1. PyPaimon consistency: check_row_id_existence still special-cases only DataFileMeta.is_blob_file(...). It would be better to reuse the new _is_dedicated_file(...) helper there as well, so vector files follow the same semantics as Java's dedicatedStorageFile(...).

  2. Positive regression coverage: the new negative test for a dedicated file spanning multiple data-file ranges is useful. Could we also add positive cases in both Java and Python for:

    • adjacent normal data files without a dedicated bridge should not conflict;
    • a dedicated file fully covered by one normal data-file range should not conflict.

With those covered, this looks good to merge as a diagnostics-only improvement. This PR should not be interpreted as allowing blob/vector compaction to span multiple normal data-file row-id ranges.

@JingsongLi JingsongLi force-pushed the codex/fix-dedicated-row-id-conflict branch from 9d0e3cc to 4f4c689 Compare July 7, 2026 08:22
@JingsongLi

Copy link
Copy Markdown
Contributor Author

Addressed the review comments in the latest push:\n\n- PyPaimon check_row_id_existence now reuses _is_dedicated_file(...), so vector files follow the same semantics as blob files.\n- Added Java and Python positive coverage for adjacent normal data files without a dedicated bridge.\n- Added Java and Python positive coverage for a dedicated file fully covered by one normal data-file range.\n\nVerified locally with:\n- mvn -pl paimon-core -DskipTests spotless:check\n- mvn -pl paimon-core -Pfast-build -Dtest=ConflictDetectionTest surefire:test after compiling the changed test class\n- PYTHONPATH=paimon-python python -m pytest paimon-python/pypaimon/tests/write/conflict_detection_test.py -q

@leaves12138 leaves12138 left a comment

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.

LGTM. Reviewed the row-id conflict diagnostics changes for Java and Python, and verified with targeted ConflictDetection tests.

@JingsongLi JingsongLi merged commit e194867 into apache:master Jul 7, 2026
17 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.

2 participants