leekeiabstraction commented on code in PR #330:
URL: https://github.com/apache/fluss-rust/pull/330#discussion_r2809922061
##########
bindings/cpp/src/lib.rs:
##########
@@ -1632,128 +1632,247 @@ mod row_reader {
use crate::types;
- pub fn column_type(columns: &[fcore::metadata::Column], field: usize) ->
i32 {
- columns
- .get(field)
- .map_or(0, |c| types::core_data_type_to_ffi(c.data_type()))
+ /// 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()
+ )
+ })
}
- pub fn column_name(columns: &[fcore::metadata::Column], field: usize) ->
&str {
- columns.get(field).map_or("", |c| c.name())
+ /// 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) {
+ return Err(format!("field {field} is null"));
+ }
+ let dt = col.data_type();
Review Comment:
Should this only become an error if data_type is not nullable?
##########
bindings/cpp/src/table.cpp:
##########
@@ -79,6 +79,369 @@ int Date::Day() const {
return tm.tm_mday;
}
+// ============================================================================
+// GenericRow — write-only row backed by opaque Rust GenericRowInner
+// ============================================================================
+
+GenericRow::GenericRow() {
+ auto box = ffi::new_generic_row(0);
+ inner_ = box.into_raw();
+}
+
+GenericRow::GenericRow(size_t field_count) {
+ auto box = ffi::new_generic_row(field_count);
+ inner_ = box.into_raw();
+}
+
+GenericRow::~GenericRow() noexcept { Destroy(); }
+
+void GenericRow::Destroy() noexcept {
+ if (inner_) {
+ rust::Box<ffi::GenericRowInner>::from_raw(inner_);
+ inner_ = nullptr;
+ }
+}
+
+GenericRow::GenericRow(GenericRow&& other) noexcept
+ : inner_(other.inner_), column_map_(std::move(other.column_map_)) {
+ other.inner_ = nullptr;
+}
+
+GenericRow& GenericRow::operator=(GenericRow&& other) noexcept {
+ if (this != &other) {
+ Destroy();
+ inner_ = other.inner_;
+ column_map_ = std::move(other.column_map_);
+ other.inner_ = nullptr;
+ }
+ return *this;
+}
+
+bool GenericRow::Available() const { return inner_ != nullptr; }
+
+void GenericRow::Reset() {
+ if (inner_) inner_->gr_reset();
+}
+
+void GenericRow::SetNull(size_t idx) {
+ if (inner_) inner_->gr_set_null(idx);
Review Comment:
QQ: If I'm reading this right, this silently lets set write through when
inner_ is null? Should we error here instead, failing fast for users?
--
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]
