pingcap / tidb

TiDB is an open-source, cloud-native, distributed, MySQL-Compatible database for elastic scale and real-time analytics. Try AI-powered Chat2Query free at : https://www.pingcap.com/tidb-serverless/
https://pingcap.com
Apache License 2.0
37.02k stars 5.82k forks source link

UTC_TIMESTAMP() rounds fraction part instead of truncating it #56430

Open mjonss opened 1 week ago

mjonss commented 1 week ago

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

tidb> select utc_timestamp(6), utc_timestamp(), now(), now(6);
+----------------------------+---------------------+---------------------+----------------------------+
| utc_timestamp(6)           | utc_timestamp()     | now()               | now(6)                     |
+----------------------------+---------------------+---------------------+----------------------------+
| 2024-10-01 08:31:57.868668 | 2024-10-01 08:31:58 | 2024-10-01 10:31:57 | 2024-10-01 10:31:57.868667 |
+----------------------------+---------------------+---------------------+----------------------------+
1 row in set (0.00 sec)

2. What did you expect to see? (Required)

UTC_TIMESTAMP() should never round up, it is very confusing if time is rounding up, since it will be earlier than what the clock on the wall shows.

3. What did you see instead (Required)

Truncating (or always rounding down).

4. What is your TiDB version? (Required)

tidb_version(): Release Version: v8.4.0-alpha-325-g74034d4ac2
Edition: Community
Git Commit Hash: 74034d4ac243b3c14dbf5f8a9edb92e740da4212
Git Branch: now-diff-utc_timestamp
UTC Build Time: 2024-10-01 08:23:56
GoVersion: go1.22.1
Race Enabled: false
Check Table Before Drop: false
Store: unistore
mjonss commented 1 week ago

Related to #9710 but for UTC_TIMESTAMP(). I will make it truncate the fsp instead of rounding it.

dulao5 commented 6 days ago

Thank you very much.

I noticed that types.ModeHalfUp is also used in other code. There may be potential issues there as well. Could you please check those as well?

e.g. https://github.com/pingcap/tidb/blob/master/pkg/expression/builtin_time.go#L1736

mjonss commented 5 days ago

Thank you very much.

I noticed that types.ModeHalfUp is also used in other code. There may be potential issues there as well. Could you please check those as well?

e.g. https://github.com/pingcap/tidb/blob/master/pkg/expression/builtin_time.go#L1736

So there are several things that are tricky. SQL Standard and MySQL are using rounding for time in general (INSERT/UPDATE) but this specific PR if for now() type of functions, that generates a new current time, where it should not round, but truncate instead.

The use you link to is for FROM_UNIXTIME() which takes a unix_timestamp like integer/float/decimal as argument, and does rounding in MySQL.

I am working on a proof-of-concept for introducing TIME_TRUNCATE_FRACTIONAL sql_mode that would allow the user to configure using truncate instead of rounding for time fractional parts, see MySQL manual here.

@dulao5 what do you think about introducing MySQL's sql_mode TIME_TRUNCATE_FRACTIONAL?

mjonss commented 5 days ago

Similar with curtime(), but only for the fsp, not if fsp == 0:

tidb> select now(), now(3), now(6), curtime(), curtime(6), curtime(3);
+---------------------+-------------------------+----------------------------+-----------+-----------------+--------------+
| now()               | now(3)                  | now(6)                     | curtime() | curtime(6)      | curtime(3)   |
+---------------------+-------------------------+----------------------------+-----------+-----------------+--------------+
| 2024-10-03 09:16:33 | 2024-10-03 09:16:33.743 | 2024-10-03 09:16:33.743866 | 09:16:33  | 09:16:33.743866 | 09:16:33.744 |
+---------------------+-------------------------+----------------------------+-----------+-----------------+--------------+
1 row in set (0.00 sec)