Add missing custom properties method and doc for table descriptor builder in C++. Add assertion for custom property in integration test.#336
Conversation
|
@fresh-borzoni Appreciate your review here 🙏 |
|
@leekeiabstraction |
|
Converted to draft, will push the doc part after #330 gets merged |
|
@leekeiabstraction Hi, #330 is merged |
leekeiabstraction
force-pushed
the
tableBuildDescriptor
branch
from
703c1d0 to
1c2b980
Compare
|
Thank you for the reviews. I've updated doc follow #330 merge. PTAL! |
There was a problem hiding this comment.
Pull request overview
This PR updates API reference documentation for both Rust and C++ bindings to document the custom properties functionality in TableDescriptor/TableDescriptorBuilder and TableInfo. The custom properties feature allows users to attach arbitrary key-value metadata to tables beyond the standard table properties.
Changes:
- Added
custom_properties()method documentation for RustTableDescriptor - Added
custom_property()andcustom_properties()method documentation for RustTableDescriptorBuilder - Added
SetCustomProperty()method documentation for C++TableDescriptor::Builder - Added
custom_propertiesfield documentation for C++TableInfo
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| website/docs/user-guide/rust/api-reference.md | Added documentation for custom_properties getter and custom_property/custom_properties setter methods in TableDescriptor/TableDescriptorBuilder |
| website/docs/user-guide/cpp/api-reference.md | Added documentation for SetCustomProperty method in TableDescriptor::Builder and custom_properties field in TableInfo |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `SetBucketKeys(const std::vector<std::string>& keys) -> Builder&` | Set bucket key columns | | ||
| | `SetProperty(const std::string& key, const std::string& value) -> Builder&` | Set a table property | | ||
| | `SetCustomProperty(const std::string& key, const std::string& value) -> Builder&` | Set a custom property | | ||
| | `SetComment(const std::string& comment) -> Builder&` | Set a table comment | |
There was a problem hiding this comment.
The C++ TableDescriptor::Builder has SetLogFormat and SetKvFormat helper methods (as seen in bindings/cpp/include/fluss.hpp lines 415-421), but these are not documented in this API reference table. These methods should be added to the documentation for completeness, similar to how log_format and kv_format are documented for the Rust API.
| | `SetComment(const std::string& comment) -> Builder&` | Set a table comment | | |
| | `SetComment(const std::string& comment) -> Builder&` | Set a table comment | | |
| | `SetLogFormat(const std::string& format) -> Builder&` | Set the log format | | |
| | `SetKvFormat(const std::string& format) -> Builder&` | Set the key-value format | |
There was a problem hiding this comment.
Since currently, only default format is support. So, not document the api is fine to me
| | `fn custom_properties(&self) -> &HashMap<String, String>` | Get custom properties | | ||
| | `fn comment(&self) -> Option<&str>` | Get table comment | |
There was a problem hiding this comment.
The PR description mentions "Add custom properties assertions in integration test" and "Integration tests ran and completed", but this PR only contains documentation updates and no test changes are visible in the diff. The integration test changes may have been committed separately or the PR description may need to be updated to accurately reflect the scope of changes.
3 participants
Purpose
Linked issue: close #335
Brief change log
Tests
Integration tests ran and completed