*: Add TIME_TRUNCATE_FRACTIONAL sql_mode

[mjonss]

What problem does this PR solve?

Issue Number: close #29406

Problem Summary:

What changed and how does it work?

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Adds support for TIME_TRUNCATE_FRACTIONAL sql_mode, truncating DATETIME/TIMESTAMP/TIME instead of rouding it.

[tiprow]

Hi @mjonss. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[codecov]

Codecov Report

Attention: Patch coverage is 79.24528% with 11 lines in your changes missing coverage. Please review.

Project coverage is 76.0375%. Comparing base (5e8bb8e) to head (7988a88).
Report is 26 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #56442        +/-   ##
================================================
+ Coverage   73.3611%   76.0375%   +2.6763%     
================================================
  Files          1626       1648        +22     
  Lines        449213     457224      +8011     
================================================
+ Hits         329548     347662     +18114     
+ Misses        99482      87905     -11577     
- Partials      20183      21657      +1474     

Flag	Coverage Δ
integration	51.8390% <71.6981%> (?)
unit	72.4429% <79.2452%> (-0.0219%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	52.9478% <ø> (ø)
parser	∅ <ø> (∅)
br	63.0995% <ø> (+17.5356%)	⬆️

[dveeden]

The example from the MySQL docs:

sql> CREATE TABLE t (id INT, tval TIME(1));
Query OK, 0 rows affected (0.0096 sec)

sql> SET sql_mode='';
Query OK, 0 rows affected (0.0003 sec)

sql> INSERT INTO t (id, tval) VALUES(1, 1.55);
Query OK, 1 row affected (0.0010 sec)

sql> SET sql_mode='TIME_TRUNCATE_FRACTIONAL';
Query OK, 0 rows affected (0.0001 sec)

sql> INSERT INTO t (id, tval) VALUES(2, 1.55);
Query OK, 1 row affected (0.0008 sec)

sql> SELECT id, tval FROM t ORDER BY id;
+----+------------+
| id | tval       |
+----+------------+
|  1 | 00:00:01.6 |
|  2 | 00:00:01.5 |
+----+------------+
2 rows in set (0.0014 sec)

sql> SELECT TIDB_VERSION()\G
*************************** 1. row ***************************
TIDB_VERSION(): Release Version: v8.4.0-alpha-338-g640dc8844c
Edition: Community
Git Commit Hash: 640dc8844cbd0d3ff1fafa64ce213afcc7dd25a4
Git Branch: time_truncate_fractional-sqlmode
UTC Build Time: 2024-10-04 15:02:03
GoVersion: devel go1.24-065c1359 Fri Oct 4 14:49:31 2024 +0000
Race Enabled: false
Check Table Before Drop: false
Store: unistore
1 row in set (0.0007 sec)

[dveeden]

I think "Changes MySQL compatibility" should be checked and I also think this needs a release note.

[dveeden]

TiDB:

sql> SELECT UTC_TIMESTAMP(7);
ERROR: 1105 (HY000): Too-big precision 7 specified for 'curtime'. Maximum is 6

MySQL:

sql> SELECT UTC_TIMESTAMP(7);
ERROR: 1426 (42000): Too-big precision 7 specified for 'utc_timestamp'. Maximum is 6.

1. The error code is different
2. The sqlstate is different
3. The name of the function in the error message is different.

Maybe we should create a new issue for this?

[mjonss]

TiDB:

sql> SELECT UTC_TIMESTAMP(7);
ERROR: 1105 (HY000): Too-big precision 7 specified for 'curtime'. Maximum is 6

MySQL:

sql> SELECT UTC_TIMESTAMP(7);
ERROR: 1426 (42000): Too-big precision 7 specified for 'utc_timestamp'. Maximum is 6.

1. The error code is different
2. The sqlstate is different
3. The name of the function in the error message is different.

Maybe we should create a new issue for this?

I created #56451.

[mjonss]

/retest

[tiprow]

@mjonss: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[dveeden]

/ok-to-test

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2024-10-04 15:15:40.264090338 +0000 UTC m=+627095.684303347: ☑️ agreed by dveeden.
• 2024-10-10 03:50:58.016555098 +0000 UTC m=+1104413.436768142: ☑️ agreed by lance6716.

[lance6716]

/assign @tangenta @zanmato1984

[zanmato1984]

Some nits.

[mjonss]

@zanmato1984 @lance6716 Are there any reason that this SQL mode needs to be included in TiKV/TiFlash/UniStore? Can there be any coprocessor calls that would be affected? Or how can I look for it?

[ti-chi-bot]

@mjonss: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
idc-jenkins-ci-tidb/mysql-test	7988a88	link	true	/test mysql-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[zanmato1984]

@zanmato1984 @lance6716 Are there any reason that this SQL mode needs to be included in TiKV/TiFlash/UniStore? Can there be any coprocessor calls that would be affected? Or how can I look for it?

Seems this SQL mode affects function from_unixtime() (please correct me if I'm missing anything), which is currently pushed down to TiFlash

tidb/pkg/expression/infer_pushdown.go

Line 277 in f4e820d

ast.DateDiff, ast.TimestampDiff, ast.DateFormat, ast.FromUnixTime,

(but not TiKV

tidb/pkg/expression/infer_pushdown.go

Line 213 in f4e820d

ast.PeriodAdd, ast.PeriodDiff, /*ast.TimestampDiff, ast.DateAdd, ast.FromUnixTime,*/

). So I think we'll need it supported in TiFlash.

It'll be great if you can directly work on that. Here are some links in TiFlash that may help:

• from_unixtime() implementation: https://github.com/pingcap/tiflash/blob/9f85bb2f1692196f5aeb056a5097e232a5c95371/dbms/src/Functions/FunctionsConversion.h#L1515
• SQL mode definitions: https://github.com/pingcap/tiflash/blob/9f85bb2f1692196f5aeb056a5097e232a5c95371/dbms/src/Flash/Coprocessor/DAGContext.h#L93

If you have any difficulties, please let me know, and our team will love to help or take over.

Thanks.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dveeden, lance6716, zanmato1984
Once this PR has been reviewed and has the lgtm label, please ask for approval from tangenta, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS [dveeden,lance6716,zanmato1984]
• pkg/ddl/OWNERS [lance6716]
• pkg/expression/OWNERS [zanmato1984]
• pkg/parser/OWNERS
• pkg/table/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment