Skip to content

[core] Honor external data paths across table writers - #981

Merged
JingsongLi merged 1 commit into
apache:mainfrom
JingsongLi:codex/native-external-data
Sep 28, 2026
Merged

JingsongLi merged 1 commit into
apache:mainfrom
JingsongLi:codex/native-external-data

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Follow up on #974 to honor data-file.external-paths when creating data files. Rust could resolve existing external files and place external deletion vectors, but its table writers still created data files under the table directory. This also kept PyPaimon's external-path tables on the Python writer.

Brief change log

  • Add an internal, Java-style per-bucket data path factory used by append, primary-key, postpone, data-evolution and copy-on-write writers.
  • Share the path provider between normal and dedicated Blob files, and between primary-key data and input changelog files. Select each file's destination once, persist external_path, and place index sidecars beside that destination.
  • Honor round-robin, weight-robin, entropy-inject, specific-fs and none. Match Java's weight validation and disabled-strategy handling.
  • Revalidate the existing primary-key managed Blob restriction when creating a writer so copied table options cannot bypass it.
  • Retain successful physical-column outputs until all closes complete, then clean up all data and sidecar files if any close fails.

Tests

31 focused Rust tests passed:

  • external_data_write_test (5 tests, including the writer/strategy matrix)
  • data_file_directory_test (7 tests)
  • table_update_paths_test (7 tests)
  • table::external_path::tests (8 tests)
  • table::data_file_writer::tests (3 tests)
  • table::dedicated_format_file_writer::tests (1 failure-injection test)

Also passed:

  • cargo fmt --all --check
  • cargo clippy --locked --all-targets --workspace --features fulltext,vortex -- -D warnings
  • Built the Python bindings and ran the companion PyPaimon regression with all five native test flags: 1593 passed, 2 skipped, 74 subtests passed. This includes append/PK/data-evolution interoperability, batch and stream writes, escaped partitions, destination changes, updates/upserts/deletes, historical reads and abort cleanup.

API and Format

No public API or storage-format change. File placement uses the existing Java-compatible options and DataFileMeta.external_path field.

Documentation

The companion PyPaimon PR apache/paimon#10301 enables and documents native external writes. PyPaimon's Blob writer fallback is preserved because its remaining native Blob input/metadata behavior needs separate work.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed b63e4d99290eb781e308840874124aecdd9ef72c together with apache/paimon#10301.

Checked the per-bucket path-provider lifecycle against Java, one-time destination selection, persisted external paths, data/changelog/Blob sharing, sidecar placement, update destinations, abort cleanup, and close-failure ownership. No blocking code-review findings; no Rust implementation changes were needed.

Validation:

  • Core library: 3,413 tests passed, 6 ignored.
  • external_data_write_test, data_file_directory_test, table_update_paths_test, and table_update_test: 40 tests passed.
  • Built the Python native extension from this exact Rust revision and ran the paired Python revision with all five native execution flags: 1,836 passed, 3 skipped, 155 subtests passed (Python 3.10 / PyArrow 18.1.0).
  • cargo fmt --all -- --check passed.
  • All 14 GitHub checks, including Windows/macOS/Linux units, passed for this commit.

The companion Python PR remains dependent on this change and should rerun Native CI against upstream main after this PR merges. LGTM.

@JingsongLi
JingsongLi merged commit fc27163 into apache:main Sep 28, 2026
14 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