fix: format throw data truncated error (#5272) by ti-chi-bot · Pull Request #5339 · pingcap/tiflash

ti-chi-bot · 2022-07-11T07:29:41Z

This is an automated cherry-pick of #5272

What problem does this PR solve?

Issue Number: close #4891

Problem Summary:

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Manual Test:

mysql> select * from test;
+------+--------------------------------------------------------------------+
| id   | value                                                              |
+------+--------------------------------------------------------------------+
|    1 | 9999999999999999999999999999999999999999999999999999999999999.9999 |
+------+--------------------------------------------------------------------+

mysql> select format(value, -3) from test;
+------------------------------------------------------------------------------------+
| format(value, -3)                                                                  |
+------------------------------------------------------------------------------------+
| 10,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000 |
+------------------------------------------------------------------------------------+

mysql> select format(value, 0) from test;
+------------------------------------------------------------------------------------+
| format(value, 0)                                                                   |
+------------------------------------------------------------------------------------+
| 10,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000 |
+------------------------------------------------------------------------------------+

mysql> select format(value, 3) from test;
+----------------------------------------------------------------------------------------+
| format(value, 3)                                                                       |
+----------------------------------------------------------------------------------------+
| 10,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000,000.000 |
+----------------------------------------------------------------------------------------+

mysql> select format(value, 10) from test;
+----------------------------------------------------------------------------------------------+
| format(value, 10)                                                                            |
+----------------------------------------------------------------------------------------------+
| 9,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999,999.9999000000 |
+----------------------------------------------------------------------------------------------+

mysql> explain select format(value, 10) from test;
+---------------------------+---------+--------------+---------------+---------------------------------------+
| id                        | estRows | task         | access object | operator info                         |
+---------------------------+---------+--------------+---------------+---------------------------------------+
| TableReader_11            | 1.00    | root         |               | data:ExchangeSender_10                |
| └─ExchangeSender_10       | 1.00    | mpp[tiflash] |               | ExchangeType: PassThrough             |
|   └─Projection_4          | 1.00    | mpp[tiflash] |               | format(test.test.value, 10)->Column#4 |
|     └─TableFullScan_9     | 1.00    | mpp[tiflash] | table:test    | keep order:false, stats:pseudo        |
+---------------------------+---------+--------------+---------------+---------------------------------------+

mysql> select * from format_test;
+------+----------+
| id   | value    |
+------+----------+
|    1 | 999.9999 |
+------+----------+

mysql> select format(value, -2) from format_test;
+-------------------+
| format(value, -2) |
+-------------------+
| 1,000             |
+-------------------+

mysql> select format(value, 0) from format_test;
+------------------+
| format(value, 0) |
+------------------+
| 1,000            |
+------------------+

mysql> select format(value, 2) from format_test;
+------------------+
| format(value, 2) |
+------------------+
| 1,000.00         |
+------------------+

mysql> select format(value, 10) from format_test;
+-------------------+
| format(value, 10) |
+-------------------+
| 999.9999000000    |
+-------------------+

mysql> explain select format(value, 10) from format_test;
+---------------------------+---------+--------------+-------------------+----------------------------------------------+
| id                        | estRows | task         | access object     | operator info                                |
+---------------------------+---------+--------------+-------------------+----------------------------------------------+
| TableReader_11            | 1.00    | root         |                   | data:ExchangeSender_10                       |
| └─ExchangeSender_10       | 1.00    | mpp[tiflash] |                   | ExchangeType: PassThrough                    |
|   └─Projection_4          | 1.00    | mpp[tiflash] |                   | format(test.format_test.value, 10)->Column#4 |
|     └─TableFullScan_9     | 1.00    | mpp[tiflash] | table:format_test | keep order:false, stats:pseudo               |
+---------------------------+---------+--------------+-------------------+----------------------------------------------+

Side effects

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

Documentation

Release note

Fix the issue that format throw data truncated error

ti-chi-bot · 2022-07-11T07:29:43Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

windtalker
ywqzzy

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

xzhangxian1008 · 2022-07-11T09:53:46Z

/run-all-tests

xzhangxian1008 · 2022-07-12T04:12:46Z

/run-all-tests

xzhangxian1008 · 2022-07-12T04:45:15Z

/run-all-tests

sre-bot · 2022-07-12T04:59:38Z

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FunctionsString.cpp                   1215               369    69.63%         230                71    69.13%        2718               738    72.85%         744               283    61.96%
tests/gtest_strings_format.cpp         463               106    77.11%           5                 0   100.00%         237                 0   100.00%         142                72    49.30%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                 1678               475    71.69%         235                71    69.79%        2955               738    75.03%         886               355    59.93%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18239      9788             46.33%    204375  98049        52.02%

full coverage report (for internal network access only)

xzhangxian1008 · 2022-07-12T15:00:41Z

/run-all-tests

sre-bot · 2022-07-12T15:12:28Z

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FunctionsString.cpp                   1215               369    69.63%         230                71    69.13%        2718               738    72.85%         744               283    61.96%
tests/gtest_strings_format.cpp         463               106    77.11%           5                 0   100.00%         237                 0   100.00%         142                72    49.30%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                 1678               475    71.69%         235                71    69.79%        2955               738    75.03%         886               355    59.93%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18239      9788             46.33%    204375  98063        52.02%

full coverage report (for internal network access only)

xzhangxian1008 · 2022-07-12T23:23:42Z

/run-all-tests

sre-bot · 2022-07-12T23:38:13Z

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FunctionsString.cpp                   1215               369    69.63%         230                71    69.13%        2718               738    72.85%         744               283    61.96%
tests/gtest_strings_format.cpp         463               106    77.11%           5                 0   100.00%         237                 0   100.00%         142                72    49.30%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                 1678               475    71.69%         235                71    69.79%        2955               738    75.03%         886               355    59.93%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18239      9788             46.33%    204375  98017        52.04%

full coverage report (for internal network access only)

xzhangxian1008 · 2022-07-13T00:35:39Z

/cc @ywqzzy /cc @windtalker

ti-chi-bot · 2022-07-13T00:35:41Z

@xzhangxian1008: GitHub didn't allow me to request PR reviews from the following users: /cc.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @ywqzzy /cc @windtalker

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.

windtalker

LGTM

ywqzzy

LGTM

ywqzzy · 2022-07-13T05:22:48Z

/merge

ti-chi-bot · 2022-07-13T05:22:49Z

@ywqzzy: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

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 ti-community-infra/tichi repository.

ti-chi-bot · 2022-07-13T05:22:51Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 5ac7284

sre-bot · 2022-07-13T05:34:02Z

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FunctionsString.cpp                   1215               369    69.63%         230                71    69.13%        2718               738    72.85%         744               283    61.96%
tests/gtest_strings_format.cpp         463               106    77.11%           5                 0   100.00%         237                 0   100.00%         142                72    49.30%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                 1678               475    71.69%         235                71    69.79%        2955               738    75.03%         886               355    59.93%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18239      9788             46.33%    204375  98044        52.03%

full coverage report (for internal network access only)

xzhangxian1008 added 11 commits July 11, 2022 07:29

fix

7970744

update

3c773c5

update

caa92d7

format

50294b0

update

f702af4

format

13c70e1

enable ut

211c456

update

8e7b6f0

update

0787b12

udpate

47ee498

add fullstack test

212813f

ti-chi-bot mentioned this pull request Jul 11, 2022

fix: format throw data truncated error #5272

Merged

12 tasks

ti-chi-bot assigned xzhangxian1008 Jul 11, 2022

resolve

5e4396b

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 11, 2022

tweaking

658306d

trigger

5ac7284

ti-chi-bot requested a review from ywqzzy July 13, 2022 00:35

ti-chi-bot requested a review from windtalker July 13, 2022 00:35

windtalker approved these changes Jul 13, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 13, 2022

ywqzzy approved these changes Jul 13, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 13, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 13, 2022

VelocityLight removed the do-not-merge/cherry-pick-not-approved label Jul 13, 2022

ti-chi-bot added the do-not-merge/cherry-pick-not-approved label Jul 13, 2022

VelocityLight added cherry-pick-approved Cherry pick PR approved by release team. and removed do-not-merge/cherry-pick-not-approved labels Jul 13, 2022

ti-chi-bot removed the do-not-merge/cherry-pick-not-approved label Jul 13, 2022

ti-chi-bot merged commit 1ebc953 into pingcap:release-6.1 Jul 13, 2022

xzhangxian1008 deleted the cherry-pick-5272-to-release-6.1 branch July 13, 2022 06:03

jayl1e added this to the v6.1.1 milestone Aug 23, 2022

Conversation

ti-chi-bot commented Jul 11, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Jul 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xzhangxian1008 commented Jul 11, 2022

Uh oh!

xzhangxian1008 commented Jul 12, 2022

Uh oh!

xzhangxian1008 commented Jul 12, 2022

Uh oh!

sre-bot commented Jul 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

xzhangxian1008 commented Jul 12, 2022

Uh oh!

sre-bot commented Jul 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

xzhangxian1008 commented Jul 12, 2022

Uh oh!

sre-bot commented Jul 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

xzhangxian1008 commented Jul 13, 2022

Uh oh!

ti-chi-bot commented Jul 13, 2022

Uh oh!

windtalker left a comment

Choose a reason for hiding this comment

Uh oh!

ywqzzy left a comment

Choose a reason for hiding this comment

Uh oh!

ywqzzy commented Jul 13, 2022

Uh oh!

ti-chi-bot commented Jul 13, 2022

Uh oh!

ti-chi-bot commented Jul 13, 2022

Uh oh!

sre-bot commented Jul 13, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ti-chi-bot commented Jul 11, 2022 •

edited

Loading