Skip to content

chore: make RowView shared_ptr#353

Merged
luoyuxia merged 1 commit intoapache:mainfrom
fresh-borzoni:cpp-scanrecords-shared-ptr
Feb 19, 2026
Merged

chore: make RowView shared_ptr#353
luoyuxia merged 1 commit intoapache:mainfrom
fresh-borzoni:cpp-scanrecords-shared-ptr

Conversation

@fresh-borzoni
Copy link
Contributor

Summary

ScanRecord in C++ held a raw pointer to the underlying scan data, making it a borrowed view that dangled when ScanRecords was destroyed. Users couldn't store or accumulate records across Poll() calls, unlike Rust (Arc) where records are self-contained.

Fixed by wrapping the opaque ScanResultInner in shared_ptr with a custom deleter that calls rust::Box::from_raw(). ScanRecord is now a value type: copyable, storable, safe to accumulate. Zero-copy preserved (same FFI pointer path), no Rust-side changes needed.

Updated docs to replace borrowing warnings with value-type semantics and added an accumulation example to log-tables guide.

@fresh-borzoni
Copy link
Contributor Author

@leekeiabstraction @zhaohaidao PTAL 🙏

Copy link

Copilot AI left a 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 refactors the C++ bindings to make ScanRecord a value type by wrapping the underlying FFI data in std::shared_ptr instead of using raw pointers. Previously, ScanRecord contained a RowView that borrowed from ScanRecords, creating dangling reference issues when users tried to store records beyond the lifetime of the ScanRecords container. The fix enables users to safely accumulate records across multiple Poll() calls, matching the Rust API's semantics.

Changes:

  • Changed RowView to hold shared_ptr instead of raw pointers for both the FFI data and column map
  • Replaced manual memory management in ScanRecords with defaulted constructors/destructors
  • Modified LogScanner::Poll() to wrap the FFI pointer in shared_ptr with a custom deleter that calls rust::Box::from_raw()

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
bindings/cpp/include/fluss.hpp Changed RowView and ScanRecords to use shared_ptr for memory management; defaulted special member functions
bindings/cpp/src/table.cpp Removed manual Destroy() logic; updated Poll() to create shared_ptr with custom deleter
website/docs/user-guide/cpp/api-reference.md Updated documentation to reflect value-type semantics and reference counting
website/docs/user-guide/cpp/data-types.md Revised lifetime documentation and examples to show safe accumulation patterns
website/docs/user-guide/cpp/example/log-tables.md Added examples for continuous polling and accumulating records across polls

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@luoyuxia luoyuxia left a comment

Choose a reason for hiding this comment

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

@fresh-borzoni Thanks. LGTM!

@luoyuxia luoyuxia merged commit 4dff1cb into apache:main Feb 19, 2026
24 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

Comments