Skip to content

[python] Enable native scalar Blob Arrow writes - #10305

Merged
JingsongLi merged 2 commits into
apache:masterfrom
JingsongLi:codex/native-blob-write
Sep 30, 2026
Merged

JingsongLi merged 2 commits into
apache:masterfrom
JingsongLi:codex/native-blob-write

Conversation

@JingsongLi

Copy link
Copy Markdown
Contributor

Purpose

Enable scalar BLOB Arrow writes through PyPaimon Native and verify that the resulting files remain interoperable with Python readers and committers.

Changes

  • Open the native write route for top-level scalar BLOB fields. ARRAY/MAP BLOB, video and optional row-sidecar routes retain their existing Python implementation.
  • Select the Python writer on the first write_row for a BLOB table, preserving custom Blob streams and URI readers without materialization. Switching from already-started native Arrow writes remains rejected and is documented.
  • Align Python inline descriptor validation with Java's known-descriptor prefix parser, including valid v1/v2 payloads with trailing bytes.
  • Exercise normal/Blob roll grouping, file metadata, write-cols optimization, stream reuse, external paths and both committers. Assert native execution and cross-read every result with Python and Native readers/planners.
  • Add cleanup, streaming-row, inline descriptor and HTTP descriptor regressions, including gzip/deflate, unknown lengths, error cleanup and an 18 MiB copy requiring a single GET.
  • Make existing tests explicit about Python internals and unordered cross-partition scans; keep the deferred-reader limit tests supplied with both partitions.

Dependency

Requires apache/paimon-rust#989. This PR is draft until that change is merged. CI continues installing apache/paimon-rust@main; no fork or branch pin is introduced.

Verification

Tests run with the locally rebuilt Rust binding, PyArrow 18 and all five Native CI options enabled. Flake8 and git diff checks pass.

Expanded regression: 1,238 passed, 3 skipped, 67 subtests passed across native writes, updates, nested updates, external paths, data directories, Blob read/write, deferred resolution, commit, write buffers and data evolution. Three optional Vortex cases were deselected because the local Vortex dependency is absent.

Native execution counters: 1,044 plans, 1,425 reads, 937 writes and 7 commits; updates exercised row-id (84), grouped (53), predicate (57), upsert (34) and incremental (15) paths.

@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.

Reviewed together with apache/paimon-rust#989 at ae58da0f07184266b3abc6f3f3517105b1f96adf, using a freshly built binding from that exact commit, PyArrow 18, and all five Native CI options enabled. The 47 new native_blob_write_test.py cases passed. An expanded local run had 1,021 passed, 1 skipped and 10 optional Vortex cases deselected, but exposed the abort regression below.

There is also a remaining Native CI test-adaptation issue: pypaimon/tests/ray_sink_test.py::RaySinkTest::test_write_does_not_return_prepared_messages_when_dedicated_close_aborts patches DedicatedFormatWriter._close_current_writers, but this table now selects NativeTableWrite. The mock is never reached, so the expected RuntimeError is not raised. Please explicitly keep that Python-specific fault-injection test on the Python writer (as done for the other Python-internal tests), or adapt the injection to the native path while retaining the failure/cleanup coverage.

Both that Ray test and DataEvolutionFormatsTest::test_blob_abort_deletes_uncommitted_files pass on this PR's base d3393eb with the same Rust binding, and fail on this head. Thus these two failures remain even after satisfying the Rust dependency; merging #989 alone will not make Native CI green. The four DataEvolutionChunkShuffleEndToEndTest cases pass with the new binding. No source fixes have been pushed as part of this review.

Comment thread paimon-python/pypaimon/write/native_write.py
Track prepared native messages until handoff to the committer or close. Clean up prepared files on explicit abort without deleting files from successful or uncertain commit attempts. Cover Blob rolling, external paths, streaming reuse, and native REST publication failures; keep Ray Python fault injection on the Python writer.

@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.

Fixed the two requested changes in d234756 and re-reviewed the resulting diff. Explicit native-writer abort now cleans prepared messages still owned by the writer, including external files and index sidecars. Close releases prepared output to the caller without deleting it. TableCommit relinquishes writer cleanup ownership before publication, so abort after either successful submission or an ambiguous commit exception cannot delete those submitted files. The Python-specific Ray fault-injection test is now explicitly marked python_write. The lifecycle contract is documented.

Added 15 regression cases covering batch/stream prepared cleanup, repeated abort, outstanding output, external locations, close-before-commit, stream reuse across committed and uncommitted batches, and failures before/after publication. The native REST publication tests assert actual native committer selection and prohibit Python fallback; DE Blob tables intentionally retain their existing Python commit fallback even when commit.native.enabled is true.

Validation against a freshly built apache/paimon-rust#989 binding at ae58da0: expanded Native-mode run 1,423 passed, 3 skipped, 10 optional Vortex cases deselected; Python-mode regression 89 passed. Native counters included 973 writes, 41 native commit preparations, and all five update entry points. Changed Python files pass the repository flake8 configuration, and git diff --check passes. Both previously failing tests now pass.

No remaining blocking code issue found in this paired review. This approval is based on the paired heads; keep the documented merge order (#989 first), then rerun the GitHub Native CI against Rust main. It does not mean the dependency has merged or that the remote CI is already green.

@JingsongLi
JingsongLi marked this pull request as ready for review September 29, 2026 06:02
@JingsongLi
JingsongLi merged commit 8dd8549 into apache:master Sep 30, 2026
26 of 28 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