luoyuxia commented on code in PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#discussion_r2686455869


##########
crates/fluss/src/row/binary/binary_writer.rs:
##########
@@ -52,14 +52,11 @@ pub trait BinaryWriter {
 
     fn write_binary(&mut self, bytes: &[u8], length: usize);
 

Review Comment:
   sorry for miss that, can we also support write_time in this pr?



##########
crates/fluss/src/row/column.rs:
##########
@@ -126,6 +126,18 @@ impl InternalRow for ColumnarRow {
             .value(self.row_id)
     }
 
+    fn get_date(&self, pos: usize) -> i32 {
+        self.get_int(pos)
+    }
+
+    fn get_timestamp_ntz(&self, pos: usize) -> i64 {
+        self.get_long(pos)
+    }
+
+    fn get_timestamp_ltz(&self, pos: usize) -> i64 {

Review Comment:
   ```suggestion
       fn get_timestamp_ltz(&self, pos: usize) -> TimestampLtz {
   ```



##########
crates/fluss/src/row/binary/binary_writer.rs:
##########
@@ -125,7 +122,11 @@ pub enum InnerValueWriter {
     BigInt,
     Float,
     Double,
-    // TODO Decimal, Date, TimeWithoutTimeZone, TimestampWithoutTimeZone, 
TimestampWithLocalTimeZone, Array, Row
+    Decimal(u32), // precision

Review Comment:
   ```suggestion
       Decimal(u32, u32), // precision, scale
   ```



##########
crates/fluss/src/row/binary/binary_writer.rs:
##########
@@ -125,7 +122,11 @@ pub enum InnerValueWriter {
     BigInt,
     Float,
     Double,
-    // TODO Decimal, Date, TimeWithoutTimeZone, TimestampWithoutTimeZone, 
TimestampWithLocalTimeZone, Array, Row
+    Decimal(u32), // precision
+    Date,
+    TimestampNtz(u32), // precision
+    TimestampLtz(u32), // precision
+                       // TODO TimeWithoutTimeZone, Array, Row

Review Comment:
   Can we also support `Time` in this pr?
   



##########
crates/fluss/src/row/column.rs:
##########
@@ -126,6 +126,18 @@ impl InternalRow for ColumnarRow {
             .value(self.row_id)
     }
 
+    fn get_date(&self, pos: usize) -> i32 {
+        self.get_int(pos)
+    }
+
+    fn get_timestamp_ntz(&self, pos: usize) -> i64 {

Review Comment:
   we can return `Timestamp` directly



##########
crates/fluss/src/row/compacted/compacted_row_writer.rs:
##########
@@ -153,4 +153,21 @@ impl CompactedRowWriter {
     pub fn write_double(&mut self, value: f64) {
         self.write_raw(&value.to_ne_bytes());
     }
+
+    pub fn write_decimal(&mut self, value: &rust_decimal::Decimal, _precision: 
u32) {
+        // For now, serialize decimal to its string representation and write 
as bytes
+        // TODO: implement compact decimal encoding based on precision similar 
to Java implementation
+        let s = value.to_string();
+        self.write_bytes(s.as_bytes());

Review Comment:
   +1



##########
crates/fluss/src/row/compacted/compacted_row_writer.rs:
##########
@@ -153,4 +153,21 @@ impl CompactedRowWriter {
     pub fn write_double(&mut self, value: f64) {
         self.write_raw(&value.to_ne_bytes());
     }
+
+    pub fn write_decimal(&mut self, value: &rust_decimal::Decimal, _precision: 
u32) {
+        // For now, serialize decimal to its string representation and write 
as bytes
+        // TODO: implement compact decimal encoding based on precision similar 
to Java implementation
+        let s = value.to_string();
+        self.write_bytes(s.as_bytes());
+    }
+
+    pub fn write_timestamp_ntz(&mut self, value: i64, _precision: u32) {
+        // Currently write timestamp as a long (epoch millis or other unit 
depending on upstream)
+        self.write_long(value);
+    }
+
+    pub fn write_timestamp_ltz(&mut self, value: i64, _precision: u32) {
+        // Currently write timestamp as a long (epoch millis or other unit 
depending on upstream)
+        self.write_long(value);

Review Comment:
   +1



##########
crates/fluss/src/row/mod.rs:
##########
@@ -65,11 +65,14 @@ pub trait InternalRow {
     // /// Returns the decimal value at the given position
     // fn get_decimal(&self, pos: usize, precision: usize, scale: usize) -> 
Decimal;

Review Comment:
   could you please also support this method?



##########
crates/fluss/src/row/compacted/compacted_row_writer.rs:
##########
@@ -153,4 +153,21 @@ impl CompactedRowWriter {
     pub fn write_double(&mut self, value: f64) {
         self.write_raw(&value.to_ne_bytes());
     }
+
+    pub fn write_decimal(&mut self, value: &rust_decimal::Decimal, _precision: 
u32) {
+        // For now, serialize decimal to its string representation and write 
as bytes
+        // TODO: implement compact decimal encoding based on precision similar 
to Java implementation
+        let s = value.to_string();
+        self.write_bytes(s.as_bytes());
+    }
+
+    pub fn write_timestamp_ntz(&mut self, value: i64, _precision: u32) {
+        // Currently write timestamp as a long (epoch millis or other unit 
depending on upstream)
+        self.write_long(value);

Review Comment:
   +1



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