You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "linliu-code (via GitHub)" <gi...@apache.org> on 2026/09/15 13:43:48 UTC
[PR] feat(cpp): port the cxx bridge onto core behind an ffi_support facade [hudi-rs]
linliu-code opened a new pull request, #782:
URL: https://github.com/apache/hudi-rs/pull/782
## What this is
Position **3 of 4** in a series porting work from Onehouse's `hudi-rs` fork onto upstream `main`. The series is linear and must land in order: **01 → 02 → 03 → 04**.
**Draft** — this cannot merge until #780 and any earlier position land.
A PR to `apache/hudi-rs` must be based on a branch in this repository, so the series cannot be stacked on GitHub. This branch therefore carries the earlier positions' commits too, and the "Files changed" view is cumulative. **The work unique to this PR is 13 commits, `650af3bd..1c6e09c6`** — please review those.
## What it does
Ports Onehouse's C++ (cxx) bridge onto the OSS core, behind a deliberately narrow `ffi_support` facade, so the bridge depends on one declared surface rather than reaching into `hudi-core` internals.
- `crates/core/src/ffi_support/mod.rs` — the facade: a re-export surface plus a process-wide object-store runtime and a stream-stats handle. `crates/core/tests/visibility.rs` asserts the seam **at compile time**, so widening it fails the build rather than a review.
- `cpp/src/` — the bridge: context, provider ABI, predicate translation, and a blocking merge-stream adapter.
## Scale, and how to review it
This is large, and most of it is not new. Roughly **7,600 lines are vendored or moved** and verified byte-identical (or near-identical) against their existing originals; about **700 lines are genuinely new logic**, concentrated in `blocking_merge_stream.rs`, the `ffi_support` facade, and `engine.rs`'s `open()` / `stream_stats_handle()`. The rest is tests. Reviewing the facade plus those 700 lines is the high-value pass.
## Known, and stated deliberately
- **The provider path is dormant.** Neither entry point calls `with_base_file_provider`, and the stats `OnceLock` is never set. Both paths `warn_once!` if a caller supplies a handle. This is Phase 1; the wiring lands later.
- **A panic in the merge would abort the process.** `BlockingMergeStream::next` is reached from arrow-rs's `extern "C" fn get_next`, which carries no `catch_unwind`. The one nameable panic source (tokio re-entry) is guarded; the general case is not.
- **One error path leaks the provider handle.** `new_file_group_reader_with_context` reclaims it after a fallible `Storage::new`, so a failure in between leaks the `ctx`. One-line move, no ABI change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): port the cxx bridge onto core behind an ffi_support facade [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code closed pull request #782: feat(cpp): port the cxx bridge onto core behind an ffi_support facade
URL: https://github.com/apache/hudi-rs/pull/782
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): port the cxx bridge onto core behind an ffi_support facade [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #782:
URL: https://github.com/apache/hudi-rs/pull/782#issuecomment-5701458018
## Do not review yet — this does not compile
Flagging a defect I introduced, before anyone spends time on it.
This branch replaces `cpp/src/lib.rs` with the ported bridge, which exports `new_file_group_reader_with_context`. It does **not** carry a matching change to the demo, which still calls the previous name:
```
demo/apps/hudi-file-group-api/cpp/main.cpp:142:34: error:
'new_file_group_reader_with_options' was not declared in this scope;
did you mean 'new_file_group_reader_with_context'?
```
`main` is self-consistent today — it exports `new_file_group_reader_with_options` and the demo calls it. The port renames the entry point and leaves the caller behind, so `integration-tests (hudi-file-group-api/cpp)` fails to build.
**Scope:** this affects this PR and the one stacked after it. The two earlier positions in the series (#780, #781) do not touch `cpp/src/lib.rs` and are unaffected.
**How it got here:** the branch was assembled by cherry-pick and pushed without a local build — the crate does not currently build in my environment, and CI had not run here yet, so nothing caught it. That is on me, not on the original author.
**What happens next:** the same failure exists on the internal PR this was ported from, and is being fixed there. I will re-port once that lands rather than invent a rename here that diverges from the internal series. Staying in draft until then.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): port the cxx bridge onto core behind an ffi_support facade [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #782:
URL: https://github.com/apache/hudi-rs/pull/782#issuecomment-5768833757
Superseded by a 24-position stack in `linliu-code/hudi-rs` (#2 … #25), replayed onto current apache main.
These four were each based on `main` rather than on each other, so their diffs were cumulative — #783 carried 47 commits and ~12.9k lines, which is the opposite of reviewable. #782 and #783 also did not compile: the ported bridge exported only `new_file_group_reader_with_context` while the demo's `main.cpp` still called `new_file_group_reader_with_options`.
The replacement is a genuine stack — each PR is based on the position below it, so each shows only its own delta (19 of 24 are under 1,500 lines). The tip is verified: `cargo check --workspace --all-targets` clean, `cargo test -p hudi-core --lib` 1716 passed / 0 failed, `cargo fmt --all --check` clean.
One thing worth carrying over from #780: its `demo/infra/mc/Dockerfile` fix has been **superseded upstream** by #799, which reached the same fix independently and pins the tag rather than using `:latest`. That commit is dropped from the new stack.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org