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.

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

*: refactor RestrictedSQLExecutor #11904

Merged
merged 3 commits into from
Aug 28, 2019
Merged

Conversation

jackysp
Copy link
Member

@jackysp jackysp commented Aug 28, 2019

Signed-off-by: Shuaipeng Yu jackysp@gmail.com

What problem does this PR solve?

sessionctx.Context in RestrictedSQLExecutor is useless.

What is changed and how it works?

Refactor RestrictedSQLExecutor to make it no longer depend on sessionctx.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
@jackysp jackysp added the type/enhancement The issue or PR belongs to an enhancement. label Aug 28, 2019
@codecov
Copy link

codecov bot commented Aug 28, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@932c4a7). Click here to learn what that means.
The diff coverage is 98.305%.

@@             Coverage Diff             @@
##             master     #11904   +/-   ##
===========================================
  Coverage          ?   81.2449%           
===========================================
  Files             ?        438           
  Lines             ?      94444           
  Branches          ?          0           
===========================================
  Hits              ?      76731           
  Misses            ?      12221           
  Partials          ?       5492

@codecov
Copy link

codecov bot commented Aug 28, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11904   +/-   ##
===========================================
  Coverage   81.2562%   81.2562%           
===========================================
  Files           438        438           
  Lines         94453      94453           
===========================================
  Hits          76749      76749           
  Misses        12231      12231           
  Partials       5473       5473

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@ngaut ngaut left a comment

Choose a reason for hiding this comment

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

LGTM

@ngaut ngaut added the status/can-merge Indicates a PR has been approved by a committer. label Aug 28, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Aug 28, 2019

/run-all-tests

@jackysp jackysp merged commit 073ab9b into pingcap:master Aug 28, 2019
@jackysp jackysp deleted the refactor_restricted branch February 27, 2020 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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