parse: fix the type of date/time parameters #48237

YangKeao · 2023-11-02T09:09:52Z

What problem does this PR solve?

Issue Number: close #45190

Problem Summary:

What is changed and how it works?

Refractor the stmtctx in parse to be types.Context.
Construct the date and time from string.

I didn't construct the Datetime and Duration directly from the binary, because developing a binary version of ParseXXX can be a little complicated (especially for handling warnings/errors). To be simple and compatible with old versions, I re-use the string from old version and parse from it.
Refractor the function of parsing parameters into two parts: one for splitting binary protocols into multiple parameters, one for parsing the binary parameters into expressions. The second one needs to run after when the stmtctx is reset, so they need to be split. It also solves some historical problem, as I also saw HandleTruncates besides the date/time part.

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.

Release note

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

None

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

ti-chi-bot · 2023-11-02T09:09:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2023-11-02T09:10:01Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

YangKeao · 2023-11-08T08:38:52Z

pkg/param/binary_params.go

@@ -0,0 +1,275 @@
+// Copyright 2023 PingCAP, Inc.


Most of this file is copied from /pkg/internal/parse. Because it's not expected to be "internal" inside the server.

YangKeao · 2023-11-08T08:40:25Z

pkg/server/conn_stmt_params_test.go

+	return
+}
+
+func TestParseExecArgs(t *testing.T) {


These tests are also moved from internal/parse/parse_test.go.

codecov · 2023-11-08T08:55:09Z

Codecov Report

Attention: Patch coverage is 25.80645% with 230 lines in your changes missing coverage. Please review.

Project coverage is 72.8384%. Comparing base (028d7b8) to head (f61a794).
Report is 3238 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #48237        +/-   ##
================================================
+ Coverage   71.5384%   72.8384%   +1.2999%     
================================================
  Files          1402       1427        +25     
  Lines        406159     414649      +8490     
================================================
+ Hits         290560     302024     +11464     
+ Misses        95775      93671      -2104     
+ Partials      19824      18954       -870

Flag	Coverage Δ
integration	`43.3871% <0.6451%> (?)`
unit	`71.4733% <74.0740%> (-0.0652%)`	⬇️

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

Components	Coverage Δ
dumpling	`53.9874% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.8327% <ø> (-4.0624%)`	⬇️

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

YangKeao · 2023-11-08T12:33:59Z

/retest

YangKeao · 2023-11-09T03:13:55Z

/retest

lcwangchao · 2023-11-09T06:12:11Z

LGTM, but is it possible to add some intergartion tests?

ti-chi-bot · 2023-11-09T06:41:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lcwangchao, xhebox

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~~ [lcwangchao,xhebox]

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

ti-chi-bot · 2023-11-09T06:41:13Z

[LGTM Timeline notifier]

Timeline:

2023-11-09 06:08:40.76028521 +0000 UTC m=+3710918.347395340: ☑️ agreed by lcwangchao.
2023-11-09 06:41:12.229679355 +0000 UTC m=+3712869.816789486: ☑️ agreed by xhebox.

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

YangKeao · 2023-11-09T07:37:18Z

LGTM, but is it possible to add some intergartion tests?

Good. Added some in the latest commit 😄

ti-chi-bot · 2023-11-09T10:40:02Z

@YangKeao: 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/unit-test	`f61a794`	link	unknown	`/test unit-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 · 2023-11-09T10:40:30Z

/retest

ti-chi-bot · 2024-12-16T06:24:33Z

In response to a cherrypick label: new pull request created to branch release-7.5: #58281.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-12-16T06:24:38Z

In response to a cherrypick label: new pull request created to branch release-7.1: #58282.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-12-16T06:24:40Z

In response to a cherrypick label: new pull request created to branch release-6.5: #58283.

ti-chi-bot · 2024-12-16T07:57:47Z

In response to a cherrypick label: new pull request created to branch release-6.1: #58288.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

fix the type of date/time parameters

c68f290

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

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Nov 2, 2023

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 2, 2023

YangKeao commented Nov 8, 2023

View reviewed changes

YangKeao force-pushed the fix-45190 branch 2 times, most recently from c412eb3 to 390feae Compare November 8, 2023 08:41

YangKeao marked this pull request as ready for review November 8, 2023 08:42

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 Nov 8, 2023

YangKeao requested a review from lcwangchao November 8, 2023 08:43

YangKeao force-pushed the fix-45190 branch from 390feae to 366f8cb Compare November 8, 2023 08:54

YangKeao force-pushed the fix-45190 branch 3 times, most recently from f883f2c to 449ce9a Compare November 8, 2023 09:41

use two-stages param parser

91734e2

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

YangKeao force-pushed the fix-45190 branch from 449ce9a to 91734e2 Compare November 8, 2023 12:09

YangKeao changed the title ~~parse: fix the type of date/time parameters~~ parse: fix the type of date/time parameters | tidb=pr/48188 Nov 8, 2023

YangKeao changed the title ~~parse: fix the type of date/time parameters | tidb=pr/48188~~ parse: fix the type of date/time parameters Nov 8, 2023

lcwangchao approved these changes Nov 9, 2023

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Nov 9, 2023

YangKeao requested a review from xhebox November 9, 2023 06:12

xhebox approved these changes Nov 9, 2023

View reviewed changes

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

add integration test

f61a794

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

ti-chi-bot bot merged commit 8ce2ad1 into pingcap:master Nov 9, 2023

YangKeao added needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. labels Dec 16, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 16, 2024

This is an automated cherry-pick of pingcap#48237

126ee90

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 16, 2024

parse: fix the type of date/time parameters (#48237) #58281

Open

5 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 16, 2024

This is an automated cherry-pick of pingcap#48237

f934d24

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 16, 2024

parse: fix the type of date/time parameters (#48237) #58282

Open

5 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 16, 2024

This is an automated cherry-pick of pingcap#48237

81825b1

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 16, 2024

parse: fix the type of date/time parameters (#48237) #58283

Open

5 tasks

YangKeao added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Dec 16, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 16, 2024

This is an automated cherry-pick of pingcap#48237

c42da00

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 16, 2024

parse: fix the type of date/time parameters (#48237) #58288

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parse: fix the type of date/time parameters #48237

parse: fix the type of date/time parameters #48237

YangKeao commented Nov 2, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 2, 2023

tiprow bot commented Nov 2, 2023

YangKeao Nov 8, 2023

YangKeao Nov 8, 2023

codecov bot commented Nov 8, 2023 •

edited

Loading

YangKeao commented Nov 8, 2023

YangKeao commented Nov 9, 2023

lcwangchao commented Nov 9, 2023

ti-chi-bot bot commented Nov 9, 2023

ti-chi-bot bot commented Nov 9, 2023

YangKeao commented Nov 9, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 9, 2023

YangKeao commented Nov 9, 2023

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

parse: fix the type of date/time parameters #48237

parse: fix the type of date/time parameters #48237

Conversation

YangKeao commented Nov 2, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Nov 2, 2023

tiprow bot commented Nov 2, 2023

YangKeao Nov 8, 2023

Choose a reason for hiding this comment

YangKeao Nov 8, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 8, 2023 • edited Loading

Codecov Report

YangKeao commented Nov 8, 2023

YangKeao commented Nov 9, 2023

lcwangchao commented Nov 9, 2023

ti-chi-bot bot commented Nov 9, 2023

ti-chi-bot bot commented Nov 9, 2023

[LGTM Timeline notifier]

YangKeao commented Nov 9, 2023 • edited Loading

ti-chi-bot bot commented Nov 9, 2023

YangKeao commented Nov 9, 2023

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

ti-chi-bot commented Dec 16, 2024

YangKeao commented Nov 2, 2023 •

edited

Loading

codecov bot commented Nov 8, 2023 •

edited

Loading

YangKeao commented Nov 9, 2023 •

edited

Loading