Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

*: support baseline capture for prepared statements #21271

Merged
merged 1 commit into from
Dec 22, 2020
Merged

*: support baseline capture for prepared statements #21271

merged 1 commit into from
Dec 22, 2020

Conversation

eurekaka
Copy link
Contributor

@eurekaka eurekaka commented Nov 25, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number: close #21215

Problem Summary:

Baseline capture does not support prepared statements.

What is changed and how it works?

What's Changed:

  • extract hint string from plan and store it into statements_summary in SummaryStmt if the query is a prepared statement;
  • in baseline capture, specially handle prepared statements, i.e, use the hint string directly, instead of using explain format='hint' to generate hint string;
  • do not use explain format = 'hint' to check the validation of the bind_sql if it is a prepared statement;
  • do not evolve baseline for prepared statements;

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test: would be covered by UTF

Side effects

  • bind_sql may appear to contain ? now.

Release note

  • support baseline capture for prepared statements

@github-actions github-actions bot added sig/execution SIG execution sig/planner SIG: Planner labels Nov 25, 2020
@eurekaka eurekaka force-pushed the capture_binary branch 2 times, most recently from ff032b2 to 0de03c7 Compare November 25, 2020 03:27
@ichn-hu ichn-hu mentioned this pull request Nov 25, 2020
Closed
@eurekaka eurekaka marked this pull request as ready for review November 25, 2020 09:29
@eurekaka eurekaka requested review from a team as code owners November 25, 2020 09:29
@eurekaka eurekaka requested review from qw4990, winoros and lzmhhh123 and removed request for a team November 25, 2020 09:29
qw4990
qw4990 reviewed Dec 3, 2020
View reviewed changes
bindinfo/handle.go Outdated Show resolved Hide resolved
@eurekaka eurekaka force-pushed the capture_binary branch 2 times, most recently from 62923da to 378b3e8 Compare December 7, 2020 08:17
@eurekaka
Copy link
Contributor Author

eurekaka commented Dec 7, 2020

This PR can partially fix #21475 as well.

@eurekaka eurekaka requested a review from qw4990 December 7, 2020 08:25
@XuHuaiyu XuHuaiyu added the type/bugfix This PR fixes a bug. label Dec 9, 2020
qw4990
qw4990 reviewed Dec 17, 2020
View reviewed changes
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 17, 2020
Reminiscent
Reminiscent reviewed Dec 17, 2020
View reviewed changes
Copy link
Contributor

@Reminiscent Reminiscent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

@qw4990
Copy link
Contributor

qw4990 commented Dec 17, 2020

Please resolve conflicts @eurekaka .

@qw4990
Copy link
Contributor

qw4990 commented Dec 22, 2020

PTAL @Reminiscent

Reminiscent
Reminiscent previously approved these changes Dec 22, 2020
View reviewed changes
Copy link
Contributor

@Reminiscent Reminiscent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 22, 2020
ti-srebot
ti-srebot previously approved these changes Dec 22, 2020
View reviewed changes
@Reminiscent
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 22, 2020
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 21788
  • 21893
  • 21901

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@eurekaka merge failed.

@qw4990
Copy link
Contributor

qw4990 commented Dec 22, 2020

PTAL @eurekaka

@eurekaka eurekaka dismissed stale reviews from ti-srebot and Reminiscent via b2445cc December 22, 2020 07:44
@eurekaka
Copy link
Contributor Author

/run-all-tests

@eurekaka
Copy link
Contributor Author

/run-unit-test

@eurekaka eurekaka merged commit 7b76edc into pingcap:master Dec 22, 2020
@eurekaka eurekaka deleted the capture_binary branch December 22, 2020 08:22
@ti-srebot ti-srebot mentioned this pull request Dec 22, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21931

eurekaka added a commit to ti-srebot/tidb that referenced this pull request Dec 29, 2020
@eurekaka
cherry pick pingcap#21271 to release-4.0
d22f29b 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
jebter pushed a commit that referenced this pull request Dec 29, 2020
@ti-srebot @eurekaka
cherry pick #21271 to release-4.0 (#21931)
c6c5da1 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Kenan Yao <cauchy1992@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qw4990 qw4990 qw4990 approved these changes

@ti-srebot ti-srebot ti-srebot left review comments

@Reminiscent Reminiscent Reminiscent left review comments

@winoros winoros Awaiting requested review from winoros

@lzmhhh123 lzmhhh123 Awaiting requested review from lzmhhh123

Assignees
No one assigned
Labels
epic/sql-plan-management sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SPM: capture failed when the query uses binary protocol
5 participants
@eurekaka @qw4990 @Reminiscent @ti-srebot @XuHuaiyu