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

planner: ensure `execution info` column in EXPLAIN ANALYZE exists #10035

Merged
merged 11 commits into from Apr 9, 2019

Conversation

Projects
None yet
5 participants
@kennytm
Copy link
Member

kennytm commented Apr 3, 2019

What problem does this PR solve?

Fixed #9805.

In #9805, when both tables are empty, the IndexLookUp plan is never visited, so the plan has no statistics. The previous implementation did not consider this case and caused the execution info column missing from some rows. This in turn caused an "index out of bound" panic as the last column is missing, and killing the connection.

What is changed and how it works?

When a plan has no stats, assume the plan is not executed at all and fill in a default value time:0ns, loops:0, rows:0

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Apr 3, 2019

/run-all-tests

@qw4990 qw4990 self-requested a review Apr 4, 2019

@qw4990
Copy link
Contributor

qw4990 left a comment

LGTM

@kennytm kennytm added the status/LGT1 label Apr 4, 2019

@lamxTyler
Copy link
Member

lamxTyler left a comment

LGTM

@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 4, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #10035   +/-   ##
===========================================
  Coverage          ?   78.0118%           
===========================================
  Files             ?        404           
  Lines             ?      82026           
  Branches          ?          0           
===========================================
  Hits              ?      63990           
  Misses            ?      13333           
  Partials          ?       4703

@lamxTyler lamxTyler added status/LGT2 and removed status/LGT1 labels Apr 4, 2019

@qw4990

This comment has been minimized.

Copy link
Contributor

qw4990 commented Apr 4, 2019

/run-all-tests

zz-jason and others added some commits Apr 4, 2019

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Apr 4, 2019

/run-all-tests tidb-test=pr/778

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Apr 4, 2019

The integration test failure is introduced by #9898, because pingcap/tidb-test#766 needs to be merged first.

@zz-jason

This comment has been minimized.

Copy link
Member

zz-jason commented Apr 5, 2019

/run-all-tests tidb-test=pr/766

@zz-jason

This comment has been minimized.

Copy link
Member

zz-jason commented Apr 9, 2019

/run-all-tests

zz-jason and others added some commits Apr 9, 2019

@ngaut ngaut merged commit 7c510b7 into pingcap:master Apr 9, 2019

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/code_coverage Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@kennytm kennytm deleted the kennytm:fix-9805 branch Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.