leekeiabstraction commented on code in PR #365:
URL: https://github.com/apache/fluss-rust/pull/365#discussion_r2839689911
##########
crates/fluss/src/row/mod.rs:
##########
@@ -63,58 +65,58 @@ pub trait InternalRow: Send + Sync {
fn is_null_at(&self, pos: usize) -> bool;
/// Returns the boolean value at the given position
- fn get_boolean(&self, pos: usize) -> bool;
+ fn get_boolean(&self, pos: usize) -> Result<bool>;
Review Comment:
I agree returning bool directly does make the simplest API albeit unsafe.
There's currently two forms of panic:
1. When user call get_bool directly on a column that is not bool (either
before append / upsert or after poll / lookup)
2. When fluss-rust attempt to append or upsert a malformed row that user has
provided.
For the first one, while I do prefer Result option (and Rust has good
support around Result), I am not too strongly opinionated. I can change this PR
so that better panic message is provided.
On the second part, this PR will make it safe by checking column count and
type (append currently does not check column count).
--
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]
