fix hexInt `sprintf` format pattern #5460

YangKeao · 2022-07-25T04:34:32Z

Signed-off-by: YangKeao yangkeao@chunibyo.icu

What problem does this PR solve?

Issue Number: close #5462

#5280 (comment)

I should use the %llX to print a unsigned uint64 😢 .

What is changed and how it works?

Change the format literal from %lX to %llX

Release note

Fix hexInt `sprintf` format pattern

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

ti-chi-bot · 2022-07-25T04:34:33Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

breezewish
fuzhe1989

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.

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

fuzhe1989 · 2022-07-25T05:32:02Z

dbms/src/Functions/FunctionsString.cpp

@@ -5285,7 +5285,7 @@ class FunctionHexInt : public IFunction
        {
            UInt64 number = col->getUInt(i);

-            int print_size = sprintf(reinterpret_cast<char *>(&res_chars[prev_res_offset]), "%lX", number);
+            int print_size = sprintf(reinterpret_cast<char *>(&res_chars[prev_res_offset]), "%llX", number);


use fmt::format_to instead?

breezewish

Verified it works.

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

breezewish · 2022-07-26T05:30:43Z

/merge

ti-chi-bot · 2022-07-26T05:30:44Z

@breezewish: 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.

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-26T05:30:45Z

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

Commit hash: 4ac3f93

sre-bot · 2022-07-26T05:54:29Z

Coverage for changed files

Filename                                                                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
/home/jenkins/agent/workspace/tiflash-build-common/tiflash/dbms/src/Functions/FunctionsString.cpp        1405               365    74.02%         267                69    74.16%        3081               743    75.88%         864               290    66.44%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                    1405               365    74.02%         267                69    74.16%        3081               743    75.88%         864               290    66.44%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18826      9437             49.87%    214678  95416        55.55%

full coverage report (for internal network access only)

SeaRise · 2022-07-26T10:14:14Z

/merge

ti-chi-bot · 2022-07-26T10:14:15Z

@SeaRise: 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.

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-26T10:14:29Z

@YangKeao: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-07-26T10:35:56Z

Coverage for changed files

Filename                                                                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
/home/jenkins/agent/workspace/tiflash-build-common/tiflash/dbms/src/Functions/FunctionsString.cpp        1405               365    74.02%         267                69    74.16%        3081               743    75.88%         864               290    66.44%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                    1405               365    74.02%         267                69    74.16%        3081               743    75.88%         864               290    66.44%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18827      9433             49.90%    214682  95328        55.60%

full coverage report (for internal network access only)

print number with %llx

8be59bd

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

ti-chi-bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 25, 2022

fuzhe1989 reviewed Jul 25, 2022

View reviewed changes

breezewish approved these changes Jul 25, 2022

View reviewed changes

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

breezewish and others added 2 commits July 26, 2022 11:58

Merge branch 'master' into fix-5280-format

addad70

use fmt::format_to to format the codes

4ac3f93

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 26, 2022

fuzhe1989 approved these changes Jul 26, 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 26, 2022

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

ti-chi-bot removed the do-not-merge/needs-linked-issue label Jul 26, 2022

JaySon-Huang changed the title ~~fix hexInt sprintf format pattern by using llx~~ fix hexInt sprintf format pattern Jul 26, 2022

Merge branch 'master' into fix-5280-format

719ba37

ti-chi-bot merged commit c4ad3f1 into pingcap:master Jul 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix hexInt `sprintf` format pattern #5460

fix hexInt `sprintf` format pattern #5460

YangKeao commented Jul 25, 2022 •

edited by SeaRise

Loading

ti-chi-bot commented Jul 25, 2022 •

edited

Loading

fuzhe1989 Jul 25, 2022 •

edited

Loading

breezewish left a comment

breezewish commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

sre-bot commented Jul 26, 2022

SeaRise commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

sre-bot commented Jul 26, 2022

fix hexInt sprintf format pattern #5460

fix hexInt sprintf format pattern #5460

Conversation

YangKeao commented Jul 25, 2022 • edited by SeaRise Loading

What problem does this PR solve?

What is changed and how it works?

Release note

ti-chi-bot commented Jul 25, 2022 • edited Loading

fuzhe1989 Jul 25, 2022 • edited Loading

Choose a reason for hiding this comment

breezewish left a comment

Choose a reason for hiding this comment

breezewish commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

sre-bot commented Jul 26, 2022

Coverage for changed files

Coverage summary

SeaRise commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

ti-chi-bot commented Jul 26, 2022

sre-bot commented Jul 26, 2022

Coverage for changed files

Coverage summary

fix hexInt `sprintf` format pattern #5460

fix hexInt `sprintf` format pattern #5460

YangKeao commented Jul 25, 2022 •

edited by SeaRise

Loading

ti-chi-bot commented Jul 25, 2022 •

edited

Loading

fuzhe1989 Jul 25, 2022 •

edited

Loading