types: use flags in types package to handle clip zero case by lcwangchao · Pull Request #47543 · pingcap/tidb

lcwangchao · 2023-10-11T06:53:19Z

What problem does this PR solve?

Issue Number: close #47518

What is changed and how it works?

This is one of the PR for code refactor, see: #47355

We use FlagClipNegativeToZero to indicate whether to clip the negative value to zero when casting the negative number to be unsigned.

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

Release note

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

None

lcwangchao · 2023-10-11T06:54:21Z

/run-check-issue-triage-complete

sessionctx/stmtctx/stmtctx.go

codecov · 2023-10-11T07:07:54Z

Codecov Report

Merging #47543 (5590475) into master (90bd2dd) will increase coverage by 0.9374%.
Report is 1 commits behind head on master.
The diff coverage is 100.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47543        +/-   ##
================================================
+ Coverage   71.8537%   72.7911%   +0.9374%     
================================================
  Files          1398       1421        +23     
  Lines        405209     411591      +6382     
================================================
+ Hits         291158     299602      +8444     
+ Misses        94396      93143      -1253     
+ Partials      19655      18846       -809

Flag	Coverage Δ
integration	`41.4618% <90.9090%> (?)`
unit	`71.8552% <88.6363%> (+0.0014%)`	⬆️

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

Components	Coverage Δ
dumpling	`53.9913% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.7105% <100.0000%> (-4.2406%)`	⬇️

lcwangchao · 2023-10-16T05:28:04Z

/retest tiprow_fast_test

ti-chi-bot · 2023-10-16T05:28:07Z

@lcwangchao: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test build
/test canary-scan-security
/test check-dev
/test check-dev2
/test mysql-test
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-br-integration-test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-ddl-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-nodejs-test
/test pull-mysql-client-test
/test pull-sqllogic-test
/test pull-tiflash-test
/test unit-test

The following commands are available to trigger optional jobs:

/test pull-notify-when-compatibility-sections-changed

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test

Details

In response to this:

/retest tiprow_fast_test

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.

tiprow · 2023-10-16T05:28:26Z

@lcwangchao: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

Details

In response to this:

/retest tiprow_fast_test

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.

pkg/sessionctx/stmtctx/stmtctx.go

lcwangchao · 2023-10-16T12:36:54Z

/retest

YangKeao

Rest LGTM

YangKeao · 2023-10-18T06:05:03Z

pkg/ddl/backfilling_scheduler.go

-		tz := *time.UTC
-		sessCtx.GetSessionVars().StmtCtx.SetTimeZone(&tz)
-	}
-	sessCtx.GetSessionVars().StmtCtx.IsDDLJobInQueue = true


Is this change expected? Though it seems that it's only used in pkg/executor/ddl.go 🤔 .

I think IsDDLJobInQueue is used to make ShouldClipToZeror in the previous code. The verify CI passed after removing it.. @tangenta PTAL

IIUC, pkg/executor/ddl.go is not related with pkg/ddl/backfilling_scheduler.go 🤔 , so it seems fine for this case.

lcwangchao · 2023-10-18T06:30:45Z

/test check-dev2

tiprow · 2023-10-18T06:31:08Z

@lcwangchao: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

Details

In response to this:

/test check-dev2

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.

YangKeao

LGTM

ti-chi-bot · 2023-10-18T06:43:05Z

[LGTM Timeline notifier]

Timeline:

2023-10-17 04:10:15.518836038 +0000 UTC m=+1716613.105946183: ☑️ agreed by lance6716.
2023-10-18 06:43:04.301501956 +0000 UTC m=+1812181.888612101: ☑️ agreed by YangKeao.

ti-chi-bot · 2023-10-19T03:25:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lance6716, XuHuaiyu, YangKeao

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [XuHuaiyu,YangKeao,lance6716]
~~br/pkg/lightning/OWNERS~~ [lance6716]
~~pkg/expression/OWNERS~~ [XuHuaiyu]

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

…7543) close pingcap#47518

ti-chi-bot bot added do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 11, 2023

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Oct 11, 2023

types: use flags in types package to handle clip zero case

ca41ccd

lcwangchao force-pushed the typectx_clip_zero branch from 1a5c268 to ca41ccd Compare October 11, 2023 06:56

lcwangchao commented Oct 11, 2023

View reviewed changes

sessionctx/stmtctx/stmtctx.go Show resolved Hide resolved

lcwangchao changed the title ~~types: use flags in types package to handle clip zero case~~ WIP: types: use flags in types package to handle clip zero case Oct 11, 2023

ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 11, 2023

lcwangchao force-pushed the typectx_clip_zero branch from 48c0251 to e92ff59 Compare October 12, 2023 06:32

Merge branch 'master' into typectx_clip_zero

7f0d336

lcwangchao force-pushed the typectx_clip_zero branch from e92ff59 to 7f0d336 Compare October 12, 2023 07:29

lcwangchao changed the title ~~WIP: types: use flags in types package to handle clip zero case~~ types: use flags in types package to handle clip zero case Oct 12, 2023

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 12, 2023

Merge branch 'master' into typectx_clip_zero

287b2ae

lcwangchao force-pushed the typectx_clip_zero branch from 62829c0 to 287b2ae Compare October 16, 2023 02:47

lcwangchao force-pushed the typectx_clip_zero branch 2 times, most recently from f1bd2c8 to 19d0bef Compare October 16, 2023 06:40

lcwangchao commented Oct 16, 2023

View reviewed changes

pkg/sessionctx/stmtctx/stmtctx.go Outdated Show resolved Hide resolved

lcwangchao force-pushed the typectx_clip_zero branch from 19d0bef to 09f9fa5 Compare October 16, 2023 07:21

json fake context

d942a33

lcwangchao force-pushed the typectx_clip_zero branch from 09f9fa5 to d942a33 Compare October 16, 2023 08:03

lance6716 approved these changes Oct 17, 2023

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Oct 17, 2023

lcwangchao force-pushed the typectx_clip_zero branch from 8a86f34 to be6834f Compare October 18, 2023 05:49

Merge branch 'master' into typectx_clip_zero

b84d8f0

lcwangchao force-pushed the typectx_clip_zero branch from be6834f to b84d8f0 Compare October 18, 2023 05:52

YangKeao reviewed Oct 18, 2023

View reviewed changes

lcwangchao requested review from XuHuaiyu and tangenta October 18, 2023 06:29

YangKeao approved these changes Oct 18, 2023

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Oct 18, 2023

XuHuaiyu approved these changes Oct 19, 2023

View reviewed changes

ti-chi-bot bot added the approved label Oct 19, 2023

Merge branch 'master' into typectx_clip_zero

5590475

ti-chi-bot bot merged commit 987663f into pingcap:master Oct 19, 2023

lcwangchao deleted the typectx_clip_zero branch October 19, 2023 08:16

lcwangchao mentioned this pull request Dec 6, 2023

stmtctx: remove OverflowAsWarning flag #49122

Merged

13 tasks

wuhuizuo pushed a commit to wuhuizuo/tidb that referenced this pull request Apr 2, 2024

types: use flags in types package to handle clip zero case (pingcap#4…

5709ccd

…7543) close pingcap#47518

Conversation

lcwangchao commented Oct 11, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

lcwangchao commented Oct 11, 2023

Uh oh!

Uh oh!

codecov bot commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lcwangchao commented Oct 16, 2023

Uh oh!

ti-chi-bot bot commented Oct 16, 2023

Uh oh!

tiprow bot commented Oct 16, 2023

Uh oh!

Uh oh!

lcwangchao commented Oct 16, 2023

Uh oh!

YangKeao left a comment

Choose a reason for hiding this comment

Uh oh!

YangKeao Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

lcwangchao Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

YangKeao Oct 18, 2023

Choose a reason for hiding this comment

Uh oh!

lcwangchao commented Oct 18, 2023

Uh oh!

tiprow bot commented Oct 18, 2023

Uh oh!

YangKeao left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Oct 18, 2023

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Oct 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Oct 11, 2023 •

edited

Loading