Skip to content

sessionctx/variable: fix panic when set variable=''#9533

Merged
xiekeyi98 merged 2 commits intopingcap:masterfrom
crazycs520:fix-set-panic
Mar 4, 2019
Merged

sessionctx/variable: fix panic when set variable=''#9533
xiekeyi98 merged 2 commits intopingcap:masterfrom
crazycs520:fix-set-panic

Conversation

@crazycs520
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

fix panic when execute below sql:

set @@global.max_user_connections='';

What is changed and how it works?

fix checkUInt64SystemVar bug.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

@crazycs520 crazycs520 added sig/execution SIG execution type/bugfix This PR fixes a bug. labels Mar 4, 2019
@crazycs520
Copy link
Copy Markdown
Contributor Author

/run-all-tests

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 4, 2019

Codecov Report

Merging #9533 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9533      +/-   ##
==========================================
- Coverage   67.38%   67.38%   -0.01%     
==========================================
  Files         374      374              
  Lines       78738    78740       +2     
==========================================
- Hits        53058    53057       -1     
- Misses      20905    20907       +2     
- Partials     4775     4776       +1
Impacted Files Coverage Δ
sessionctx/variable/varsutil.go 25.3% <0%> (-0.16%) ⬇️
store/tikv/scan.go 73.94% <0%> (-3.37%) ⬇️
store/tikv/lock_resolver.go 41.7% <0%> (-0.95%) ⬇️
executor/join.go 79.58% <0%> (+0.51%) ⬆️
executor/index_lookup_join.go 78.05% <0%> (+0.62%) ⬆️
expression/schema.go 94.53% <0%> (+0.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0267a7f...cdc0d05. Read the comment docs.

Copy link
Copy Markdown
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@alivxxx alivxxx added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 4, 2019
Copy link
Copy Markdown
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka eurekaka added status/LGT2 Indicates that a PR has LGTM 2. status/all tests passed and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants