fresh-borzoni commented on code in PR #161:
URL: https://github.com/apache/fluss-rust/pull/161#discussion_r2692668945
##########
crates/fluss/src/row/encode/mod.rs:
##########
@@ -62,3 +64,65 @@ impl dyn KeyEncoder {
}
}
}
+
+/// An encoder to write [`BinaryRow`]. It's used to write row
+/// multi-times one by one. When writing a new row:
+///
+/// 1. call method [`RowEncoder::start_new_row()`] to start the writing.
+/// 2. call method [`RowEncoder::encode_field()`] to write the row's field.
+/// 3. call method [`RowEncoder::finishRow()`] to finish the writing and get
the written row.
+#[allow(dead_code)]
+pub trait RowEncoder {
+ /// Start to write a new row.
+ ///
+ /// # Returns
+ /// * Ok(()) if successful
+ fn start_new_row(&mut self) -> Result<()>;
+
+ /// Write the row's field in given pos with given value.
+ ///
+ /// # Arguments
+ /// * pos - the position of the field to write.
+ /// * value - the value of the field to write.
+ ///
+ /// # Returns
+ /// * Ok(()) if successful
+ fn encode_field(&mut self, pos: usize, value: Datum) -> Result<()>;
+
+ /// Finish write the row, returns the written row.
+ ///
+ /// Note that returned row borrows from [`RowEncoder`]'s internal buffer
which is reused for subsequent rows
+ /// [`RowEncoder::start_new_row()`] should only be called after the
returned row goes out of scope.
+ ///
+ /// # Returns
+ /// * the written row
+ fn finish_row(&mut self) -> Result<impl BinaryRow>;
+
+ /// Closes the row encoder
+ ///
+ /// # Returns
+ /// * Ok(()) if successful
+ fn close(&mut self) -> Result<()>;
+}
+
+#[allow(dead_code)]
+pub struct RowEncoderFactory {}
+
+#[allow(dead_code)]
+impl RowEncoderFactory {
+ pub fn create(kv_format: KvFormat, row_type: RowType) -> Result<impl
RowEncoder> {
Review Comment:
Factory currently takes RowType by value and get_children() clones all
DataTypes. Could we instead take &RowType and make the cloning explicit in the
factory?
RowType is schema metadata and is typically shared/read-only, taking
ownership forces callers to move or clone it, even when no needed, brittle API.
e.g.
```rust
impl RowType {
pub fn field_types(&self) -> impl Iterator<Item = &DataType> + '_ {
self.fields.iter().map(|f| &f.data_type)
}
}
impl RowEncoderFactory {
pub fn create(kv_format: KvFormat, row_type: &RowType) -> Result<impl
RowEncoder> {
let field_data_types: Vec<DataType> =
row_type.field_types().cloned().collect();
Self::create_for_field_types(kv_format, field_data_types)
}
}
```
--
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]
