Copilot commented on code in PR #249:
URL: https://github.com/apache/fluss-rust/pull/249#discussion_r2771454425
##########
bindings/cpp/include/fluss.hpp:
##########
@@ -490,23 +491,36 @@ class Table {
bool Available() const;
Result NewAppendWriter(AppendWriter& out);
- Result NewLogScanner(LogScanner& out);
- Result NewLogScannerWithProjection(const std::vector<size_t>&
column_indices, LogScanner& out);
- Result NewRecordBatchLogScanner(LogScanner& out);
- Result NewRecordBatchLogScannerWithProjection(const std::vector<size_t>&
column_indices, LogScanner& out);
+ TableScan NewScan();
TableInfo GetTableInfo() const;
TablePath GetTablePath() const;
bool HasPrimaryKey() const;
private:
friend class Connection;
+ friend class TableScan;
Table(ffi::Table* table) noexcept;
void Destroy() noexcept;
ffi::Table* table_{nullptr};
};
+class TableScan {
+public:
+ TableScan& Project(std::vector<size_t> column_indices);
+
+ Result CreateLogScanner(LogScanner& out);
+ Result CreateRecordBatchScanner(LogScanner& out);
+
+private:
+ friend class Table;
+ explicit TableScan(ffi::Table* table) noexcept;
+
+ ffi::Table* table_{nullptr};
+ std::vector<size_t> projection_;
+};
Review Comment:
The TableScan class holds a non-owning raw pointer to ffi::Table* but
doesn't explicitly delete copy/move operations or define a destructor. This
could lead to dangling pointer issues if a TableScan outlives the Table that
created it. While the intended usage (immediate method chaining like
table.NewScan().CreateLogScanner(scanner)) is safe, the class doesn't prevent
unsafe usage patterns. Consider either: (1) explicitly deleting copy
constructor and copy assignment operator while keeping default move semantics,
or (2) adding documentation to clarify that TableScan must not outlive the
Table object, or (3) using a shared_ptr or similar RAII mechanism if longer
lifetimes are needed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
