feat(types): support ns timestamp #19827

xxhZs · 2024-12-17T08:37:42Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

our timestamp only support ms, But our internal implementation is capable of ns, it's just that we automatically convert it to ms when we read it in, and this pr can read ns to support ns.

Checklist

I have written necessary rustdoc comments.
I have added necessary unit tests and integration tests.
I have added test labels as necessary.
I have added fuzzing tests or opened an issue to track them.
My PR contains breaking changes.
My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

My PR needs documentation updates.

Release note

src/common/src/types/datetime.rs

wcy-fdu · 2024-12-18T09:26:33Z

I tested whether I could read nanosecond in the parquet file on this branch, and it seemed to work.

wcy-fdu · 2024-12-19T04:40:52Z

sorry, mis clicked update branch

wcy-fdu · 2024-12-19T04:56:52Z

cc @xiangjinwu PTAL❤️

xiangjinwu

Could you also include the testing SQLs you have tried?
Is any caller of to_protobuf/from_protobuf assuming that Timestamp would be fixed 64-bit? This assumption is no longer true.

xiangjinwu · 2024-12-19T06:06:19Z

src/common/src/types/datetime.rs

-            )
-        }
+        let dt = s
+            .parse::<jiff::civil::DateTime>()


Why do we need to introduce yet another dependency in addition to chrono and speedate?

Because chrono can't be parsed, and speedate is only parsed at the us level

src/common/src/types/datetime.rs

xiangjinwu · 2024-12-19T06:22:50Z

src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs

@@ -495,7 +495,7 @@ impl HummockVersion {
                            .clone_from(&group_construct.table_ids);
                        self.levels.insert(*compaction_group_id, new_levels);
                        let member_table_ids = if group_construct.version
-                            >= CompatibilityVersion::NoMemberTableIds as _
+                            >= CompatibilityVersion::NoMemberTableIds as i32


avoid unrelated change

It seems we do need this as long as we use jiff in src/common/src/types/datetime.rs. Not sure why though ... maybe a rustc bug.

Btw it can be group_construct.version() >= CompatibilityVersion::NoMemberTableIds rather than if group_construct.version >= CompatibilityVersion::NoMemberTableIds as i32

It doesn't seem to work because the type of CompatibilityVersion::NoMemberTableIds is not i32

group_construct.version() returns CompatibilityVersion, while group_construct.version is i32

xxhZs · 2024-12-23T07:05:15Z

Could you also include the testing SQLs you have tried?

added in ci

src/common/src/types/datetime.rs

xiangjinwu · 2024-12-24T10:15:27Z

src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs

@@ -495,7 +495,7 @@ impl HummockVersion {
                            .clone_from(&group_construct.table_ids);
                        self.levels.insert(*compaction_group_id, new_levels);
                        let member_table_ids = if group_construct.version
-                            >= CompatibilityVersion::NoMemberTableIds as _
+                            >= CompatibilityVersion::NoMemberTableIds as i32


It seems we do need this as long as we use jiff in src/common/src/types/datetime.rs. Not sure why though ... maybe a rustc bug.

Btw it can be group_construct.version() >= CompatibilityVersion::NoMemberTableIds rather than if group_construct.version >= CompatibilityVersion::NoMemberTableIds as i32

e2e_test/batch/types/timestamp_ns.slt.part

xiangjinwu · 2024-12-26T06:16:58Z

Btw you can fix the e2e test with rowsort.

https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki

The default is "nosort". In nosort mode, the results appear in exactly the order in which they were received from the database engine. The nosort mode should only be used on queries that have an ORDER BY clause or which only have a single row of result, since otherwise the order of results is undefined and might vary from one database engine to another. The "rowsort" mode gathers all output from the database engine then sorts it by rows on the client side.

#6776

If there's rowsort, slt file will contain sorted results.

xiangjinwu · 2024-12-26T09:21:24Z

FYI regarding your latest e2e failure, it is actually a bug in our existing to_char implementation #19944:

1 BC in PostgreSQL is year 0 in chrono. Although chrono also provides DateLike::year_ce method, its strftime module does not provide a specifier to use year_ce instead of year.

Given it is out of scope of this PR, let's just replace the case with one without BC.

fxi ci fix ci fix ci

support

efcb97b

xxhZs requested a review from a team as a code owner December 17, 2024 08:37

xxhZs requested a review from xxchan December 17, 2024 08:37

github-actions bot added the type/feature label Dec 17, 2024

graphite-app bot requested a review from a team December 17, 2024 09:25

hzxa21 requested a review from wcy-fdu December 18, 2024 03:40

fuyufjh reviewed Dec 18, 2024

View reviewed changes

src/common/src/types/datetime.rs Show resolved Hide resolved

fix comm

4aad830

Merge branch 'main' into xxh/support-timestamp-ns-1

9bf25a9

wcy-fdu enabled auto-merge December 19, 2024 04:39

wcy-fdu disabled auto-merge December 19, 2024 04:40

wcy-fdu added the need-cherry-pick-release-2.1 label Dec 19, 2024

wcy-fdu requested a review from xiangjinwu December 19, 2024 04:56

xiangjinwu reviewed Dec 19, 2024

View reviewed changes

xxhZs added 2 commits December 23, 2024 14:25

add ci

a6acfc2

fix comm

026e08a

fix fmt

f1cb8ab

xxhZs requested a review from xiangjinwu December 23, 2024 07:35

xiangjinwu reviewed Dec 24, 2024

View reviewed changes

add doc + fix ci

d09d463

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 26d9664 to 6b8af32 Compare December 26, 2024 07:49

fix ci

2c87c76

fxi ci fix ci fix ci

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 6b8af32 to 2c87c76 Compare December 26, 2024 09:54

fix ci

d975bf0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(types): support ns timestamp #19827

feat(types): support ns timestamp #19827

xxhZs commented Dec 17, 2024 •

edited

Loading

wcy-fdu commented Dec 18, 2024

wcy-fdu commented Dec 19, 2024

wcy-fdu commented Dec 19, 2024

xiangjinwu left a comment

xiangjinwu Dec 19, 2024

xxhZs Dec 23, 2024

xiangjinwu Dec 19, 2024

xiangjinwu Dec 24, 2024

xxhZs Dec 25, 2024

xiangjinwu Dec 25, 2024

xxhZs commented Dec 23, 2024 •

edited

Loading

xiangjinwu Dec 24, 2024

xiangjinwu commented Dec 26, 2024

xiangjinwu commented Dec 26, 2024 •

edited

Loading

feat(types): support ns timestamp #19827

Are you sure you want to change the base?

feat(types): support ns timestamp #19827

Conversation

xxhZs commented Dec 17, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

wcy-fdu commented Dec 18, 2024

wcy-fdu commented Dec 19, 2024

wcy-fdu commented Dec 19, 2024

xiangjinwu left a comment

Choose a reason for hiding this comment

xiangjinwu Dec 19, 2024

Choose a reason for hiding this comment

xxhZs Dec 23, 2024

Choose a reason for hiding this comment

xiangjinwu Dec 19, 2024

Choose a reason for hiding this comment

xiangjinwu Dec 24, 2024

Choose a reason for hiding this comment

xxhZs Dec 25, 2024

Choose a reason for hiding this comment

xiangjinwu Dec 25, 2024

Choose a reason for hiding this comment

xxhZs commented Dec 23, 2024 • edited Loading

xiangjinwu Dec 24, 2024

Choose a reason for hiding this comment

xiangjinwu commented Dec 26, 2024

xiangjinwu commented Dec 26, 2024 • edited Loading

xxhZs commented Dec 17, 2024 •

edited

Loading

xxhZs commented Dec 23, 2024 •

edited

Loading

xiangjinwu commented Dec 26, 2024 •

edited

Loading