Chore: support all basic types by Varad-VK · Pull Request #144 · apache/fluss-rust

Varad-VK · 2026-01-11T11:00:24Z

Fixes #130

Summary:

support for basic datatypes
Added tests for lookup behavior

Testing:

cargo test

Copilot

Pull request overview

This PR adds support for Date, Timestamp, and TimestampLtz basic data types across the row handling system. The implementation enables these types to be used in field getters, binary writers, and key encoders.

Changes:

Added InternalRow methods for Date, Timestamp (NTZ), and Timestamp (LTZ) types
Extended field getters, binary writers, and encoders to handle the new types
Updated test coverage to verify encoding behavior for Date and Timestamp types

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/fluss/src/row/mod.rs	Added get_date, get_timestamp_ntz, and get_timestamp_ltz methods to InternalRow trait and GenericRow implementation
crates/fluss/src/row/field_getter.rs	Added Date, Timestamp, and TimestampLtz variants to InnerFieldGetter enum with corresponding get_field implementations
crates/fluss/src/row/encode/compacted_key_encoder.rs	Updated test_all_data_types test to include Date and Timestamp encoding verification
crates/fluss/src/row/datum.rs	Added new() and get_inner() methods for Timestamp and TimestampLtz types; updated test to be explicit about Cow type
crates/fluss/src/row/compacted/compacted_row_writer.rs	Added write_decimal, write_timestamp_ntz, and write_timestamp_ltz methods
crates/fluss/src/row/compacted/compacted_key_writer.rs	Delegated new write methods from BinaryWriter trait to CompactedRowWriter
crates/fluss/src/row/column.rs	Implemented get_date, get_timestamp_ntz, and get_timestamp_ltz for ColumnarRow
crates/fluss/src/row/binary/binary_writer.rs	Updated BinaryWriter trait and InnerValueWriter to support Decimal, Date, TimestampNtz, and TimestampLtz types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

leekeiabstraction

Left comments, PTAL!

leekeiabstraction · 2026-01-11T18:57:56Z


-    // TODO Decimal type
-    // fn write_decimal(&mut self, pos: i32, value: f64);
+    fn write_decimal(&mut self, value: &rust_decimal::Decimal, precision: u32);


nit: decimal as name should suffice, none other args within this trait starts with rust_

leekeiabstraction · 2026-01-11T19:03:27Z

+        // 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());


We should really have an implementation is consistent with Java's for this task.

leekeiabstraction · 2026-01-11T19:04:51Z

+        // Currently write timestamp as a long (epoch millis or other unit depending on upstream)
+        self.write_long(value);


We should really have an implementation is consistent with Java's for this task.

leekeiabstraction · 2026-01-11T19:06:29Z

+        // Currently write timestamp as a long (epoch millis or other unit depending on upstream)
+        self.write_long(value);


We should really have an implementation is consistent with Java's for this task.

leekeiabstraction · 2026-01-11T19:09:40Z

            Datum::from(15.21f64),
-            // TODO Date
-            // TODO Time
+            Datum::Date(crate::row::datum::Date::new(5)),


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

leekeiabstraction · 2026-01-11T19:10:24Z

-            // TODO Date
-            // TODO Time
+            Datum::Date(crate::row::datum::Date::new(5)),
+            Datum::Timestamp(crate::row::datum::Timestamp::new(13)),


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

luoyuxia

@Varad-VK Thanks for the pr. I left minor comment. PTAL

luoyuxia · 2026-01-13T13:33:43Z

@@ -52,14 +52,11 @@ pub trait BinaryWriter {

    fn write_binary(&mut self, bytes: &[u8], length: usize);



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

luoyuxia · 2026-01-13T13:34:31Z

+    Date,
+    TimestampNtz(u32), // precision
+    TimestampLtz(u32), // precision
+                       // TODO TimeWithoutTimeZone, Array, Row


Can we also support Time in this pr?

luoyuxia · 2026-01-13T13:36:16Z

    Float,
    Double,
-    // TODO Decimal, Date, TimeWithoutTimeZone, TimestampWithoutTimeZone, TimestampWithLocalTimeZone, Array, Row
+    Decimal(u32), // precision


Suggested change

Decimal(u32), // precision

Decimal(u32, u32), // precision, scale

luoyuxia · 2026-01-13T13:37:53Z

+        // 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());


luoyuxia · 2026-01-13T13:38:17Z

+        // Currently write timestamp as a long (epoch millis or other unit depending on upstream)
+        self.write_long(value);


luoyuxia · 2026-01-13T13:38:21Z

+        // Currently write timestamp as a long (epoch millis or other unit depending on upstream)
+        self.write_long(value);


luoyuxia · 2026-01-13T13:42:13Z

+        self.get_int(pos)
+    }
+
+    fn get_timestamp_ntz(&self, pos: usize) -> i64 {


we can return Timestamp directly

luoyuxia · 2026-01-13T13:42:33Z

+        self.get_long(pos)
+    }
+
+    fn get_timestamp_ltz(&self, pos: usize) -> i64 {


Suggested change

fn get_timestamp_ltz(&self, pos: usize) -> i64 {

fn get_timestamp_ltz(&self, pos: usize) -> TimestampLtz {

luoyuxia · 2026-01-13T13:43:59Z

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


could you please also support this method?

luoyuxia · 2026-01-16T07:50:41Z

@Varad-VK Hi, Do you still have time to working on this pr? If not, we're going to take over on it.

Varad-VK · 2026-01-16T08:19:57Z

Hi @luoyuxia, please take it ahead .

fresh-borzoni · 2026-01-17T16:15:19Z

I'll help here

luoyuxia · 2026-01-19T10:41:38Z

Will continue in #175

Chore: support all basic types

06b5ac7

Varad-VK mentioned this pull request Jan 11, 2026

Support all basic datatypes in CompactedKeyEncoder #130

Closed

2 tasks

luoyuxia requested a review from Copilot January 11, 2026 11:08

Copilot started reviewing on behalf of luoyuxia January 11, 2026 11:09 View session

Copilot AI reviewed Jan 11, 2026

View reviewed changes

Comment thread crates/fluss/src/row/mod.rs Outdated

Update crates/fluss/src/row/mod.rs

6fdc861

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

leekeiabstraction reviewed Jan 11, 2026

View reviewed changes

luoyuxia reviewed Jan 13, 2026

View reviewed changes

fresh-borzoni mentioned this pull request Jan 18, 2026

[Issue 130] support all basic datatypes in compacted key encoder(continuation) #175

Merged

luoyuxia closed this Jan 19, 2026

		// Currently write timestamp as a long (epoch millis or other unit depending on upstream)
		self.write_long(value);

		@@ -52,14 +52,11 @@ pub trait BinaryWriter {

		fn write_binary(&mut self, bytes: &[u8], length: usize);

	Decimal(u32), // precision
	Decimal(u32, u32), // precision, scale

	fn get_timestamp_ltz(&self, pos: usize) -> i64 {
	fn get_timestamp_ltz(&self, pos: usize) -> TimestampLtz {

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

Conversation

Varad-VK commented Jan 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

leekeiabstraction left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luoyuxia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luoyuxia commented Jan 16, 2026

Uh oh!

Varad-VK commented Jan 16, 2026

Uh oh!

fresh-borzoni commented Jan 17, 2026

Uh oh!

luoyuxia commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

leekeiabstraction Jan 11, 2026 •

edited

Loading

leekeiabstraction Jan 11, 2026 •

edited

Loading

leekeiabstraction Jan 11, 2026 •

edited

Loading