stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx #47592

lcwangchao · 2023-10-12T10:02:54Z

What problem does this PR solve?

Issue Number: close #47590

What is changed and how it works?

Use StatementContext.TypeCtx.Location as the timezone in StatementContext and remove the dup field StatementContext.TimeZone
Provide StatementContext.TimeZone() to return the inner location info.
Provide some construct methods NewStmtCtx, NewStmtCtxWithTimeZone, and Reset to create or reset StatementContext to make sure the inner TypeCtx will be properly inited.
Replace all usage like &stmtctx.StatementContext{...} or new(stmtctx.StatementContext) with these constructors.
Provide SetTimeZone, SetTypeFlags and UpdateTypeFlags to provide easy way to update TypeCtx
Introduce NoCopy struct to mark an object as no copy.

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

codecov · 2023-10-12T10:28:30Z

Codecov Report

Merging #47592 (15745e6) into master (656b96b) will increase coverage by 0.8211%.
Report is 14 commits behind head on master.
The diff coverage is 91.1392%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #47592        +/-   ##
================================================
+ Coverage   71.8876%   72.7088%   +0.8211%     
================================================
  Files          1353       1376        +23     
  Lines        401066     408249      +7183     
================================================
+ Hits         288317     296833      +8516     
+ Misses        93329      92526       -803     
+ Partials      19420      18890       -530

Flag	Coverage Δ
integration	`40.7505% <61.7834%> (?)`
unit	`71.8831% <91.0828%> (-0.0046%)`	⬇️

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

Components	Coverage Δ
dumpling	`53.9913% <ø> (ø)`
parser	`84.7222% <ø> (-0.0106%)`	⬇️
br	`48.9982% <50.0000%> (-4.3797%)`	⬇️

lcwangchao · 2023-10-13T07:35:37Z

/retest

lcwangchao · 2023-10-13T07:53:46Z

/retest

YangKeao

LGTM

YangKeao · 2023-10-13T07:53:21Z

sessionctx/stmtctx/stmtctx.go

@@ -420,6 +425,55 @@ type StatementContext struct {
 	}
 }

+// NewStmtCtx creates a new statement context
+func NewStmtCtx() *StatementContext {


I've tried to write a linter to make sure the &StatementContext{} is only constructed manually in NewStmtCtx or NewStmtCtxWithTimeZone. I'll submit that linter after this PR is merged 🍻

tiprow · 2023-10-13T08:04:24Z

@lcwangchao: 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
tiprow_fast_test	`15745e6`	link	true	`/test tiprow_fast_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.

YangKeao

No Comment

ti-chi-bot · 2023-10-13T09:15:13Z

[LGTM Timeline notifier]

Timeline:

2023-10-13 08:06:58.017973926 +0000 UTC m=+1385215.605084058: ☑️ agreed by YangKeao.
2023-10-13 09:15:11.742792033 +0000 UTC m=+1389309.329902178: ☑️ agreed by lance6716.

windtalker

LGTM

lcwangchao · 2023-10-13T09:43:23Z

/test tiprow_fast_test

ti-chi-bot · 2023-10-13T09:43:26Z

@lcwangchao: The specified target(s) for /test were not found.
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
pull-notify-when-compatibility-sections-changed

In response to this:

/test 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.

easonn7 · 2023-10-13T09:46:10Z

/approve

bb7133

LGTM

ti-chi-bot · 2023-10-13T09:55:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, bb7133, easonn7, lance6716, windtalker, YangKeao

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [3pointer,YangKeao,bb7133,lance6716,windtalker]
~~br/OWNERS~~ [3pointer]
~~br/pkg/lightning/OWNERS~~ [lance6716]
~~distsql/OWNERS~~ [windtalker]
~~expression/OWNERS~~ [windtalker]
~~sessionctx/variable/OWNERS~~ [easonn7]

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

lcwangchao requested a review from a team as a code owner October 12, 2023 10:02

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 12, 2023

lcwangchao mentioned this pull request Oct 12, 2023

move time zone info in stmtctx.StatementContext into TypeCtx #47590

Closed

stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx

464d5a9

lcwangchao force-pushed the stmtctx_timezone branch from bda83f7 to 464d5a9 Compare October 12, 2023 10:04

Merge branch 'master' into stmtctx_timezone

5f2f0c4

lcwangchao force-pushed the stmtctx_timezone branch from b542d27 to 2b1d197 Compare October 12, 2023 10:13

lcwangchao force-pushed the stmtctx_timezone branch 7 times, most recently from 8919ffa to 8195008 Compare October 13, 2023 05:30

update

15745e6

lcwangchao force-pushed the stmtctx_timezone branch from 8195008 to 15745e6 Compare October 13, 2023 06:28

YangKeao reviewed Oct 13, 2023

View reviewed changes

YangKeao approved these changes Oct 13, 2023

View reviewed changes

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

lance6716 approved these changes Oct 13, 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 13, 2023

3pointer approved these changes Oct 13, 2023

View reviewed changes

windtalker approved these changes Oct 13, 2023

View reviewed changes

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

bb7133 approved these changes Oct 13, 2023

View reviewed changes

ti-chi-bot bot merged commit 8d934ea into pingcap:master Oct 13, 2023
15 checks passed

lcwangchao deleted the stmtctx_timezone branch October 13, 2023 13:20

YangKeao mentioned this pull request Oct 16, 2023

Add a linter to ensure the StmtCtx is constructed correctly #47660

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx #47592

stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx #47592

lcwangchao commented Oct 12, 2023 •

edited

Loading

codecov bot commented Oct 12, 2023 •

edited

Loading

lcwangchao commented Oct 13, 2023

lcwangchao commented Oct 13, 2023

YangKeao left a comment •

edited

Loading

YangKeao Oct 13, 2023

tiprow bot commented Oct 13, 2023

YangKeao left a comment

ti-chi-bot bot commented Oct 13, 2023

windtalker left a comment

lcwangchao commented Oct 13, 2023

ti-chi-bot bot commented Oct 13, 2023

easonn7 commented Oct 13, 2023

bb7133 left a comment

ti-chi-bot bot commented Oct 13, 2023

stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx #47592

stmtctx: move time zone info in stmtctx.StatementContext into TypeCtx #47592

Conversation

lcwangchao commented Oct 12, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

codecov bot commented Oct 12, 2023 • edited Loading

Codecov Report

lcwangchao commented Oct 13, 2023

lcwangchao commented Oct 13, 2023

YangKeao left a comment • edited Loading

Choose a reason for hiding this comment

YangKeao Oct 13, 2023

Choose a reason for hiding this comment

tiprow bot commented Oct 13, 2023

YangKeao left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Oct 13, 2023

[LGTM Timeline notifier]

windtalker left a comment

Choose a reason for hiding this comment

lcwangchao commented Oct 13, 2023

ti-chi-bot bot commented Oct 13, 2023

easonn7 commented Oct 13, 2023

bb7133 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Oct 13, 2023

lcwangchao commented Oct 12, 2023 •

edited

Loading

codecov bot commented Oct 12, 2023 •

edited

Loading

YangKeao left a comment •

edited

Loading