Skip to content

session,executor: tiny clean up the runStmt function#17911

Merged
tiancaiamao merged 5 commits intopingcap:masterfrom
tiancaiamao:tiny-clean-up
Jun 11, 2020
Merged

session,executor: tiny clean up the runStmt function#17911
tiancaiamao merged 5 commits intopingcap:masterfrom
tiancaiamao:tiny-clean-up

Conversation

@tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Jun 10, 2020

What problem does this PR solve?

Problem Summary:

Tiny clean up code after #17678

What is changed and how it works?

What's Changed:

  • Clean up the old runStmt, use runStmtWrap to replace it.
  • rename CommonExec to preparedStmtExec which is only used by ExecPreparedStmt
  • unexport CachedPlanExec which is not used outside the session package

How it Works:

Clean up

Check List

Tests

  • No code

Release note

  • No release note

@tiancaiamao tiancaiamao added component/session type/enhancement The issue or PR belongs to an enhancement. labels Jun 10, 2020
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

After a series of refactoring PRs, I am now satisified with the current Execute* API in the session package.

PTAL @jackysp @coocood @lysu

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #17911 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #17911   +/-   ##
===========================================
  Coverage   79.6779%   79.6779%           
===========================================
  Files           524        524           
  Lines        143470     143470           
===========================================
  Hits         114314     114314           
  Misses        20021      20021           
  Partials       9135       9135           

@coocood
Copy link
Member

coocood commented Jun 10, 2020

LGTM

Copy link
Contributor

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Contributor

jackysp commented Jun 10, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 10, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

@tiancaiamao merge failed.

@jackysp
Copy link
Contributor

jackysp commented Jun 10, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

@tiancaiamao merge failed.

@jackysp
Copy link
Contributor

jackysp commented Jun 10, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jun 10, 2020

@tiancaiamao merge failed.

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-integration-br-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/session status/can-merge Indicates a PR has been approved by a committer. type/enhancement The issue or PR belongs to an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants