-
Notifications
You must be signed in to change notification settings - Fork 27
[TASK-293] Builder pattern, cleanup, consistent API #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@luoyuxia @leekeiabstraction PTAL 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aligns the C++ bindings’ Table/Connection APIs with the Java/Rust-style builder pattern, while consolidating Rust FFI entrypoints (scanner/upsert) to reduce surface area and duplication.
Changes:
- Introduce
TableAppend/TableUpsert/TableLookupbuilder-style APIs (NewAppend/NewUpsert/NewLookup) and update examples accordingly. - Rename
Connection::ConnecttoConnection::Createto match other languages’ naming. - Consolidate Rust FFI scanner creation into
create_scanner(...)and upsert writer creation intocreate_upsert_writer(...).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| bindings/cpp/src/table.cpp | Implements new C++ builder objects for append/upsert/lookup and routes scan creation through consolidated FFI. |
| bindings/cpp/src/lib.rs | Consolidates FFI methods (create_scanner, create_upsert_writer) and refactors Table internals to reuse fluss_table(). |
| bindings/cpp/src/connection.cpp | Renames connection factory from Connect to Create and small include/format cleanup. |
| bindings/cpp/include/fluss.hpp | Exposes new builder classes and updates the public C++ API accordingly. |
| bindings/cpp/examples/kv_example.cpp | Updates example usage to new builder APIs + Connection::Create. |
| bindings/cpp/examples/example.cpp | Updates example usage to new builder APIs + Connection::Create. |
| bindings/cpp/examples/admin_example.cpp | Updates example usage to Connection::Create. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0d26eca to
ea8ac21
Compare
|
Addressed comments |
ea8ac21 to
206f0ad
Compare
206f0ad to
8fbd979
Compare
leekeiabstraction
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, TY for the PR!
luoyuxia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fresh-borzoni Thanks. +1
Summary
closes #293
Connection::ConnecttoConnection::Createto align with Java'screateConnectionAPI changes
Connection
Connection::Connect(addr, conn) -> Connection::Create(addr, conn)
Append
table.NewAppendWriter(writer) -> table.NewAppend().CreateWriter(writer)
Upsert
table.NewUpsertWriter(writer)-> table.NewUpsert().CreateWriter(writer)
table.NewUpsertWriter(writer, names) -> table.NewUpsert().PartialUpdateByName(names).CreateWriter(writer)
table.NewUpsertWriter(writer, indices) -> table.NewUpsert().PartialUpdateByIndex(indices).CreateWriter(writer)
Lookup
table.NewLookuper(lookuper) -> table.NewLookup().CreateLookuper(lookuper)
Rust FFI consolidation