fresh-borzoni commented on code in PR #330:
URL: https://github.com/apache/fluss-rust/pull/330#discussion_r2811426706


##########
bindings/cpp/src/lib.rs:
##########
@@ -1417,3 +1510,602 @@ impl LogScanner {
         }
     }
 }
+
+// ============================================================================
+// Opaque types: GenericRowInner (write path)
+// ============================================================================
+
+pub struct GenericRowInner {
+    row: fcore::row::GenericRow<'static>,
+}
+
+fn new_generic_row(field_count: usize) -> Box<GenericRowInner> {
+    Box::new(GenericRowInner {
+        row: fcore::row::GenericRow::new(field_count),
+    })
+}
+
+impl GenericRowInner {
+    fn gr_reset(&mut self) {
+        let len = self.row.values.len();
+        self.row = fcore::row::GenericRow::new(len);
+    }
+
+    fn gr_set_null(&mut self, idx: usize) {
+        self.ensure_size(idx);
+        self.row.set_field(idx, fcore::row::Datum::Null);
+    }
+
+    fn gr_set_bool(&mut self, idx: usize, val: bool) {
+        self.ensure_size(idx);
+        self.row.set_field(idx, fcore::row::Datum::Bool(val));
+    }
+
+    fn gr_set_i32(&mut self, idx: usize, val: i32) {
+        self.ensure_size(idx);
+        self.row.set_field(idx, fcore::row::Datum::Int32(val));
+    }
+
+    fn gr_set_i64(&mut self, idx: usize, val: i64) {
+        self.ensure_size(idx);
+        self.row.set_field(idx, fcore::row::Datum::Int64(val));
+    }
+
+    fn gr_set_f32(&mut self, idx: usize, val: f32) {
+        self.ensure_size(idx);
+        self.row
+            .set_field(idx, fcore::row::Datum::Float32(val.into()));
+    }
+
+    fn gr_set_f64(&mut self, idx: usize, val: f64) {
+        self.ensure_size(idx);
+        self.row
+            .set_field(idx, fcore::row::Datum::Float64(val.into()));
+    }
+
+    fn gr_set_str(&mut self, idx: usize, val: &str) {
+        self.ensure_size(idx);
+        self.row.set_field(
+            idx,
+            
fcore::row::Datum::String(std::borrow::Cow::Owned(val.to_string())),
+        );
+    }
+
+    fn gr_set_bytes(&mut self, idx: usize, val: &[u8]) {
+        self.ensure_size(idx);
+        self.row.set_field(
+            idx,
+            fcore::row::Datum::Blob(std::borrow::Cow::Owned(val.to_vec())),
+        );
+    }
+
+    fn gr_set_date(&mut self, idx: usize, days: i32) {
+        self.ensure_size(idx);
+        self.row
+            .set_field(idx, 
fcore::row::Datum::Date(fcore::row::Date::new(days)));
+    }
+
+    fn gr_set_time(&mut self, idx: usize, millis: i32) {
+        self.ensure_size(idx);
+        self.row
+            .set_field(idx, 
fcore::row::Datum::Time(fcore::row::Time::new(millis)));
+    }
+
+    fn gr_set_ts_ntz(&mut self, idx: usize, millis: i64, nanos: i32) {
+        self.ensure_size(idx);
+        // Use from_millis_nanos, falling back to millis-only on error
+        let ts = fcore::row::TimestampNtz::from_millis_nanos(millis, nanos)
+            .unwrap_or_else(|_| fcore::row::TimestampNtz::new(millis));
+        self.row.set_field(idx, fcore::row::Datum::TimestampNtz(ts));
+    }
+
+    fn gr_set_ts_ltz(&mut self, idx: usize, millis: i64, nanos: i32) {
+        self.ensure_size(idx);
+        let ts = fcore::row::TimestampLtz::from_millis_nanos(millis, nanos)
+            .unwrap_or_else(|_| fcore::row::TimestampLtz::new(millis));
+        self.row.set_field(idx, fcore::row::Datum::TimestampLtz(ts));
+    }
+
+    fn gr_set_decimal_str(&mut self, idx: usize, val: &str) {
+        self.ensure_size(idx);
+        // Store as string; resolve_row_types() will parse and validate 
against schema
+        self.row.set_field(
+            idx,
+            
fcore::row::Datum::String(std::borrow::Cow::Owned(val.to_string())),
+        );
+    }
+
+    fn ensure_size(&mut self, idx: usize) {
+        if self.row.values.len() <= idx {
+            self.row.values.resize(idx + 1, fcore::row::Datum::Null);
+        }
+    }
+}
+
+// ============================================================================
+// Shared row-reading helpers (used by both ScanResultInner and 
LookupResultInner)
+// ============================================================================
+
+mod row_reader {
+    use fcore::row::InternalRow;
+    use fluss as fcore;
+
+    use crate::types;
+
+    /// Get column at `field`, or error if out of bounds.
+    fn get_column(
+        columns: &[fcore::metadata::Column],
+        field: usize,
+    ) -> Result<&fcore::metadata::Column, String> {
+        columns.get(field).ok_or_else(|| {
+            format!(
+                "field index {field} out of range ({} columns)",
+                columns.len()
+            )
+        })
+    }
+
+    /// Validate bounds, null, and type compatibility in a single pass.
+    /// Returns the data type on success for callers that need to dispatch on 
it.
+    fn validate<'a>(
+        row: &dyn InternalRow,
+        columns: &'a [fcore::metadata::Column],
+        field: usize,
+        getter: &str,
+        allowed: impl FnOnce(&fcore::metadata::DataType) -> bool,
+    ) -> Result<&'a fcore::metadata::DataType, String> {
+        let col = get_column(columns, field)?;
+        if row.is_null_at(field) {

Review Comment:
   `get_column(columns, field)?` already bounds-checks against the schema. A 
separate `row.get_field_count()` check is redundant — both values derive from 
the same schema source. 
   
   Java/Rust doesn't do it either. I thinks it's because Arrow IPC decoding has 
its checks, so we can rely on it.



-- 
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]

Reply via email to