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: add store type to statement summary (#17825) #19159

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #17825 to release-4.0


What problem does this PR solve?

Issue Number: close #17278

Problem Summary:

What is changed and how it works?

What's Changed:
Add store type to plan in information_schema.statements_summary

How it Works:
Read store type from physical plan, and add to plan field.

Related changes

Check List

Tests

  • Manual test
mysql root@localhost:test> select * from t;                                                                                                                                                                                                 
0 rows in set
Time: 0.016s
mysql root@localhost:test>                                                                                                                                                                                                                  
mysql root@localhost:test> select plan from information_schema.statements_summary where digest_text='select * from t'; 
                                                                                                                                                                                                                                            
+------------------------------------------------------------------------------------+
| plan                                                                               |
+------------------------------------------------------------------------------------+
|     TableReader_5    root         10000    data:TableFullScan_4                    |
|     └─TableScan_4    cop[tikv]    10000    table:t, keep order:false, stats:pseudo |
+------------------------------------------------------------------------------------+
1 row in set
Time: 0.014s
mysql root@localhost:test>  
  • Unit test

Release note

  • Fix inconsistence of Plan between information_schema.statements_summary and explain, which contains tikv/tiflash info.

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot added contribution This PR is from a community contributor. sig/sql-infra SIG: SQL Infra sig/planner SIG: Planner type/4.0-cherry-pick labels Aug 12, 2020
@ti-srebot ti-srebot added this to the v4.0.6 milestone Aug 12, 2020
@breezewish
Copy link
Member

/run-all-tests

Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 13, 2020
Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

NormalizePlanNode is untouched, so different store types result in the same plan digest.
Is that right? @crazycs520

@crazycs520
Copy link
Contributor

NormalizePlanNode is untouched, so different store types result in the same plan digest.
Is that right? @crazycs520

Great catch, I think it is a bug, @pingyu Could you pull another PR to fix it?

@pingyu
Copy link
Contributor

pingyu commented Aug 14, 2020

NormalizePlanNode is untouched, so different store types result in the same plan digest.
Is that right? @crazycs520

Great catch, I think it is a bug, @pingyu Could you pull another PR to fix it?

Okay, no problem~

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 14, 2020
@AilinKid AilinKid requested a review from zimulala August 14, 2020 06:15
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Aug 14, 2020
@ti-srebot ti-srebot added status/LGT4 The PR has already had 4 LGTM. and removed status/LGT3 The PR has already had 3 LGTM. labels Sep 2, 2020
@zz-jason
Copy link
Member

zz-jason commented Sep 2, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 2, 2020
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 19457
  • 19471
  • 19362

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@zz-jason zz-jason merged commit 293da99 into pingcap:release-4.0 Sep 2, 2020
@breezewish breezewish deleted the release-4.0-148f2d456bdc branch September 2, 2020 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT4 The PR has already had 4 LGTM. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants