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

executor: fix invalid key error of fast analyze #11072

Merged
merged 3 commits into from
Jul 4, 2019
Merged

executor: fix invalid key error of fast analyze #11072

merged 3 commits into from
Jul 4, 2019

Conversation

alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Jul 4, 2019 •
edited
Loading

What problem does this PR solve? 

[18:6]invalid key - ""
github.com/pingcap/errors.AddStack
        /home/go/pkg/mod/github.com/pingcap/[email protected]/errors.go:174
github.com/pingcap/parser/terror.(*Error).GenWithStack
        /home/go/pkg/mod/github.com/pingcap/[email protected]/terror/terror.go:231
github.com/pingcap/tidb/tablecodec.DecodeRowKey
        /home/go/src/github.com/pingcap/tidb/tablecodec/tablecodec.go:205
github.com/pingcap/tidb/executor.(*AnalyzeFastExec).updateCollectorSamples
        /home/go/src/github.com/pingcap/tidb/executor/analyze.go:801
github.com/pingcap/tidb/executor.(*AnalyzeFastExec).handleBatchSeekResponse
        /home/go/src/github.com/pingcap/tidb/executor/analyze.go:872
github.com/pingcap/tidb/executor.(*AnalyzeFastExec).handleSampTasks
        /home/go/src/github.com/pingcap/tidb/executor/analyze.go:968

What is changed and how it works?

We should check the validity of iterator before using them.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • None

Related changes

  • Need to cherry-pick to the release branch

@alivxxx alivxxx added type/bugfix This PR fixes a bug. sig/execution SIG execution needs-cherry-pick-3.0 labels Jul 4, 2019
@codecov
Copy link

codecov bot commented Jul 4, 2019

Codecov Report

Merging #11072 into master will decrease coverage by 0.0292%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #11072        +/-   ##
================================================
- Coverage   80.9685%   80.9393%   -0.0293%     
================================================
  Files           419        418         -1     
  Lines         89326      89194       -132     
================================================
- Hits          72326      72193       -133     
+ Misses        11767      11759         -8     
- Partials       5233       5242         +9

@codecov
Copy link

codecov bot commented Jul 4, 2019 •
edited
Loading

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11072   +/-   ##
===========================================
  Coverage   81.2155%   81.2155%           
===========================================
  Files           419        419           
  Lines         90117      90117           
===========================================
  Hits          73189      73189           
  Misses        11656      11656           
  Partials       5272       5272

lzmhhh123
lzmhhh123 reviewed Jul 4, 2019
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM.

@lzmhhh123 lzmhhh123 added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 4, 2019
zz-jason
zz-jason approved these changes Jul 4, 2019
View reviewed changes
Copy link
Member

@zz-jason zz-jason 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
Copy link
Contributor Author

alivxxx commented Jul 4, 2019

/run-all-tests

@alivxxx
Copy link
Contributor Author

alivxxx commented Jul 4, 2019

/run-integration-common-test

1 similar comment
@alivxxx
Copy link
Contributor Author

alivxxx commented Jul 4, 2019

/run-integration-common-test

@alivxxx
Copy link
Contributor Author

alivxxx commented Jul 4, 2019

/run-all-tests

@alivxxx alivxxx merged commit 9eb2379 into pingcap:master Jul 4, 2019
@alivxxx alivxxx deleted the invalid branch July 4, 2019 11:51
alivxxx added a commit to alivxxx/tidb that referenced this pull request Jul 5, 2019
@alivxxx
executor: fix invalid key error of fast analyze (pingcap#11072)
0220a96
@alivxxx alivxxx mentioned this pull request Jul 5, 2019
Merged
zz-jason pushed a commit that referenced this pull request Jul 6, 2019
@alivxxx @zz-jason
executor: fix invalid key error of fast analyze (#11072) (#11098)
e0c69f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lzmhhh123 lzmhhh123 lzmhhh123 left review comments

@zz-jason zz-jason zz-jason approved these changes

@winoros winoros Awaiting requested review from winoros

Assignees
No one assigned
Labels
sig/execution SIG execution status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alivxxx @zz-jason @lzmhhh123