charlesdong1991 commented on code in PR #365:
URL: https://github.com/apache/fluss-rust/pull/365#discussion_r2838234753
##########
crates/fluss/src/row/column.rs:
##########
@@ -307,60 +344,56 @@ impl InternalRow for ColumnarRow {
precision as u32,
scale as u32,
)
- .unwrap_or_else(|e| {
- panic!(
- "Failed to create Decimal at column {} row {}: {}",
- pos, self.row_id, e
- )
- })
}
- fn get_date(&self, pos: usize) -> crate::row::datum::Date {
- crate::row::datum::Date::new(self.read_date_from_arrow(pos))
+ fn get_date(&self, pos: usize) -> Result<Date> {
+ Ok(Date::new(self.read_date_from_arrow(pos)))
Review Comment:
it seems this `read_date_from_arrow` still uses `expect` and i think will
panic on type mismatch? maybe we should update helper functions as well?
##########
crates/fluss/src/row/mod.rs:
##########
@@ -127,98 +129,149 @@ pub struct GenericRow<'a> {
pub values: Vec<Datum<'a>>,
}
+impl<'a> GenericRow<'a> {
+ fn get_value(&self, pos: usize) -> Result<&Datum<'a>> {
+ self.values.get(pos).ok_or_else(|| IllegalArgument {
+ message: format!(
+ "position {pos} out of bounds (row has {} fields)",
+ self.values.len()
+ ),
+ })
+ }
+
+ fn try_convert<T: TryFrom<&'a Datum<'a>>>(
+ &'a self,
+ pos: usize,
+ expected_type: &str,
+ ) -> Result<T> {
+ let datum = self.get_value(pos)?;
+ T::try_from(datum).map_err(|_| IllegalArgument {
+ message: format!(
+ "type mismatch at position {pos}: expected {expected_type},
got {datum:?}"
+ ),
+ })
+ }
+}
+
impl<'a> InternalRow for GenericRow<'a> {
fn get_field_count(&self) -> usize {
self.values.len()
}
fn is_null_at(&self, pos: usize) -> bool {
- self.values
- .get(pos)
- .expect("position out of bounds")
- .is_null()
+ self.values.get(pos).is_none_or(|v| v.is_null())
Review Comment:
This changes the semantics from "panic on out-of-bounds" to "treat
out-of-bounds as null", because it is called before calling the typed getter.
should we keep this as panic or change this function to return
`Result<bool>` so out-of-bounds can be surfaced as an error?
--
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]
