You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "Varad-VK (via GitHub)" <gi...@apache.org> on 2026/01/11 11:00:27 UTC
[PR] Chore: support all basic types [fluss-rust]
Varad-VK opened a new pull request, #144:
URL: https://github.com/apache/fluss-rust/pull/144
Fixes #130
Summary:
- support for basic datatypes
- Added tests for lookup behavior
Testing:
- cargo test
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#issuecomment-3758623120
@Varad-VK Hi, Do you still have time to working on this pr? If not, we're going to take over 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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#issuecomment-3767651655
Will continue in #175
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#discussion_r2680092938
##########
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:
We should really have an implementation is consistent with Java's for this task.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#discussion_r2680086852
##########
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:
We should really have an implementation is consistent with Java's for this task.
##########
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:
We should really have an implementation is consistent with Java's for this task.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "Varad-VK (via GitHub)" <gi...@apache.org>.
Varad-VK commented on PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#issuecomment-3758720629
Hi @luoyuxia, please take it ahead .
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "leekeiabstraction (via GitHub)" <gi...@apache.org>.
leekeiabstraction commented on code in PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#discussion_r2680079125
##########
crates/fluss/src/row/binary/binary_writer.rs:
##########
@@ -52,14 +52,11 @@ pub trait BinaryWriter {
fn write_binary(&mut self, bytes: &[u8], length: usize);
- // TODO Decimal type
- // fn write_decimal(&mut self, pos: i32, value: f64);
+ fn write_decimal(&mut self, value: &rust_decimal::Decimal, precision: u32);
Review Comment:
nit: `decimal` as name should suffice, none other args within this trait starts with `rust_`
##########
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:
It might be better to leave this as TODO instead of having an implementation that differs from Java.
##########
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:
It might be better to leave this as TODO instead of having an implementation that differs from Java.
##########
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:
It might be better to leave this as TODO instead of having an implementation that differs from Java.
##########
crates/fluss/src/row/encode/compacted_key_encoder.rs:
##########
@@ -270,14 +270,13 @@ mod tests {
Datum::from(-6101065172474983726i64), // from Java test case: new BigInteger("12345678901234567890").longValue()
Datum::from(13.2f32),
Datum::from(15.21f64),
- // TODO Date
- // TODO Time
+ Datum::Date(crate::row::datum::Date::new(5)),
Review Comment:
What does 5 mean?
I suggest using the date 2023-10-25 to keep test case parity with java side.
Ref here: https://github.com/apache/fluss/blob/2b0e2b1b08ede61874e71e21877134d8945fe8c8/fluss-common/src/test/java/org/apache/fluss/row/indexed/IndexedRowTest.java#L196
##########
crates/fluss/src/row/encode/compacted_key_encoder.rs:
##########
@@ -270,14 +270,13 @@ mod tests {
Datum::from(-6101065172474983726i64), // from Java test case: new BigInteger("12345678901234567890").longValue()
Datum::from(13.2f32),
Datum::from(15.21f64),
- // TODO Date
- // TODO Time
+ Datum::Date(crate::row::datum::Date::new(5)),
+ Datum::Timestamp(crate::row::datum::Timestamp::new(13)),
Review Comment:
I suggest using 09:30:00.0 to keep test case parity with java side.
Ref here: https://github.com/apache/fluss/blob/2b0e2b1b08ede61874e71e21877134d8945fe8c8/fluss-common/src/test/java/org/apache/fluss/row/indexed/IndexedRowTest.java#L198
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "fresh-borzoni (via GitHub)" <gi...@apache.org>.
fresh-borzoni commented on PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#issuecomment-3764068982
I'll help here
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia closed pull request #144: Chore: support all basic types
URL: https://github.com/apache/fluss-rust/pull/144
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Chore: support all basic types [fluss-rust]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #144:
URL: https://github.com/apache/fluss-rust/pull/144#discussion_r2679448299
##########
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;
- // /// Returns the timestamp value at the given position
- // fn get_timestamp_ntz(&self, pos: usize, precision: usize) -> TimestampNtz;
+ /// Returns the timestamp value at the given position (date as days since epoch)
Review Comment:
The comment incorrectly states "Returns the timestamp value at the given position" when this method actually returns a date value (days since epoch). The comment should read "Returns the date value at the given position (date as days since epoch)".
```suggestion
/// Returns the date value at the given position (date as days since epoch)
```
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org