[client] Add lookup support for primary key tables #159
Conversation
Implements lookup functionality to verify put kv operations (Issue apache#114). - Add Lookup API key (1017) to api_key.rs - Add lookup proto messages (LookupRequest, LookupResponse, etc.) - Create lookup.rs message module with LookupRequest - Add async lookup() method to FlussTable
|
@luoyuxia good morning, i didn't know if u were already working on this, i'm sorry if its the case @leekeiabstraction mr.lee as you are proficient in java and i'm not it would be really appreciated if you can check this out too Have a nice day, open to feedbacks/changes |
There was a problem hiding this comment.
Pull request overview
This PR adds lookup functionality for primary key tables, enabling clients to retrieve values by key from key-value tables. The implementation follows the simple lookup approach without complex batching/queuing, mirroring the Java client implementation.
Changes:
- Added Lookup API (key 1017) to support lookup operations
- Implemented protobuf messages for lookup requests and responses
- Added
lookup()async method toFlussTablefor key-value retrieval
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/fluss/src/rpc/api_key.rs | Added Lookup API key (1017) enum variant and conversions |
| crates/fluss/src/proto/fluss_api.proto | Defined protobuf messages for lookup requests and responses |
| crates/fluss/src/rpc/message/lookup.rs | Implemented LookupRequest struct with RequestBody trait |
| crates/fluss/src/rpc/message/mod.rs | Added lookup module import and export |
| crates/fluss/src/client/table/mod.rs | Added public lookup() method to FlussTable for key retrieval |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| message PbValue { | ||
| optional bytes values = 1; |
There was a problem hiding this comment.
The field name 'values' (plural) is misleading since PbValue represents a single value. Consider renaming to 'value' (singular) for clarity.
| optional bytes values = 1; | |
| optional bytes value = 1; |
leekeiabstraction
left a comment
leekeiabstraction
left a comment
There was a problem hiding this comment.
Thank you for the PR! I think while we do not need to implement queuing/batching, we should still have the abstraction/interfaces that users use consistent with the rest of the FlussTable function e.g. TableAppend, TableUpsert. These are public contract that would be difficult to change once released without breaking user's code.
luoyuxia
left a comment
luoyuxia
left a comment
There was a problem hiding this comment.
@AndreaBozzo Thanks for the pr. Just one comment to keep api align with java side.
3 participants
Purpose
Linked issue: close #114
Implements lookup functionality to verify put kv operations. This follows the simple lookup approach as suggested in the issue comments (without complex batching/queuing).
Brief change log
LookupAPI key (1017) toapi_key.rs- matching Java Fluss ApiKeys.javaLookupRequest,LookupResponse,PbLookupReqForBucket,PbLookupRespForBucket,PbValue) tofluss_api.protolookup.rsmessage module withLookupRequeststruct implementingRequestBodytraitlookup(bucket_id, key)method toFlussTablethat returnsOption<Vec<u8>>Reference Java implementation:
Tests
API and Format
Adds new public API:
FlussTable::lookup(bucket_id: i32, key: Vec<u8>) -> Result<Option<Vec<u8>>>Documentation
No documentation changes required at this time.